[Autofill Assistant] Improve overlay behavior in browse mode

Currently, the overlay is depending on the current state. In the case
of browse mode, the state is set to BROWSE by the prompt action and
reset to RUNNING at the end of it. This causes the overlay to flicker
between prompt actions when multiple are present.

To prevent this we introduce a new action to manually configure the
overlay state from the backend. When the the new state is set to HIDDEN
the overlay will never be shown. This overrides the current logic that
decides the overlay state based on the current AutofillAssistantState.


Bug: b/157457059, b/156232664
Change-Id: I38a720ec6a4a6d73ef4bf31f3f5286188942464e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2252045Reviewed-by: Marian Fechete <marianfe@google.com>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Commit-Queue: Luca Hunkeler <hluca@google.com>
Cr-Commit-Position: refs/heads/master@{#784380}

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

@@ -4,6 +4,8 @@
 
 package org.chromium.chrome.browser.autofill_assistant;
 
+import static androidx.test.espresso.Espresso.onView;
+import static androidx.test.espresso.action.ViewActions.click;
 import static androidx.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
 import static androidx.test.espresso.matcher.ViewMatchers.withText;
 
@@ -29,10 +31,13 @@ import org.junit.runner.RunWith;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.chrome.browser.autofill_assistant.proto.ActionProto;
 import org.chromium.chrome.browser.autofill_assistant.proto.ChipProto;
+import org.chromium.chrome.browser.autofill_assistant.proto.ConfigureUiStateProto;
+import org.chromium.chrome.browser.autofill_assistant.proto.ConfigureUiStateProto.OverlayBehavior;
 import org.chromium.chrome.browser.autofill_assistant.proto.ElementAreaProto;
 import org.chromium.chrome.browser.autofill_assistant.proto.ElementAreaProto.Rectangle;
 import org.chromium.chrome.browser.autofill_assistant.proto.FocusElementProto;
 import org.chromium.chrome.browser.autofill_assistant.proto.PromptProto;
+import org.chromium.chrome.browser.autofill_assistant.proto.PromptProto.Choice;
 import org.chromium.chrome.browser.autofill_assistant.proto.SelectorProto;
 import org.chromium.chrome.browser.autofill_assistant.proto.SupportedScriptProto;
 import org.chromium.chrome.browser.autofill_assistant.proto.SupportedScriptProto.PresentationProto;
@@ -279,6 +284,85 @@ public class AutofillAssistantOverlayIntegrationTest {
                 checkElementExists(mTestRule.getWebContents(), "iframe", "touch_area_4"), is(true));
     }
 
