[Autofill Assistant] Remove STOPPED UI on navigation

When detaching the |UiControllerAndroid| this adds a new
|WebContentsObserver| to the current |WebContent|, that triggers the
UI self destruct on navigation.

This can not be handled in the |Controller| as this one gets destroyed
when the |Client| shuts down.

Bug: b/159977785
Change-Id: I88a36278b0f517519286d53f49c2de4486f41e00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2279978
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: Marian Fechete <marianfe@google.com>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786237}

chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantChromeTabIntegrationTest.java

@@ -28,6 +28,7 @@ import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUi
 import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntilKeyboardMatchesCondition;
 import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntilViewAssertionTrue;
 import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntilViewMatchesCondition;
+import static org.chromium.content_public.browser.test.util.CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL;
 
 import android.support.test.InstrumentationRegistry;
 
@@ -143,7 +144,7 @@ public class AutofillAssistantChromeTabIntegrationTest {
                 .check(matches(withEffectiveVisibility(Visibility.VISIBLE)));
 
         onView(withId(org.chromium.chrome.R.id.tab_switcher_button)).perform(click());
-        waitUntilViewAssertionTrue(withText("Prompt"), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(withText("Prompt"), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
         onView(withClassName(is(ScrimView.class.getName()))).check(doesNotExist());
 
         Espresso.pressBack();
@@ -179,7 +180,7 @@ public class AutofillAssistantChromeTabIntegrationTest {
 
         ChromeTabUtils.fullyLoadUrlInNewTab(InstrumentationRegistry.getInstrumentation(),
                 mTestRule.getActivity(), getURL(TEST_PAGE_B), false);
-        waitUntilViewAssertionTrue(withText("Prompt"), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(withText("Prompt"), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
 
         ChromeTabUtils.switchTabInCurrentTabModel(mTestRule.getActivity(),
                 TabModelUtils.getTabIndexById(
@@ -210,7 +211,7 @@ public class AutofillAssistantChromeTabIntegrationTest {
 
         ChromeTabUtils.fullyLoadUrlInNewTab(InstrumentationRegistry.getInstrumentation(),
                 mTestRule.getActivity(), getURL(TEST_PAGE_B), false);
-        waitUntilViewAssertionTrue(withText("Prompt"), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(withText("Prompt"), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
 
         ChromeTabUtils.closeCurrentTab(
                 InstrumentationRegistry.getInstrumentation(), mTestRule.getActivity());
@@ -260,7 +261,7 @@ public class AutofillAssistantChromeTabIntegrationTest {
 
         ChromeTabUtils.fullyLoadUrlInNewTab(InstrumentationRegistry.getInstrumentation(),
                 mTestRule.getActivity(), getURL(TEST_PAGE_B), false);
-        waitUntilViewAssertionTrue(withText("Prompt A"), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(withText("Prompt A"), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
 
         startAutofillAssistantOnTab(TEST_PAGE_B);
         waitUntilViewMatchesCondition(withText("Prompt B"), isCompletelyDisplayed());
@@ -268,7 +269,7 @@ public class AutofillAssistantChromeTabIntegrationTest {
         ChromeTabUtils.switchTabInCurrentTabModel(mTestRule.getActivity(),
                 TabModelUtils.getTabIndexById(
                         mTestRule.getActivity().getCurrentTabModel(), initialTabId));
-        waitUntilViewAssertionTrue(withText("Prompt B"), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(withText("Prompt B"), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
         waitUntilViewMatchesCondition(withText("Prompt A"), isCompletelyDisplayed());
     }
 
@@ -311,14 +312,14 @@ public class AutofillAssistantChromeTabIntegrationTest {
 
         ChromeTabUtils.fullyLoadUrlInNewTab(InstrumentationRegistry.getInstrumentation(),
                 mTestRule.getActivity(), getURL(TEST_PAGE_B), false);
-        waitUntilViewAssertionTrue(withText("Prompt A"), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(withText("Prompt A"), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
 
         startAutofillAssistantOnTab(TEST_PAGE_B);
         waitUntilViewMatchesCondition(withText("Prompt B"), isCompletelyDisplayed());
 
         ChromeTabUtils.closeCurrentTab(
                 InstrumentationRegistry.getInstrumentation(), mTestRule.getActivity());
-        waitUntilViewAssertionTrue(withText("Prompt B"), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(withText("Prompt B"), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
         waitUntilViewMatchesCondition(withText("Prompt A"), isCompletelyDisplayed());
     }
 
@@ -410,7 +411,8 @@ public class AutofillAssistantChromeTabIntegrationTest {
 
         // First press on back button fully destroys Autofill Assistant UI, without Undo.
         Espresso.pressBack();
-        waitUntilViewAssertionTrue(withId(R.id.autofill_assistant), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(
+                withId(R.id.autofill_assistant), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
         onView(withText("Shutdown")).check(doesNotExist());
         onView(withText(R.string.undo)).check(doesNotExist());
         assertThat(mTestRule.getActivity().getActivityTab().getUrl().getSpec(),
@@ -489,7 +491,7 @@ public class AutofillAssistantChromeTabIntegrationTest {
 
         // Clicking location bar hides UI and shows the keyboard.
         onView(withId(org.chromium.chrome.R.id.url_bar)).perform(click());
-        waitUntilViewAssertionTrue(withText("Prompt"), doesNotExist(), 3000L);
+        waitUntilViewAssertionTrue(withText("Prompt"), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
 
         ChromeTabbedActivity activity = mTestRule.getActivity();
         assertThat(activity.getWindowAndroid().getKeyboardDelegate().isKeyboardShowing(
@@ -577,4 +579,32 @@ public class AutofillAssistantChromeTabIntegrationTest {
                           -> mTestRule.getActivity().getActivityTab().getUrl().getSpec().equals(
                                   getURL("form_target_website.html")));
     }
+
+    @Test
+    @MediumTest
+    public void navigatingInStoppedAutofillAssistantState() {
+        ArrayList<ActionProto> list = new ArrayList<>();
+        list.add((ActionProto) ActionProto.newBuilder()
+                         .setTell(TellProto.newBuilder().setMessage("Shutdown"))
+                         .build());
+        list.add((ActionProto) ActionProto.newBuilder().setStop(StopProto.newBuilder()).build());
+
+        AutofillAssistantTestScript script = new AutofillAssistantTestScript(
+                (SupportedScriptProto) SupportedScriptProto.newBuilder()
+                        .setPath(TEST_PAGE_A)
+                        .setPresentation(PresentationProto.newBuilder().setAutostart(true).setChip(
+                                ChipProto.newBuilder().setText("Done")))
+                        .build(),
+                list);
+        setupScripts(script);
+        startAutofillAssistantOnTab(TEST_PAGE_A);
+
+        waitUntilViewMatchesCondition(withText("Shutdown"), isCompletelyDisplayed());
+
+        onView(withId(org.chromium.chrome.R.id.url_bar))
+                .perform(click(), typeText(getURL(TEST_PAGE_B)));
+        onView(withId(org.chromium.chrome.R.id.url_bar)).perform(pressImeActionButton());
+        waitUntilViewAssertionTrue(
+                withId(R.id.autofill_assistant), doesNotExist(), DEFAULT_MAX_TIME_TO_POLL);
+    }
 }

chrome/browser/android/autofill_assistant/ui_controller_android.cc

@@ -54,6 +54,7 @@
 #include "components/version_info/channel.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "google_apis/google_api_keys.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -314,6 +315,9 @@ void UiControllerAndroid::Attach(content::WebContents* web_contents,
 
   client_ = client;
 
+  // Remove the self destruction.
+  self_destruct_observer_.reset();
+
   // Detach from the current ui_delegate, if one was set previously.
   if (ui_delegate_)
     ui_delegate_->RemoveObserver(this);
@@ -653,6 +657,7 @@ std::string UiControllerAndroid::GetDebugContext() {
 }
 
 void UiControllerAndroid::DestroySelf() {
+  self_destruct_observer_.reset();
   client_->DestroyUI();
 }
 
@@ -960,10 +965,36 @@ void UiControllerAndroid::CloseCustomTab() {
       AttachCurrentThread(), java_object_);
 }
 
+UiControllerAndroid::SelfDestructObserver::SelfDestructObserver(
+    content::WebContents* web_contents,
+    UiControllerAndroid* ui_controller,
+    int64_t navigation_id_to_ignore)
+    : content::WebContentsObserver(web_contents),
+      ui_controller_(ui_controller),
+      navigation_id_to_ignore_(navigation_id_to_ignore) {}
+
+UiControllerAndroid::SelfDestructObserver::~SelfDestructObserver() {}
+
+void UiControllerAndroid::SelfDestructObserver::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame() ||
+      navigation_handle->IsRendererInitiated() ||
+      navigation_handle->GetNavigationId() == navigation_id_to_ignore_) {
+    return;
+  }
+  ui_controller_->DestroySelf();
+}
+
 void UiControllerAndroid::Detach() {
   if (!ui_delegate_)
     return;
 
+  auto* web_contents = client_->GetWebContents();
+  if (web_contents != nullptr) {
+    self_destruct_observer_ = std::make_unique<SelfDestructObserver>(
+        web_contents, this, ui_delegate_->GetErrorCausingNavigationId());
+  }
+
   ResetGenericUiControllers();
 
   // Capture the debug context, for including into a feedback possibly sent

chrome/browser/android/autofill_assistant/ui_controller_android.h

@@ -26,6 +26,8 @@
 #include "components/autofill_assistant/browser/overlay_state.h"
 #include "components/autofill_assistant/browser/trigger_context.h"
 #include "components/autofill_assistant/browser/user_action.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/web_contents_observer.h"
 
 namespace autofill_assistant {
 struct ClientSettings;
@@ -202,6 +204,21 @@ class UiControllerAndroid : public ControllerObserver {
                   jboolean visible);
 
  private:
+  class SelfDestructObserver : private content::WebContentsObserver {
+   public:
+    SelfDestructObserver(content::WebContents* web_contents,
+                         UiControllerAndroid* ui_controller,
+                         int64_t navigation_id_to_ignore);
+    ~SelfDestructObserver() override;
+
+    void DidStartNavigation(
+        content::NavigationHandle* navigation_handle) override;
+
+   private:
+    UiControllerAndroid* ui_controller_;
+    int64_t navigation_id_to_ignore_;
+  };
+
   // A pointer to the client. nullptr until Attach() is called.
   Client* client_ = nullptr;
 
@@ -281,6 +298,9 @@ class UiControllerAndroid : public ControllerObserver {
 
   OverlayState desired_overlay_state_ = OverlayState::FULL;
   OverlayState overlay_state_ = OverlayState::FULL;
+
+  std::unique_ptr<SelfDestructObserver> self_destruct_observer_;
+
   base::WeakPtrFactory<UiControllerAndroid> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(UiControllerAndroid);

components/autofill_assistant/browser/controller.cc

@@ -1121,6 +1121,10 @@ AutofillAssistantState Controller::GetState() {
   return state_;
 }
 
+int64_t Controller::GetErrorCausingNavigationId() const {
+  return error_causing_navigation_id_;
+}
+
 bool Controller::ShouldShowOverlay() const {
   return overlay_behavior_ == ConfigureUiStateProto::DEFAULT;
 }
@@ -1703,6 +1707,7 @@ void Controller::DidStartNavigation(
       web_contents()->GetLastCommittedURL().is_valid() &&
       !navigation_handle->WasServerRedirect() &&
       !navigation_handle->IsRendererInitiated()) {
+    error_causing_navigation_id_ = navigation_handle->GetNavigationId();
     OnScriptError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP),
                   Metrics::DropOutReason::NAVIGATION);
     return;
@@ -1714,6 +1719,7 @@ void Controller::DidStartNavigation(
     // user initiated navigation will cause an error.
     if (state_ == AutofillAssistantState::RUNNING &&
         !navigation_handle->IsRendererInitiated()) {
+      error_causing_navigation_id_ = navigation_handle->GetNavigationId();
       OnScriptError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP),
                     Metrics::DropOutReason::NAVIGATION_WHILE_RUNNING);
       return;

components/autofill_assistant/browser/controller.h

@@ -164,6 +164,7 @@ class Controller : public ScriptExecutorDelegate,
 
   // Overrides autofill_assistant::UiDelegate:
   AutofillAssistantState GetState() override;
+  int64_t GetErrorCausingNavigationId() const override;
   void OnUserInteractionInsideTouchableArea() override;
   const Details* GetDetails() const override;
   const InfoBox* GetInfoBox() const override;
@@ -337,6 +338,7 @@ class Controller : public ScriptExecutorDelegate,
   std::unique_ptr<TriggerContext> trigger_context_;
 
   AutofillAssistantState state_ = AutofillAssistantState::INACTIVE;
+  int64_t error_causing_navigation_id_ = -1;
 
   // The URL passed to Start(). Used only as long as there's no committed URL.
   // Note that this is the deeplink passed by a caller.

components/autofill_assistant/browser/ui_delegate.h

@@ -42,6 +42,9 @@ class UiDelegate {
   // Returns the current state of the controller.
   virtual AutofillAssistantState GetState() = 0;
 
+  // Returns the last navigation id that caused an error.
+  virtual int64_t GetErrorCausingNavigationId() const = 0;
+
   // Called when user interaction within the allowed touchable area was
   // detected. This should cause rerun of preconditions check.
   virtual void OnUserInteractionInsideTouchableArea() = 0;