[APC Desktop] Shows Error screen on start script failure

Shows the error screen if a script does not start correctly.
Note that a script error returns success = true,
for example a failed login. In this case we do not
have to worry about showing the completion screen because such is bounded
to the progress bar reaching the end.

Change-Id: I318cafadd47524975647a8bf90adf0178a85e632
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3811520
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@{#1032473}

chrome/app/generated_resources.grd

@@ -13480,6 +13480,12 @@ Please help our engineers fix this problem. Tell us what happened right before y
       <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>
+      <message name="IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_ERROR_SCREEN_TITLE" desc="The title shown in the side panel when an error with the script execution occurred." translateable="false">
+        Something went wrong.
+      </message>
+      <message name="IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_ERROR_SCREEN_DESCRIPTION" desc="The description shown in the side panel when an error with the script execution occurred." translateable="false">
+        Check the site and try changing your password.
+      </message>
     </if>
   </messages>
 </release>

chrome/browser/autofill_assistant/password_change/apc_client_impl.cc

@@ -191,7 +191,7 @@ void ApcClientImpl::OnRunComplete(
     apc_external_action_delegate_->ShowCompletionScreen(base::BindRepeating(
         &ApcClientImpl::Stop, base::Unretained(this), result.success));
   } else {
-    // TODO(crbug.com/1324089): Handle failed result.
+    apc_external_action_delegate_->ShowErrorScreen();
     Stop(result.success);
   }
 }

chrome/browser/autofill_assistant/password_change/apc_client_impl_unittest.cc

@@ -64,6 +64,7 @@ class MockApcExternalActionDelegate : public ApcExternalActionDelegate {
 
   MOCK_METHOD(void, ShowStartingScreen, (const GURL&), (override));
   MOCK_METHOD(void, ShowCompletionScreen, (base::RepeatingClosure), (override));
+  MOCK_METHOD(void, ShowErrorScreen, (), (override));
   MOCK_METHOD(void, SetupDisplay, (), (override));
 };
 
@@ -351,7 +352,7 @@ TEST_F(ApcClientImplTest, CreateAndStartApcFlow_Success) {
   EXPECT_TRUE(client->IsRunning());
 
   autofill_assistant::HeadlessScriptController::ScriptResult script_result = {
-      /* success= */ true};
+      .success = true};
 
   EXPECT_CALL(*runtime_manager(),
               SetUIState(autofill_assistant::UIState::kNotShown));
@@ -369,6 +370,39 @@ TEST_F(ApcClientImplTest, CreateAndStartApcFlow_Success) {
   EXPECT_FALSE(client->IsRunning());
 }
 
+TEST_F(ApcClientImplTest, CreateAndStartApcFlow_ScriptFails) {
+  raw_ptr<ApcClient> client =
+      ApcClient::GetOrCreateForWebContents(web_contents());
+
+  // Prepare to extract the callback to the coordinator.
+  ApcOnboardingCoordinator::Callback coordinator_callback;
+  base::MockCallback<ApcClient::ResultCallback> result_callback1,
+      result_callback2;
+  EXPECT_CALL(*coordinator(), PerformOnboarding)
+      .WillOnce(MoveArg<0>(&coordinator_callback));
+
+  client->Start(GURL(kUrl1), kUsername1, /*skip_login=*/false,
+                result_callback1.Get());
+
+  // Prepare to extract the callback to the external script controller.
+  base::OnceCallback<void(
+      autofill_assistant::HeadlessScriptController::ScriptResult)>
+      external_script_controller_callback;
+  EXPECT_CALL(*external_script_controller(), StartScript(_, _))
+      .Times(1)
+      .WillOnce(MoveArg<1>(&external_script_controller_callback));
+
+  // Successful onboarding.
+  std::move(coordinator_callback).Run(true);
+
+  autofill_assistant::HeadlessScriptController::ScriptResult script_result = {
+      .success = false};
+
+  EXPECT_CALL(*apc_external_action_delegate(), ShowErrorScreen());
+
+  std::move(external_script_controller_callback).Run(script_result);
+}
+
 TEST_F(ApcClientImplTest, CreateAndStartApcFlow_fromSettings) {
   // Prepare to extract the callback to the coordinator.
   ApcOnboardingCoordinator::Callback coordinator_callback;

chrome/browser/autofill_assistant/password_change/apc_external_action_delegate.cc

@@ -243,6 +243,10 @@ void ApcExternalActionDelegate::ShowCompletionScreen(
       std::move(onShowCompletionScreenDoneButtonClicked));
 }
 