+    /**
+     * Tests that changing the OverlayBehavior setting affects the overlay as intended.
+     */
+    @Test
+    @MediumTest
+    public void testOverlayBehaviorSetting() throws Exception {
+        SelectorProto element =
+                (SelectorProto) SelectorProto.newBuilder()
+                        .addFilters(
+                                SelectorProto.Filter.newBuilder().setCssSelector("#touch_area_one"))
+                        .build();
+
+        ArrayList<ActionProto> list = new ArrayList<>();
+        list.add((ActionProto) ActionProto.newBuilder()
+                         .setPrompt(
+                                 PromptProto.newBuilder()
+                                         .setMessage("Overlay present")
+                                         .addChoices(Choice.newBuilder().setChip(
+                                                 ChipProto.newBuilder()
+                                                         .setType(org.chromium.chrome.browser
+                                                                          .autofill_assistant.proto
+                                                                          .ChipType.DONE_ACTION)
+                                                         .setText("Hide"))))
+                         .build());
+        list.add((ActionProto) ActionProto.newBuilder()
+                         .setConfigureUiState(ConfigureUiStateProto.newBuilder().setOverlayBehavior(
+                                 OverlayBehavior.HIDDEN))
+                         .build());
+        list.add((ActionProto) ActionProto.newBuilder()
+                         .setPrompt(
+                                 PromptProto.newBuilder()
+                                         .setMessage("Overlay hidden")
+                                         .addChoices(Choice.newBuilder().setChip(
+                                                 ChipProto.newBuilder()
+                                                         .setType(org.chromium.chrome.browser
+                                                                          .autofill_assistant.proto
+                                                                          .ChipType.DONE_ACTION)
+                                                         .setText("Default"))))
+                         .build());
+        list.add((ActionProto) ActionProto.newBuilder()
+                         .setConfigureUiState(ConfigureUiStateProto.newBuilder().setOverlayBehavior(
+                                 OverlayBehavior.DEFAULT))
+                         .build());
+        list.add((ActionProto) ActionProto.newBuilder()
+                         .setPrompt(PromptProto.newBuilder()
+                                            .setMessage("Overlay present")
+                                            .addChoices(PromptProto.Choice.newBuilder()))
+                         .build());
+
+        AutofillAssistantTestScript script = new AutofillAssistantTestScript(
+                (SupportedScriptProto) SupportedScriptProto.newBuilder()
+                        .setPath("autofill_assistant_target_website.html")
+                        .setPresentation(PresentationProto.newBuilder().setAutostart(true).setChip(
+                                ChipProto.newBuilder().setText("Done")))
+                        .build(),
+                list);
+        runScript(script);
+
+        waitUntil(() -> checkElementOnScreen(mTestRule, "touch_area_one"));
+        waitUntilViewMatchesCondition(withText("Overlay present"), isCompletelyDisplayed());
+
+        // Tapping the element should not do anything  since the overlay should be present.
+        assertThat(checkElementExists(mTestRule.getWebContents(), "touch_area_one"), is(true));
+        tapElement(mTestRule, "touch_area_one");
+
+        // Go to the next action to hide the overlay.
+        onView(withText("Hide")).perform(click());
+        // Tapping the item should now hide it.
+        assertThat(checkElementExists(mTestRule.getWebContents(), "touch_area_one"), is(true));
+        tapElement(mTestRule, "touch_area_one");
+        waitUntil(() -> !checkElementExists(mTestRule.getWebContents(), "touch_area_one"));
+
+        // Go to the next action to set the overlay to full again.
+        onView(withText("Default")).perform(click());
+        // Tapping on the element should be blocked by the overlay.
+        tapElement(mTestRule, "touch_area_four");
+        assertThat(checkElementExists(mTestRule.getWebContents(), "touch_area_four"), is(true));
+    }
+
     private void runScript(AutofillAssistantTestScript script) {
         AutofillAssistantTestService testService =
                 new AutofillAssistantTestService(Collections.singletonList(script));

chrome/browser/android/autofill_assistant/ui_controller_android.cc

@@ -821,7 +821,8 @@ bool UiControllerAndroid::OnBackButtonClicked(
 
   // For BROWSE state the back button should react in its default way.
   if (ui_delegate_ != nullptr &&
-      ui_delegate_->GetState() == AutofillAssistantState::BROWSE) {
+      (ui_delegate_->GetState() == AutofillAssistantState::BROWSE ||
+       !ui_delegate_->ShouldShowOverlay())) {
     return false;
   }
 
@@ -890,13 +891,28 @@ void UiControllerAndroid::SetOverlayState(OverlayState state) {
       state = OverlayState::FULL;
     }
   }
+  overlay_state_ = state;
 
+  if (ui_delegate_->ShouldShowOverlay()) {
+    ApplyOverlayState(state);
+  }
+}
+
+void UiControllerAndroid::ApplyOverlayState(OverlayState state) {
   Java_AssistantOverlayModel_setState(AttachCurrentThread(), GetOverlayModel(),
                                       state);
   Java_AssistantModel_setAllowTalkbackOnWebsite(
       AttachCurrentThread(), GetModel(), state != OverlayState::FULL);
 }
 
