This is the cherry pick merge for the M73 branch which was approved here: http://crbug.com/927203

[Autofill Assistant] Avoid race conditions in graceful shutdown.

Before this patch, there were cases where the controller would enter
graceful shutdown because of a script, with a successful message, and
then notice an error, and change that message to an error. The error was
usually caused by DidStartNavigation noticing an unexpected navigation
event, triggered by an action run at the end of the last script.

This patch attempts to avoid such case by:
 - only checking for unexpected navigation events in the PROMPT state.
 This specifically excludes the STOPPED and RUNNING states.
 - not reporting errors or accepting script updates (OnGetScript) in the
 STOPPED state
 - stopping script checks when entering the fatal error state

Bug: 806868
Change-Id: I408d2652257d9524740e44c79b7fa5201ef7cc17
Reviewed-on: https://chromium-review.googlesource.com/c/1436047
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#626937}(cherry picked from commit a42a78954e4c0da3d5b7dd1935626e815416fe09)
Reviewed-on: https://chromium-review.googlesource.com/c/1449551
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#108}
Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896}
diff --git a/components/autofill_assistant/browser/controller.cc b/components/autofill_assistant/browser/controller.cc
index e26c580..7509aa7 100644
--- a/components/autofill_assistant/browser/controller.cc
+++ b/components/autofill_assistant/browser/controller.cc
@@ -147,6 +147,8 @@
 void Controller::EnterState(AutofillAssistantState state) {
   if (state_ == state)
     return;
+  DCHECK_NE(state_, AutofillAssistantState::STOPPED)
+      << "Unexpected transition from STOPPED to " << static_cast<int>(state);
 
   state_ = state;
   GetUiController()->OnStateChanged(state);
@@ -204,10 +206,8 @@
   if (should_fail_after_checking_scripts_ &&
       ++total_script_check_count_ >= kAutostartCheckCountLimit) {
     should_fail_after_checking_scripts_ = false;
-    SetStatusMessage(
-        l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_DEFAULT_ERROR));
-    stop_reason_ = Metrics::AUTOSTART_TIMEOUT;
-    EnterState(AutofillAssistantState::STOPPED);
+    OnFatalError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_DEFAULT_ERROR),
+                 Metrics::AUTOSTART_TIMEOUT);
     return;
   }
 
@@ -223,6 +223,9 @@
 void Controller::OnGetScripts(const GURL& url,
                               bool result,
                               const std::string& response) {
+  if (state_ == AutofillAssistantState::STOPPED)
+    return;
+
   // If the domain of the current URL changed since the request was sent, the
   // response is not relevant anymore and can be safely discarded.
   if (url.host() != script_domain_)
@@ -265,10 +268,8 @@
                                   const ScriptExecutor::Result& result) {
   if (!result.success) {
     LOG(ERROR) << "Failed to execute script " << script_path;
-    SetStatusMessage(
-        l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_DEFAULT_ERROR));
-    stop_reason_ = Metrics::SCRIPT_FAILED;
-    EnterState(AutofillAssistantState::STOPPED);
+    OnFatalError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_DEFAULT_ERROR),
+                 Metrics::SCRIPT_FAILED);
     return;
   }
 
@@ -320,8 +321,13 @@
   GetOrCheckScripts(web_contents()->GetLastCommittedURL());
 }
 
-void Controller::GiveUp(Metrics::DropOutReason reason) {
-  SetStatusMessage(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP));
+void Controller::OnFatalError(const std::string& error_message,
+                              Metrics::DropOutReason reason) {
+  if (state_ == AutofillAssistantState::STOPPED)
+    return;
+
+  StopPeriodicScriptChecks();
+  SetStatusMessage(error_message);
   stop_reason_ = reason;
   EnterState(AutofillAssistantState::STOPPED);
 }
@@ -480,7 +486,8 @@
 
   // We're navigated to a page that has no scripts or the scripts have reached a
   // state from which they cannot recover through a DOM change.
-  GiveUp(Metrics::NO_SCRIPTS);
+  OnFatalError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP),
+               Metrics::NO_SCRIPTS);
   return;
 }
 
@@ -488,7 +495,7 @@
     const std::vector<ScriptHandle>& runnable_scripts) {
   // Script selection is disabled when a script is already running. We will
   // check again and maybe update when the current script has finished.
-  if (script_tracker()->running())
+  if (script_tracker()->running() || state_ == AutofillAssistantState::STOPPED)
     return;
 
   if (!runnable_scripts.empty()) {
@@ -557,10 +564,8 @@
 
 void Controller::DidStartNavigation(
     content::NavigationHandle* navigation_handle) {
-  if (!started_)
-    return;
-
-  // The following types of navigations are allowed for the main frame:
+  // The following types of navigations are allowed for the main frame, when
+  // in PROMPT state:
   //  - first-time URL load
   //  - script-directed navigation, while a script is running unless
   //    there's a touchable area.
@@ -574,16 +579,14 @@
   //
   // Everything else, such as going back to a previous page, or refreshing the
   // page is considered an end condition.
-  if (navigation_handle->IsInMainFrame() &&
+  if (state_ == AutofillAssistantState::PROMPT &&
+      navigation_handle->IsInMainFrame() &&
       web_contents()->GetLastCommittedURL().is_valid() &&
       !navigation_handle->WasServerRedirect() &&
       !navigation_handle->IsSameDocument() &&
       !navigation_handle->IsRendererInitiated()) {
-    // The action can define a touchable element area that prevents navigation.
-    if (!script_tracker_ || !script_tracker()->running() ||
-        touchable_element_area()->HasElements()) {
-      GiveUp(Metrics::NAVIGATION);
-    }
+    OnFatalError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP),
+                 Metrics::NAVIGATION);
   }
 }
 
diff --git a/components/autofill_assistant/browser/controller.h b/components/autofill_assistant/browser/controller.h
index 0bf5641..cf9ceb7 100644
--- a/components/autofill_assistant/browser/controller.h
+++ b/components/autofill_assistant/browser/controller.h
@@ -103,7 +103,10 @@
   void StartPeriodicScriptChecks();
   void StopPeriodicScriptChecks();
   void OnPeriodicScriptCheck();
-  void GiveUp(Metrics::DropOutReason reason);
+
+  // Shows the given message and stops the controller with |reason|.
+  void OnFatalError(const std::string& error_message,
+                    Metrics::DropOutReason reason);
 
   // Runs autostart scripts from |runnable_scripts|, if the conditions are
   // right. Returns true if a script was auto-started.