+void ApcExternalActionDelegate::ShowErrorScreen() {
+  password_change_run_display_->ShowErrorScreen();
+}
+
 void ApcExternalActionDelegate::Show(
     base::WeakPtr<PasswordChangeRunDisplay> password_change_run_display) {
   password_change_run_display_ = password_change_run_display;

chrome/browser/autofill_assistant/password_change/apc_external_action_delegate.h

@@ -82,6 +82,7 @@ class ApcExternalActionDelegate
   void ShowStartingScreen(const GURL& url) override;
   void ShowCompletionScreen(
       base::RepeatingClosure onShowCompletionScreenDoneButtonClicked) override;
+  void ShowErrorScreen() override;
 
  private:
   friend class ApcExternalActionDelegateTest;

chrome/browser/autofill_assistant/password_change/apc_external_action_delegate_unittest.cc

@@ -242,6 +242,23 @@ TEST_F(ApcExternalActionDelegateTest, ShowStartingScreen) {
   action_delegate()->ShowStartingScreen(url);
 }
 
+TEST_F(ApcExternalActionDelegateTest, ShowCompletionScreen) {
+  const GURL url(kUrl);
+
+  base::RepeatingClosure show_completion_screen_callback;
+  EXPECT_CALL(*display(),
+              ShowCompletionScreen(show_completion_screen_callback));
+
+  action_delegate()->ShowCompletionScreen(show_completion_screen_callback);
+}
+
+TEST_F(ApcExternalActionDelegateTest, ShowErrorScreen) {
+  const GURL url(kUrl);
+
+  EXPECT_CALL(*display(), ShowErrorScreen());
+
+  action_delegate()->ShowErrorScreen();
+}
 TEST_F(ApcExternalActionDelegateTest, ReceiveInvalidAction) {
   autofill_assistant::external::Action empty_action;
 

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

@@ -51,6 +51,7 @@ class MockPasswordChangeRunController : public PasswordChangeRunController {
               ShowCompletionScreen,
               (base::RepeatingClosure done_button_callback),
               (override));
+  MOCK_METHOD(void, ShowErrorScreen, (), (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

@@ -50,6 +50,7 @@ class MockPasswordChangeRunDisplay : public PasswordChangeRunDisplay {
               ShowCompletionScreen,
               (base::RepeatingClosure done_button_callback),
               (override));
+  MOCK_METHOD(void, ShowErrorScreen, (), (override));
   MOCK_METHOD(void, OnControllerGone, (), (override));
 
   base::WeakPtr<MockPasswordChangeRunDisplay> GetWeakPtr() {

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

@@ -70,6 +70,9 @@ class PasswordChangeRunController {
   virtual void ShowCompletionScreen(
       base::RepeatingClosure done_button_callback) = 0;
 
+  // Shows the error screen.
+  virtual void ShowErrorScreen() = 0;
+
   // Returns a weak pointer to this controller.
   virtual base::WeakPtr<PasswordChangeRunController> GetWeakPtr() = 0;
 

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

@@ -70,6 +70,7 @@ class PasswordChangeRunDisplay {
   virtual void ShowStartingScreen(const GURL& url) = 0;
   virtual void ShowCompletionScreen(
       base::RepeatingClosure done_button_callback) = 0;
+  virtual void ShowErrorScreen() = 0;
 
   // Notifies the view that the controller was destroyed so that the view
   // can close itself.

chrome/browser/ui/views/autofill_assistant/password_change/password_change_run_progress.cc

@@ -292,6 +292,11 @@ void PasswordChangeRunProgress::SetAnimationEndedCallback(
           .icon->SetAnimationEndedCallback(std::move(callback));
 }
 
+void PasswordChangeRunProgress::StopAnimation() {
+  progress_step_ui_elements_[current_progress_step_]
+      .icon->StopPulsingAnimation();
+}
+
 void PasswordChangeRunProgress::OnLastProgressBarAnimationCompleted() {
   progress_step_ui_elements_
       [autofill_assistant::password_change::ProgressStep::PROGRESS_STEP_END]

chrome/browser/ui/views/autofill_assistant/password_change/password_change_run_progress.h

@@ -52,6 +52,11 @@ class PasswordChangeRunProgress : public views::View {
   // The completion happens after the last step animation is done.
   void SetAnimationEndedCallback(base::OnceClosure callback);
 
+  // Stops the current step animation. Note that this only stops the icon
+  // animation. Once stopped, an animation cannot be resumed for the current
+  // stage.
+  void StopAnimation();
+
  private:
   // Method ran once the last progress bar animation is completed,
   // Used to trigger the last item animation.

chrome/browser/ui/views/autofill_assistant/password_change/password_change_run_view.cc

@@ -251,6 +251,16 @@ void PasswordChangeRunView::ShowStartingScreen(const GURL& url) {
   SetDescription(std::u16string());
 }
 
+void PasswordChangeRunView::ShowErrorScreen() {
+  password_change_run_progress_->StopAnimation();
+  SetTopIcon(
+      autofill_assistant::password_change::TopIcon::TOP_ICON_ERROR_OCCURRED);
+  SetTitle(l10n_util::GetStringUTF16(
+      IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_ERROR_SCREEN_TITLE));
+  SetDescription(l10n_util::GetStringUTF16(
+      IDS_AUTOFILL_ASSISTANT_PASSWORD_CHANGE_ERROR_SCREEN_DESCRIPTION));
+}
+
 void PasswordChangeRunView::ShowCompletionScreen(
     base::RepeatingClosure done_button_callback) {
   show_completion_screen_done_button_callback_ =

chrome/browser/ui/views/autofill_assistant/password_change/password_change_run_view.h

@@ -72,6 +72,7 @@ class PasswordChangeRunView : public views::View,
   void ShowStartingScreen(const GURL& url) override;
   void ShowCompletionScreen(
       base::RepeatingClosure done_button_callback) override;
+  void ShowErrorScreen() override;
   void OnControllerGone() override;
 
   // Returns a weak pointer to itself.