+void UiControllerAndroid::OnShouldShowOverlayChanged(bool should_show) {
+  if (should_show) {
+    ApplyOverlayState(overlay_state_);
+  } else {
+    ApplyOverlayState(OverlayState::HIDDEN);
+  }
+}
+
 void UiControllerAndroid::OnTouchableAreaChanged(
     const RectF& visual_viewport,
     const std::vector<RectF>& touchable_areas,

chrome/browser/android/autofill_assistant/ui_controller_android.h

@@ -122,6 +122,7 @@ class UiControllerAndroid : public ControllerObserver {
   void OnClientSettingsChanged(const ClientSettings& settings) override;
   void OnGenericUserInterfaceChanged(
       const GenericUserInterfaceProto* generic_ui) override;
+  void OnShouldShowOverlayChanged(bool should_show) override;
 
   // Called by AssistantOverlayDelegate:
   void OnUnexpectedTaps();
@@ -224,7 +225,13 @@ class UiControllerAndroid : public ControllerObserver {
   base::android::ScopedJavaLocalRef<jobject> GetFormModel();
   base::android::ScopedJavaLocalRef<jobject> GetGenericUiModel();
 
+  // The UiDelegate has the last say on whether we should show the overlay.
+  // This saves the AutofillAssistantState-determined OverlayState and then
+  // applies it the actual UI only if the UiDelegate's ShouldShowOverlay is
+  // true.
   void SetOverlayState(OverlayState state);
+  // Applies the specified OverlayState to the UI.
+  void ApplyOverlayState(OverlayState state);
   void AllowShowingSoftKeyboard(bool enabled);
   void ShowContentAndExpandBottomSheet();
   void SetSpinPoodle(bool enabled);
@@ -273,6 +280,7 @@ class UiControllerAndroid : public ControllerObserver {
   std::unique_ptr<GenericUiControllerAndroid> generic_ui_controller_;
 
   OverlayState desired_overlay_state_ = OverlayState::FULL;
+  OverlayState overlay_state_ = OverlayState::FULL;
   base::WeakPtrFactory<UiControllerAndroid> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(UiControllerAndroid);

components/autofill_assistant/browser/BUILD.gn

@@ -30,6 +30,8 @@ jumbo_static_library("browser") {
     "actions/collect_user_data_action.h",
     "actions/configure_bottom_sheet_action.cc",
     "actions/configure_bottom_sheet_action.h",
+    "actions/configure_ui_state_action.cc",
+    "actions/configure_ui_state_action.h",
     "actions/expect_navigation_action.cc",
     "actions/expect_navigation_action.h",
     "actions/fallback_handler/required_field.cc",

components/autofill_assistant/browser/actions/action.cc

@@ -134,6 +134,9 @@ std::ostream& operator<<(std::ostream& out,
     case ActionProto::kSaveGeneratedPassword:
       out << "SaveGeneratedPassword";
       break;
+    case ActionProto::ActionInfoCase::kConfigureUiState:
+      out << "ConfigureUiState";
+      break;
     case ActionProto::ActionInfoCase::ACTION_INFO_NOT_SET:
       out << "ACTION_INFO_NOT_SET";
       break;

components/autofill_assistant/browser/actions/action_delegate.h

@@ -14,6 +14,7 @@
 #include "components/autofill_assistant/browser/details.h"
 #include "components/autofill_assistant/browser/info_box.h"
 #include "components/autofill_assistant/browser/selector.h"
+#include "components/autofill_assistant/browser/state.h"
 #include "components/autofill_assistant/browser/top_padding.h"
 #include "components/autofill_assistant/browser/user_data.h"
 #include "components/autofill_assistant/browser/viewport_mode.h"
@@ -387,6 +388,9 @@ class ActionDelegate {
   // |user_model| will persist and will not be affected by this call.
   virtual void ClearGenericUi() = 0;
 
+  // Sets the OverlayBehavior.
+  virtual void SetOverlayBehavior(ConfigureUiStateProto::OverlayBehavior) = 0;
+
  protected:
   ActionDelegate() = default;
 };

components/autofill_assistant/browser/actions/configure_ui_state_action.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/autofill_assistant/browser/actions/configure_ui_state_action.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "components/autofill_assistant/browser/actions/action_delegate.h"
+#include "components/autofill_assistant/browser/client_status.h"
+#include "components/autofill_assistant/browser/viewport_mode.h"
+
+namespace autofill_assistant {
+
+ConfigureUiStateAction::ConfigureUiStateAction(ActionDelegate* delegate,
+                                               const ActionProto& proto)
+    : Action(delegate, proto) {}
+
+ConfigureUiStateAction::~ConfigureUiStateAction() {}
+
+void ConfigureUiStateAction::InternalProcessAction(
+    ProcessActionCallback callback) {
+  const ConfigureUiStateProto& proto = proto_.configure_ui_state();
+
+  if (proto.has_overlay_behavior()) {
+    delegate_->SetOverlayBehavior(proto.overlay_behavior());
+  }
+
+  UpdateProcessedAction(OkClientStatus());
+  std::move(callback).Run(std::move(processed_action_proto_));
+}
+
+}  // namespace autofill_assistant

components/autofill_assistant/browser/actions/configure_ui_state_action.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_CONFIGURE_UI_STATE_ACTION_H_
+#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_CONFIGURE_UI_STATE_ACTION_H_
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "components/autofill_assistant/browser/actions/action.h"
+
+namespace autofill_assistant {
+
+// Action to configure the ui.
+class ConfigureUiStateAction : public Action {
+ public:
+  explicit ConfigureUiStateAction(ActionDelegate* delegate,
+                                  const ActionProto& proto);
+  ~ConfigureUiStateAction() override;
+
+ private:
+  // Overrides Action:
+  void InternalProcessAction(ProcessActionCallback callback) override;
+
+  base::WeakPtrFactory<ConfigureUiStateAction> weak_ptr_factory_{this};
+  DISALLOW_COPY_AND_ASSIGN(ConfigureUiStateAction);
+};
+
+}  // namespace autofill_assistant
+#endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_CONFIGURE_UI_STATE_ACTION_H_

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -306,6 +306,8 @@ class MockActionDelegate : public ActionDelegate {
                    view_inflation_finished_callback);
   }
   MOCK_METHOD0(ClearGenericUi, void());
+  MOCK_METHOD1(SetOverlayBehavior,
+               void(ConfigureUiStateProto::OverlayBehavior));
 
   const ClientSettings& GetSettings() override { return client_settings_; }
 

components/autofill_assistant/browser/controller.cc

@@ -695,6 +695,14 @@ bool Controller::EnterState(AutofillAssistantState state) {
   return true;
 }
 
+void Controller::SetOverlayBehavior(
+    ConfigureUiStateProto::OverlayBehavior overlay_behavior) {
+  overlay_behavior_ = overlay_behavior;
+  for (ControllerObserver& observer : observers_) {
+    observer.OnShouldShowOverlayChanged(ShouldShowOverlay());
+  }
+}
+
 void Controller::SetWebControllerForTest(
     std::unique_ptr<WebController> web_controller) {
   web_controller_ = std::move(web_controller);
@@ -1113,6 +1121,10 @@ AutofillAssistantState Controller::GetState() {
   return state_;
 }
 
+bool Controller::ShouldShowOverlay() const {
+  return overlay_behavior_ == ConfigureUiStateProto::DEFAULT;
+}
+
 void Controller::OnScriptSelected(const ScriptHandle& handle,
                                   std::unique_ptr<TriggerContext> context) {
   ExecuteScript(handle.path, handle.start_message, handle.needs_ui,

components/autofill_assistant/browser/controller.h

@@ -154,6 +154,8 @@ class Controller : public ScriptExecutorDelegate,
   void SetBrowseDomainsWhitelist(std::vector<std::string> domains) override;
 
   bool EnterState(AutofillAssistantState state) override;
+  void SetOverlayBehavior(
+      ConfigureUiStateProto::OverlayBehavior overlay_behavior) override;
   void SetCollectUserDataOptions(CollectUserDataOptions* options) override;
   void WriteUserData(
       base::OnceCallback<void(UserData*, UserData::FieldChange*)>) override;
@@ -224,6 +226,7 @@ class Controller : public ScriptExecutorDelegate,
   bool ShouldPromptActionExpandSheet() const override;
   BasicInteractions* GetBasicInteractions() override;
   const GenericUserInterfaceProto* GetGenericUiProto() const override;
+  bool ShouldShowOverlay() const override;
 
  private:
   friend ControllerTest;
@@ -450,6 +453,10 @@ class Controller : public ScriptExecutorDelegate,
   // result - and updated the state and set of available actions.
   bool has_run_first_check_ = false;
 
+  // Whether the overlay should be set according to state or always hidden.
+  ConfigureUiStateProto::OverlayBehavior overlay_behavior_ =
+      ConfigureUiStateProto::DEFAULT;
+
   // Callbacks to call when |has_run_first_check_| becomes true.
   std::vector<base::OnceCallback<void()>> on_has_run_first_check_;
 

components/autofill_assistant/browser/controller_observer.h

@@ -128,6 +128,9 @@ class ControllerObserver : public base::CheckedObserver {
   // Called when the generic user interface to show has been changed or cleared.
   virtual void OnGenericUserInterfaceChanged(
       const GenericUserInterfaceProto* generic_ui) = 0;
+
+  // Called when the desired overlay behavior has changed.
+  virtual void OnShouldShowOverlayChanged(bool should_show) = 0;
 };
 }  // namespace autofill_assistant
 #endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_CONTROLLER_OBSERVER_H_

components/autofill_assistant/browser/fake_script_executor_delegate.cc

@@ -210,4 +210,7 @@ void FakeScriptExecutorDelegate::SetGenericUi(
 
 void FakeScriptExecutorDelegate::ClearGenericUi() {}
 
+void FakeScriptExecutorDelegate::SetOverlayBehavior(
+    ConfigureUiStateProto::OverlayBehavior overaly_behavior) {}
+
 }  // namespace autofill_assistant

components/autofill_assistant/browser/fake_script_executor_delegate.h

@@ -82,6 +82,8 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate {
       base::OnceCallback<void(const ClientStatus&)>
           view_inflation_finished_callback) override;
   void ClearGenericUi() override;
+  void SetOverlayBehavior(
+      ConfigureUiStateProto::OverlayBehavior overlay_behavior) override;
 
   ClientSettings* GetMutableSettings() { return &client_settings_; }
 

components/autofill_assistant/browser/mock_controller_observer.h

@@ -61,6 +61,7 @@ class MockControllerObserver : public ControllerObserver {
   MOCK_METHOD1(OnClientSettingsChanged, void(const ClientSettings& settings));
   MOCK_METHOD1(OnGenericUserInterfaceChanged,
                void(const GenericUserInterfaceProto* generic_ui));
+  MOCK_METHOD1(OnShouldShowOverlayChanged, void(bool should_show));
 };
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/protocol_utils.cc

@@ -11,6 +11,7 @@
 #include "components/autofill_assistant/browser/actions/click_action.h"
 #include "components/autofill_assistant/browser/actions/collect_user_data_action.h"
 #include "components/autofill_assistant/browser/actions/configure_bottom_sheet_action.h"
+#include "components/autofill_assistant/browser/actions/configure_ui_state_action.h"
 #include "components/autofill_assistant/browser/actions/expect_navigation_action.h"
 #include "components/autofill_assistant/browser/actions/focus_element_action.h"
 #include "components/autofill_assistant/browser/actions/generate_password_for_form_field_action.h"
@@ -335,6 +336,11 @@ bool ProtocolUtils::ParseActions(ActionDelegate* delegate,
             std::make_unique<SaveGeneratedPasswordAction>(delegate, action);
         break;
       }
+      case ActionProto::ActionInfoCase::kConfigureUiState: {
+        client_action =
+            std::make_unique<ConfigureUiStateAction>(delegate, action);
+        break;
+      }
       case ActionProto::ActionInfoCase::ACTION_INFO_NOT_SET: {
         VLOG(1) << "Encountered action with ACTION_INFO_NOT_SET";
         client_action = std::make_unique<UnsupportedAction>(delegate, action);

components/autofill_assistant/browser/script_executor.cc

@@ -651,6 +651,11 @@ void ScriptExecutor::ClearGenericUi() {
   delegate_->ClearGenericUi();
 }
 
+void ScriptExecutor::SetOverlayBehavior(
+    ConfigureUiStateProto::OverlayBehavior overlay_behavior) {
+  delegate_->SetOverlayBehavior(overlay_behavior);
+}
+
 void ScriptExecutor::OnGetActions(bool result, const std::string& response) {
   bool success = result && ProcessNextActionResponse(response);
   VLOG(2) << __func__ << " result=" << result;

components/autofill_assistant/browser/script_executor.h

@@ -239,6 +239,7 @@ class ScriptExecutor : public ActionDelegate,
       base::OnceCallback<void(const ClientStatus&)>
           view_inflation_finished_callback) override;
   void ClearGenericUi() override;
+  void SetOverlayBehavior(ConfigureUiStateProto::OverlayBehavior) override;
 
  private:
   // Helper for WaitForElementVisible that keeps track of the state required to

components/autofill_assistant/browser/script_executor_delegate.h

@@ -63,6 +63,9 @@ class ScriptExecutorDelegate {
   // Enters the given state. Returns true if the state was changed.
   virtual bool EnterState(AutofillAssistantState state) = 0;
 
+  virtual void SetOverlayBehavior(
+      ConfigureUiStateProto::OverlayBehavior overlay_behavior) = 0;
+
   // Make the area of the screen that correspond to the given elements
   // touchable.
   virtual void SetTouchableElementArea(const ElementAreaProto& element) = 0;

components/autofill_assistant/browser/service.proto

@@ -473,6 +473,7 @@ message ActionProto {
     ShowGenericUiProto show_generic_ui = 49;
     GeneratePasswordForFormFieldProto generate_password_for_form_field = 52;
     SaveGeneratedPasswordProto save_generated_password = 53;
+    ConfigureUiStateProto configure_ui_state = 54;
   }
 
   // Set to true to make the client remove any contextual information if the
@@ -1865,6 +1866,21 @@ message SaveGeneratedPasswordProto {
   optional string memory_key = 1;
 }
 
+// Configures the UI of the autofill assistant client.
+message ConfigureUiStateProto {
+  enum OverlayBehavior {
+    // The overlay is decided according to the state the client is in.
+    DEFAULT = 0;
+    // The overlay is always hidden.
+    // At the moment this also changes the behavior of the back button and it
+    // should only be used for browse mode.
+    // TODO(HLUCA): add a separate setting in this action to configure the back
+    // button behavior.
+    HIDDEN = 1;
+  }
+  optional OverlayBehavior overlay_behavior = 1;
+}
+
 // Set an element attribute to a specific value.
 message SetAttributeProto {
   // A reference to the form element whose attribute should be set.

components/autofill_assistant/browser/state.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_STATE_H_
 
 #include <ostream>
+#include "components/autofill_assistant/browser/service.pb.h"
 
 namespace autofill_assistant {
 

components/autofill_assistant/browser/ui_delegate.h

@@ -233,6 +233,8 @@ class UiDelegate {
   // The generic user interface to show, if any.
   virtual const GenericUserInterfaceProto* GetGenericUiProto() const = 0;
 
+  virtual bool ShouldShowOverlay() const = 0;
+
  protected:
   UiDelegate() = default;
 };