[APC desktop] Add completion screen.

- We still need to update the description to link the user to password manager.

https://screencast.googleplex.com/cast/NTI2OTMyMjc2NDEyNDE2MHw1Mjg1NjNjMi0zNQ

Bug: 1329179
Change-Id: I61e28ad33cb0bc45b00835b0ac0226478638f730
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3802731
Reviewed-by: Jan Keitel <jkeitel@google.com>
Commit-Queue: Bruno Braga <brunobraga@google.com>
Reviewed-by: Luca Hunkeler <hluca@google.com>
Cr-Commit-Position: refs/heads/main@{#1031462}

chrome/app/generated_resources.grd

@@ -13444,6 +13444,15 @@ Please help our engineers fix this problem. Tell us what happened right before y
       <message name="IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_STARTING_SCREEN_TITLE" desc="The title shown on the side panel after an Automated Password Change flow has been started. This message is shown until the first response from a script is received that resets the initial state.">
         Opening <ph name="URL">$1<ex>example.com</ex></ph>...
       </message>
+      <message name="IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_SUCCESSFULLY_CHANGED_PASSWORD_TITLE" desc="The title shown on the side panel after an Automated Password Change flow has been succeeded." translateable="false" >
+        Successfully changed the compromised password
+      </message>
+      <message name="IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_SUCCESSFULLY_CHANGED_PASSWORD_DESCRIPTION" desc="The description shown on the side panel after an Automated Password Change flow has been succeeded." translateable="false" >
+        Check your passwords any time in <ph name="GOOGLE_PASSWORD_MANAGER">$1<ex>Google Password Manager</ex></ph>.
+      </message>
+      <message name="IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_SUCCESSFULLY_CHANGED_PASSWORD_CLOSE_SIDE_PANEL" desc="The label of the button shown in the side panel after a sucessfull run. Once a user clicks on it, the side panel will close." translateable="false">
+        Done
+      </message>
     </if>
   </messages>
 </release>

chrome/browser/autofill_assistant/password_change/apc_client_impl.cc

@@ -22,7 +22,6 @@
 #include "chrome/common/channel_info.h"
 #include "components/autofill_assistant/browser/public/autofill_assistant_factory.h"
 #include "components/autofill_assistant/browser/public/headless_script_controller.h"
-#include "components/autofill_assistant/browser/public/password_change/empty_website_login_manager_impl.h"
 #include "components/autofill_assistant/browser/public/password_change/website_login_manager_impl.h"
 #include "components/autofill_assistant/browser/public/public_script_parameters.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
@@ -60,6 +59,7 @@ void ApcClientImpl::Start(
     std::move(callback).Run(false);
     return;
   }
+
   // Ensure that only one run is ongoing.
   if (is_running_) {
     std::move(callback).Run(false);
@@ -171,6 +171,13 @@ void ApcClientImpl::OnOnboardingComplete(bool success) {
   }
 
   scrim_manager_ = CreateApcScrimManager();
+
+  website_login_manager_ = CreateWebsiteLoginManager();
+
+  apc_external_action_delegate_ = CreateApcExternalActionDelegate();
+  apc_external_action_delegate_->SetupDisplay();
+  apc_external_action_delegate_->ShowStartingScreen(url_);
+
   external_script_controller_ = CreateHeadlessScriptController();
   scrim_manager_->Show();
   external_script_controller_->StartScript(
@@ -180,8 +187,13 @@ void ApcClientImpl::OnOnboardingComplete(bool success) {
 
 void ApcClientImpl::OnRunComplete(
     autofill_assistant::HeadlessScriptController::ScriptResult result) {
-  // TODO(crbug.com/1324089): Handle failed result.
-  Stop(result.success);
+  if (result.success) {
+    apc_external_action_delegate_->ShowCompletionScreen(base::BindRepeating(
+        &ApcClientImpl::Stop, base::Unretained(this), result.success));
+  } else {
+    // TODO(crbug.com/1324089): Handle failed result.
+    Stop(result.success);
+  }
 }
 
 void ApcClientImpl::OnHidden() {
@@ -206,16 +218,8 @@ ApcClientImpl::CreateSidePanel() {
 std::unique_ptr<autofill_assistant::HeadlessScriptController>
 ApcClientImpl::CreateHeadlessScriptController() {
   DCHECK(scrim_manager_);
-
-  website_login_manager_ =
-      std::make_unique<autofill_assistant::WebsiteLoginManagerImpl>(
-          GetPasswordManagerClient(), &GetWebContents());
-
-  apc_external_action_delegate_ = std::make_unique<ApcExternalActionDelegate>(
-      side_panel_coordinator_.get(), scrim_manager_.get(),
-      website_login_manager_.get());
-  apc_external_action_delegate_->SetupDisplay();
-  apc_external_action_delegate_->ShowStartingScreen(url_);
+  DCHECK(apc_external_action_delegate_);
+  DCHECK(website_login_manager_);
 
   std::unique_ptr<autofill_assistant::AutofillAssistant> autofill_assistant =
       autofill_assistant::AutofillAssistantFactory::CreateForBrowserContext(
@@ -235,6 +239,22 @@ std::unique_ptr<ApcScrimManager> ApcClientImpl::CreateApcScrimManager() {
   return ApcScrimManager::Create(&GetWebContents());
 }
 
+std::unique_ptr<ApcExternalActionDelegate>
+ApcClientImpl::CreateApcExternalActionDelegate() {
+  DCHECK(scrim_manager_);
+  DCHECK(website_login_manager_);
+
+  return std::make_unique<ApcExternalActionDelegate>(
+      side_panel_coordinator_.get(), scrim_manager_.get(),
+      website_login_manager_.get());
+}
+
+std::unique_ptr<autofill_assistant::WebsiteLoginManager>
+ApcClientImpl::CreateWebsiteLoginManager() {
+  return std::make_unique<autofill_assistant::WebsiteLoginManagerImpl>(
+      GetPasswordManagerClient(), &GetWebContents());
+}
+
 password_manager::PasswordManagerClient*
 ApcClientImpl::GetPasswordManagerClient() {
   return ChromePasswordManagerClient::FromWebContents(&GetWebContents());

chrome/browser/autofill_assistant/password_change/apc_client_impl.h

@@ -79,6 +79,16 @@ class ApcClientImpl : public content::WebContentsUserData<ApcClientImpl>,
   // during script runs.
   virtual std::unique_ptr<ApcScrimManager> CreateApcScrimManager();
 
+  // Creates the external action delegate responsible for receiving and handling
+  // action protos.
+  virtual std::unique_ptr<ApcExternalActionDelegate>
+  CreateApcExternalActionDelegate();
+
+  // Creates the website login manager to handle interactions with the password
+  // manager.
+  virtual std::unique_ptr<autofill_assistant::WebsiteLoginManager>
+  CreateWebsiteLoginManager();
+
   // Get the `PasswordManagerClient` so that we can initialize
   // `website_login_manager_`.
   virtual password_manager::PasswordManagerClient* GetPasswordManagerClient();

chrome/browser/autofill_assistant/password_change/apc_client_impl_unittest.cc

@@ -7,11 +7,13 @@
 #include <memory>
 #include <string>
 
+#include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/memory/raw_ptr.h"
 #include "base/test/gmock_move_support.h"
 #include "base/test/mock_callback.h"
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/autofill_assistant/password_change/apc_external_action_delegate.h"
 #include "chrome/browser/autofill_assistant/password_change/apc_onboarding_coordinator_impl.h"
 #include "chrome/browser/autofill_assistant/password_change/mock_apc_onboarding_coordinator.h"
 #include "chrome/browser/ui/autofill_assistant/password_change/mock_apc_scrim_manager.h"
@@ -20,6 +22,7 @@
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "components/autofill_assistant/browser/public/mock_headless_script_controller.h"
 #include "components/autofill_assistant/browser/public/mock_runtime_manager.h"
+#include "components/autofill_assistant/browser/public/password_change/mock_website_login_manager.h"
 #include "components/password_manager/core/browser/password_manager_client.h"
 #include "components/password_manager/core/browser/password_manager_client_helper.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
@@ -48,6 +51,22 @@ constexpr char kSourcePasswordChangeSettings[] = "11";
 constexpr int kDescriptionId1 = 3;
 constexpr int kDescriptionId2 = 17;
 
+class MockApcExternalActionDelegate : public ApcExternalActionDelegate {
+ public:
+  MockApcExternalActionDelegate(
+      AssistantDisplayDelegate* display_delegate,
+      ApcScrimManager* apc_scrim_manager,
+      autofill_assistant::WebsiteLoginManager* website_login_manager)
+      : ApcExternalActionDelegate(display_delegate,
+                                  apc_scrim_manager,
+                                  website_login_manager) {}
+  ~MockApcExternalActionDelegate() override = default;
+
+  MOCK_METHOD(void, ShowStartingScreen, (const GURL&), (override));
+  MOCK_METHOD(void, ShowCompletionScreen, (base::RepeatingClosure), (override));
+  MOCK_METHOD(void, SetupDisplay, (), (override));
+};
+
 }  // namespace
 
 using ::testing::_;
@@ -85,6 +104,16 @@ class TestApcClientImpl : public ApcClientImpl {
     return std::move(scrim_manager_);
   }
 
+  std::unique_ptr<ApcExternalActionDelegate> CreateApcExternalActionDelegate()
+      override {
+    return std::move(apc_external_action_delegate_);
+  }
+
+  std::unique_ptr<autofill_assistant::WebsiteLoginManager>
+  CreateWebsiteLoginManager() override {
+    return std::move(website_login_manager_);
+  }
+
   password_manager::PasswordManagerClient* GetPasswordManagerClient() override {
     return password_manager_client_.get();
   }
@@ -122,6 +151,19 @@ class TestApcClientImpl : public ApcClientImpl {
     scrim_manager_ = std::move(scrim_manager);
   }
 
+  // Allows setting a ApcExternalActionDelegate.
+  void InjectApcExternalActionDelegateForTesting(
+      std::unique_ptr<ApcExternalActionDelegate> apc_external_action_delegate) {
+    apc_external_action_delegate_ = std::move(apc_external_action_delegate);
+  }
+
+  // Allows setting a WebsiteLoginManager.
+  void InjectWebsiteLoginManagerForTesting(
+      std::unique_ptr<autofill_assistant::WebsiteLoginManager>
+          website_login_manager) {
+    website_login_manager_ = std::move(website_login_manager);
+  }
+
   // Allows setting an PasswordManagerClient.
   void InjectPasswordManagerClientForTesting(
       std::unique_ptr<password_manager::PasswordManagerClient>
@@ -136,6 +178,9 @@ class TestApcClientImpl : public ApcClientImpl {
       external_script_controller_;
   raw_ptr<autofill_assistant::RuntimeManager> runtime_manager_;
   std::unique_ptr<ApcScrimManager> scrim_manager_;
+  std::unique_ptr<ApcExternalActionDelegate> apc_external_action_delegate_;
+  std::unique_ptr<autofill_assistant::WebsiteLoginManager>
+      website_login_manager_;
   std::unique_ptr<password_manager::PasswordManagerClient>
       password_manager_client_;
 };
@@ -195,14 +240,33 @@ class ApcClientImplTest : public ChromeRenderViewHostTestHarness {
     // Prepare the PasswordManagerClient.
     auto password_manager_client =
         std::make_unique<password_manager::StubPasswordManagerClient>();
+    password_manager_client_ref_ = password_manager_client.get();
     test_apc_client_->InjectPasswordManagerClientForTesting(
         std::move(password_manager_client));
+
+    // Prepare the WebsiteLoginManager.
+    auto website_login_manager =
+        std::make_unique<autofill_assistant::MockWebsiteLoginManager>();
+    website_login_manager_ref_ = website_login_manager.get();
+    test_apc_client_->InjectWebsiteLoginManagerForTesting(
+        std::move(website_login_manager));
+
+    // Prepare the ApcExternalActionDelegate.
+    auto apc_external_action_delegate =
+        std::make_unique<MockApcExternalActionDelegate>(
+            side_panel_ref_, scrim_manager_ref_, website_login_manager_ref_);
+    apc_external_action_delegate_ref_ = apc_external_action_delegate.get();
+    test_apc_client_->InjectApcExternalActionDelegateForTesting(
+        std::move(apc_external_action_delegate));
   }
 
   TestApcClientImpl* apc_client() { return test_apc_client_; }
   MockApcOnboardingCoordinator* coordinator() { return coordinator_ref_; }
   MockAssistantSidePanelCoordinator* side_panel() { return side_panel_ref_; }
   MockApcScrimManager* scrim_manager() { return scrim_manager_ref_; }
+  MockApcExternalActionDelegate* apc_external_action_delegate() {
+    return apc_external_action_delegate_ref_;
+  }
   AssistantSidePanelCoordinator::Observer* side_panel_observer() {
     return side_panel_observer_;
   }
@@ -224,6 +288,12 @@ class ApcClientImplTest : public ChromeRenderViewHostTestHarness {
   raw_ptr<autofill_assistant::MockHeadlessScriptController>
       external_script_controller_ref_ = nullptr;
   raw_ptr<MockApcScrimManager> scrim_manager_ref_ = nullptr;
+  raw_ptr<MockApcExternalActionDelegate> apc_external_action_delegate_ref_ =
+      nullptr;
+  raw_ptr<password_manager::StubPasswordManagerClient>
+      password_manager_client_ref_ = nullptr;
+  raw_ptr<autofill_assistant::WebsiteLoginManager> website_login_manager_ref_ =
+      nullptr;
 
   // The last registered side panel observer - may be null or dangling.
   raw_ptr<AssistantSidePanelCoordinator::Observer> side_panel_observer_ =
@@ -254,6 +324,7 @@ TEST_F(ApcClientImplTest, CreateAndStartApcFlow_Success) {
       .WillOnce(MoveArg<0>(&coordinator_callback));
   EXPECT_CALL(*runtime_manager(),
               SetUIState(autofill_assistant::UIState::kShown));
+  EXPECT_CALL(*apc_external_action_delegate(), ShowStartingScreen(GURL(kUrl1)));
   EXPECT_CALL(*scrim_manager(), Show());
 
   client->Start(GURL(kUrl1), kUsername1, /*skip_login=*/false,
@@ -281,10 +352,20 @@ TEST_F(ApcClientImplTest, CreateAndStartApcFlow_Success) {
 
   autofill_assistant::HeadlessScriptController::ScriptResult script_result = {
       /* success= */ true};
+
   EXPECT_CALL(*runtime_manager(),
               SetUIState(autofill_assistant::UIState::kNotShown));
   EXPECT_CALL(result_callback1, Run(true));
+
+  // Prepare to extract the callback from the completion screen call.
+  base::RepeatingClosure show_completion_screen_callback;
+  EXPECT_CALL(*apc_external_action_delegate(), ShowCompletionScreen(_))
+      .Times(1)
+      .WillOnce(MoveArg<0>(&show_completion_screen_callback));
+
   std::move(external_script_controller_callback).Run(script_result);
+  std::move(show_completion_screen_callback).Run();
+
   EXPECT_FALSE(client->IsRunning());
 }
 

chrome/browser/autofill_assistant/password_change/apc_external_action_delegate.cc

@@ -234,22 +234,13 @@ void ApcExternalActionDelegate::OnGeneratedPasswordSelected(
 }
 
 void ApcExternalActionDelegate::ShowStartingScreen(const GURL& url) {
-  SetTopIcon(
-      autofill_assistant::password_change::TopIcon::TOP_ICON_UNSPECIFIED);
-  SetProgressBarStep(
-      autofill_assistant::password_change::ProgressStep::PROGRESS_STEP_START);
-
-  const std::u16string formatted_url = url_formatter::FormatUrl(
-      url,
-      url_formatter::kFormatUrlOmitHTTP | url_formatter::kFormatUrlOmitHTTPS |
-          url_formatter::kFormatUrlOmitTrivialSubdomains |
-          url_formatter::kFormatUrlTrimAfterHost,
-      base::UnescapeRule::SPACES, /*new_parsed=*/nullptr,
-      /*prefix_end=*/nullptr, /*offset_for_adjustment=*/nullptr);
-  SetTitle(l10n_util::GetStringFUTF16(
-      IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_STARTING_SCREEN_TITLE,
-      formatted_url));
-  SetDescription(std::u16string());
+  password_change_run_display_->ShowStartingScreen(url);
+}
+
+void ApcExternalActionDelegate::ShowCompletionScreen(
+    base::RepeatingClosure onShowCompletionScreenDoneButtonClicked) {
+  password_change_run_display_->ShowCompletionScreen(
+      std::move(onShowCompletionScreenDoneButtonClicked));
 }
 
 void ApcExternalActionDelegate::Show(

chrome/browser/autofill_assistant/password_change/apc_external_action_delegate.h

@@ -17,6 +17,7 @@
 class PasswordChangeRunDisplay;
 class AssistantDisplayDelegate;
 class ApcScrimManager;
+class GURL;
 
 namespace autofill_assistant {
 struct RectF;
@@ -45,7 +46,7 @@ class ApcExternalActionDelegate
 
   // Sets up the display to render a password change run UI,
   // needs to be called BEFORE starting a script.
-  void SetupDisplay();
+  virtual void SetupDisplay();
 
   // ExternalActionDelegate:
   void OnActionRequested(
@@ -79,6 +80,8 @@ class ApcExternalActionDelegate
       const std::u16string& generated_password) override;
   void OnGeneratedPasswordSelected(bool selected) override;
   void ShowStartingScreen(const GURL& url) override;
+  void ShowCompletionScreen(
+      base::RepeatingClosure onShowCompletionScreenDoneButtonClicked) override;
 
  private:
   friend class ApcExternalActionDelegateTest;

chrome/browser/autofill_assistant/password_change/apc_external_action_delegate_unittest.cc

@@ -237,14 +237,7 @@ TEST_F(ApcExternalActionDelegateTest, OnTouchableAreaChangedShowAndHideScrim) {
 TEST_F(ApcExternalActionDelegateTest, ShowStartingScreen) {
   const GURL url(kUrl);
 
-  EXPECT_CALL(*display(), SetTopIcon(TopIcon::TOP_ICON_UNSPECIFIED));
-  EXPECT_CALL(*display(),
-              SetProgressBarStep(ProgressStep::PROGRESS_STEP_START));
-  EXPECT_CALL(*display(),
-              SetTitle(l10n_util::GetStringFUTF16(
-                  IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_STARTING_SCREEN_TITLE,
-                  u"example.com")));
-  EXPECT_CALL(*display(), SetDescription(std::u16string()));
+  EXPECT_CALL(*display(), ShowStartingScreen(url));
 
   action_delegate()->ShowStartingScreen(url);
 }

chrome/browser/ui/autofill_assistant/password_change/mock_password_change_run_controller.h

@@ -7,6 +7,7 @@
 
 #include "chrome/browser/ui/autofill_assistant/password_change/password_change_run_controller.h"
 
+#include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "url/gurl.h"
@@ -46,6 +47,10 @@ class MockPasswordChangeRunController : public PasswordChangeRunController {
                const std::u16string&),
               (override));
   MOCK_METHOD(void, ShowStartingScreen, (const GURL&), (override));
+  MOCK_METHOD(void,
+              ShowCompletionScreen,
+              (base::RepeatingClosure done_button_callback),
+              (override));
   MOCK_METHOD(void, OnGeneratedPasswordSelected, (bool), (override));
   base::WeakPtr<PasswordChangeRunController> GetWeakPtr() override {
     return weak_ptr_factory_.GetWeakPtr();

chrome/browser/ui/autofill_assistant/password_change/mock_password_change_run_display.h

@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "components/autofill_assistant/browser/public/password_change/proto/actions.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -44,6 +45,11 @@ class MockPasswordChangeRunDisplay : public PasswordChangeRunDisplay {
                const PromptChoice&),
               (override));
   MOCK_METHOD(void, ClearPrompt, (), (override));
+  MOCK_METHOD(void, ShowStartingScreen, (const GURL&), (override));
+  MOCK_METHOD(void,
+              ShowCompletionScreen,
+              (base::RepeatingClosure done_button_callback),
+              (override));
   MOCK_METHOD(void, OnControllerGone, (), (override));
 
   base::WeakPtr<MockPasswordChangeRunDisplay> GetWeakPtr() {