[Autofill Assistant] Append feedback button on error

When the script enters an error, we automatically append
a "Send Feedback" button.

Bug: b/173401004
Change-Id: I62b47a0fb2f332cf74d4aaa1f287d1452c588d42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575034Reviewed-by: Colin Blundell <blundell@chromium.org>
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@{#846040}

chrome/browser/android/autofill_assistant/ui_controller_android.cc

@@ -538,6 +538,10 @@ void UiControllerAndroid::OnFeedbackButtonClicked() {
       ConvertUTF8ToJavaString(env, ui_delegate_->GetDebugContext()));
 }
 
+void UiControllerAndroid::OnFeedbackFormRequested() {
+  OnFeedbackButtonClicked();
+}
+
 void UiControllerAndroid::OnViewEvent(const EventHandler::EventKey& key) {
   ui_delegate_->DispatchEvent(key);
 }
@@ -1858,6 +1862,7 @@ void UiControllerAndroid::OnFatalError(
     return;
   ui_delegate_->OnFatalError(
       base::android::ConvertJavaStringToUTF8(env, jmessage),
+      /*show_feedback_chip=*/false,
       static_cast<Metrics::DropOutReason>(jreason));
 }
 

chrome/browser/android/autofill_assistant/ui_controller_android.h

@@ -122,6 +122,7 @@ class UiControllerAndroid : public ControllerObserver {
   void OnGenericUserInterfaceChanged(
       const GenericUserInterfaceProto* generic_ui) override;
   void OnShouldShowOverlayChanged(bool should_show) override;
+  void OnFeedbackFormRequested() override;
 
   // Called by AssistantOverlayDelegate:
   void OnUnexpectedTaps();

chrome/browser/flags/android/chrome_feature_list.cc

@@ -82,6 +82,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &autofill_assistant::features::kAutofillAssistantChromeEntry,
     &autofill_assistant::features::kAutofillAssistantDirectActions,
     &autofill_assistant::features::kAutofillAssistantDisableOnboardingFlow,
+    &autofill_assistant::features::kAutofillAssistantFeedbackChip,
     &autofill_assistant::features::kAutofillAssistantLoadDFMForTriggerScripts,
     &autofill_assistant::features::kAutofillAssistantProactiveHelp,
     &autofill_assistant::features::

chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java

@@ -224,6 +224,7 @@ public abstract class ChromeFeatureList {
     public static final String AUTOFILL_ASSISTANT_DIRECT_ACTIONS = "AutofillAssistantDirectActions";
     public static final String AUTOFILL_ASSISTANT_DISABLE_ONBOARDING_FLOW =
             "AutofillAssistantDisableOnboardingFlow";
+    public static final String AUTOFILL_ASSISTANT_FEEDBACK_CHIP = "AutofillAssistantFeedbackChip";
     public static final String AUTOFILL_ASSISTANT_LOAD_DFM_FOR_TRIGGER_SCRIPTS =
             "AutofillAssistantLoadDFMForTriggerScripts";
     public static final String AUTOFILL_ASSISTANT_PROACTIVE_HELP = "AutofillAssistantProactiveHelp";

components/autofill_assistant/browser/controller.cc

@@ -813,13 +813,28 @@ void Controller::ReportNavigationStateChanged() {
   }
 }
 
-void Controller::EnterStoppedState() {
+void Controller::EnterStoppedState(bool show_feedback_chip) {
   if (script_tracker_)
     script_tracker_->StopScript();
 
+  std::unique_ptr<std::vector<UserAction>> final_actions;
+  if (base::FeatureList::IsEnabled(features::kAutofillAssistantFeedbackChip) &&
+      show_feedback_chip) {
+    final_actions = std::make_unique<std::vector<UserAction>>();
+    UserAction feedback_action;
+    Chip feedback_chip;
+    feedback_chip.type = FEEDBACK_ACTION;
+    feedback_chip.text =
+        l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_SEND_FEEDBACK);
+    feedback_action.SetCallback(base::BindOnce(&Controller::ShutdownIfNecessary,
+                                               weak_ptr_factory_.GetWeakPtr()));
+    feedback_action.chip() = feedback_chip;
+    final_actions->emplace_back(std::move(feedback_action));
+  }
+
   ClearInfoBox();
   SetDetails(nullptr, base::TimeDelta());
-  SetUserActions(nullptr);
+  SetUserActions(std::move(final_actions));
   SetCollectUserDataOptions(nullptr);
   SetForm(nullptr, base::DoNothing(), base::DoNothing());
   EnterState(AutofillAssistantState::STOPPED);
@@ -986,6 +1001,7 @@ void Controller::OnGetScripts(const GURL& url,
             << ", http-status=" << http_status;
 #endif
     OnFatalError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_DEFAULT_ERROR),
+                 /*show_feedback_chip=*/true,
                  Metrics::DropOutReason::GET_SCRIPTS_FAILED);
     return;
   }
@@ -999,6 +1015,7 @@ void Controller::OnGetScripts(const GURL& url,
             << "unparseable response";
 #endif
     OnFatalError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_DEFAULT_ERROR),
+                 /*show_feedback_chip=*/true,
                  Metrics::DropOutReason::GET_SCRIPTS_UNPARSABLE);
     return;
   }
@@ -1050,7 +1067,7 @@ void Controller::OnGetScripts(const GURL& url,
     if (state_ == AutofillAssistantState::TRACKING) {
       OnFatalError(
           l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_DEFAULT_ERROR),
-          Metrics::DropOutReason::NO_SCRIPTS);
+          /*show_feedback_chip=*/false, Metrics::DropOutReason::NO_SCRIPTS);
       return;
     }
     OnNoRunnableScriptsForPage();
@@ -1120,7 +1137,7 @@ void Controller::OnScriptExecuted(const std::string& script_path,
 
     case ScriptExecutor::SHUTDOWN_GRACEFULLY:
       if (!tracking_) {
-        EnterStoppedState();
+        EnterStoppedState(/*show_feedback_chip=*/false);
         RecordDropOutOrShutdown(Metrics::DropOutReason::SCRIPT_SHUTDOWN);
         return;
       }
@@ -1692,7 +1709,7 @@ void Controller::OnScriptError(const std::string& error_message,
     SetProgressBarErrorState(true);
   }
 
-  EnterStoppedState();
+  EnterStoppedState(/*show_feedback_chip=*/true);
 
   if (tracking_) {
     EnterState(AutofillAssistantState::TRACKING);
@@ -1703,6 +1720,7 @@ void Controller::OnScriptError(const std::string& error_message,
 }
 
 void Controller::OnFatalError(const std::string& error_message,
+                              bool show_feedback_chip,
                               Metrics::DropOutReason reason) {
   LOG(ERROR) << "Autofill Assistant has encountered a fatal error and is "
                 "shutting down, reason="
@@ -1712,7 +1730,7 @@ void Controller::OnFatalError(const std::string& error_message,
 
   SetStatusMessage(error_message);
   SetProgressBarErrorState(true);
-  EnterStoppedState();
+  EnterStoppedState(show_feedback_chip);
 
   // If we haven't managed to check the set of scripts yet at this point, we
   // never will.

components/autofill_assistant/browser/controller.h

@@ -229,6 +229,7 @@ class Controller : public ScriptExecutorDelegate,
   void GetRestrictedArea(std::vector<RectF>* area) const override;
   void GetVisualViewport(RectF* visual_viewport) const override;
   void OnFatalError(const std::string& error_message,
+                    bool show_feedback_chip,
                     Metrics::DropOutReason reason) override;
   void OnStop(const std::string& message,
               const std::string& button_label) override;
@@ -379,7 +380,11 @@ class Controller : public ScriptExecutorDelegate,
   void ShowFirstMessageAndStart();
 
   // Clear out visible state and enter the stopped state.
-  void EnterStoppedState();
+  // If |show_feedback_chip| is true, a "Send feedback" chip will be added to
+  // the bottom sheet.
+  void EnterStoppedState(bool show_feedback_chip);
+
+  void OnFeedbackChipClicked();
 
   ElementArea* touchable_element_area();
   ScriptTracker* script_tracker();

components/autofill_assistant/browser/controller_observer.h

@@ -131,6 +131,9 @@ class ControllerObserver : public base::CheckedObserver {
 
   // Called when the desired overlay behavior has changed.
   virtual void OnShouldShowOverlayChanged(bool should_show) = 0;
+
+  // Called when the feedback form has been requested.
+  virtual void OnFeedbackFormRequested() = 0;
 };
 }  // namespace autofill_assistant
 #endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_CONTROLLER_OBSERVER_H_

components/autofill_assistant/browser/controller_unittest.cc

@@ -640,7 +640,26 @@ TEST_F(ControllerTest, Autostart) {
                                    AutofillAssistantState::STOPPED));
 }
 
-TEST_F(ControllerTest, AutostartIsNotPassedToTheUi) {
+TEST_F(ControllerTest,
+       AutostartFallbackWithNoRunnableScriptsShowsFeedbackChip) {
+  SupportsScriptResponseProto script_response;
+  auto* autostart = AddRunnableScript(&script_response, "runnable");
+  autostart->mutable_presentation()->set_autostart(true);
+  RunOnce(autostart);
+  SetRepeatedScriptResponse(script_response);
+
+  Start("http://a.example.com/path");
+  ASSERT_THAT(controller_->GetUserActions(), SizeIs(1));
+  EXPECT_EQ(FEEDBACK_ACTION, controller_->GetUserActions().at(0).chip().type);
+}
+
+TEST_F(ControllerTest,
+       AutostartErrorDoesNotShowFeedbackChipWithFeatureFlagDisabled) {
+  // Disable the feedback chip feature.
+  scoped_feature_list_.Reset();
+  scoped_feature_list_.InitAndDisableFeature(
+      features::kAutofillAssistantFeedbackChip);
+
   SupportsScriptResponseProto script_response;
   auto* autostart = AddRunnableScript(&script_response, "runnable");
   autostart->mutable_presentation()->set_autostart(true);
@@ -834,8 +853,9 @@ TEST_F(ControllerTest, StateChanges) {
                                    AutofillAssistantState::PROMPT,
                                    AutofillAssistantState::STOPPED));
 
-  // The cancel button is removed.
-  EXPECT_TRUE(controller_->GetUserActions().empty());
+  // The cancel button is removed and the feedback chip is displayed.
+  ASSERT_THAT(controller_->GetUserActions(), SizeIs(1));
+  EXPECT_EQ(FEEDBACK_ACTION, controller_->GetUserActions().at(0).chip().type);
 }
 
 TEST_F(ControllerTest, AttachUIWhenStarting) {
@@ -858,7 +878,8 @@ TEST_F(ControllerTest, AttachUIWhenContentsFocused) {
   SimulateWebContentsFocused();  // must call AttachUI
 
   EXPECT_CALL(mock_client_, AttachUI());
-  controller_->OnFatalError("test", Metrics::DropOutReason::TAB_CHANGED);
+  controller_->OnFatalError("test", /*show_feedback_chip= */ false,
+                            Metrics::DropOutReason::TAB_CHANGED);
   EXPECT_EQ(AutofillAssistantState::STOPPED, controller_->GetState());
   SimulateWebContentsFocused();  // must call AttachUI
 }
@@ -3016,4 +3037,19 @@ TEST_F(ControllerTest, Details) {
   EXPECT_THAT(controller_->GetDetails(), IsEmpty());
   EXPECT_THAT(observed_details, IsEmpty());
 }
+
+TEST_F(ControllerTest, OnScriptErrorWillAppendVanishingFeedbackChip) {
+  // A script error should show the feedback chip.
+  EXPECT_CALL(mock_observer_, OnUserActionsChanged(SizeIs(1)));
+  EXPECT_CALL(mock_client_, RecordDropOut(Metrics::DropOutReason::NAVIGATION));
+  controller_->OnScriptError("Error", Metrics::DropOutReason::NAVIGATION);
+  EXPECT_EQ(AutofillAssistantState::STOPPED, controller_->GetState());
+
+  // The chip should vanish once clicked.
+  EXPECT_CALL(mock_observer_, OnUserActionsChanged(SizeIs(0)));
+  EXPECT_CALL(mock_client_,
+              Shutdown(Metrics::DropOutReason::UI_CLOSED_UNEXPECTEDLY));
+  EXPECT_TRUE(controller_->PerformUserAction(0));
+}
+
 }  // namespace autofill_assistant

components/autofill_assistant/browser/features.cc

@@ -28,6 +28,18 @@ const base::Feature kAutofillAssistantDisableOnboardingFlow{
     "AutofillAssistantDisableOnboardingFlow",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Controls whether to show the "Send feedback" chip while in an error state.
+const base::Feature kAutofillAssistantFeedbackChip{
+    "AutofillAssistantFeedbackChip", base::FEATURE_ENABLED_BY_DEFAULT};
+
+const base::Feature kAutofillAssistantProactiveHelp{
+    "AutofillAssistantProactiveHelp", base::FEATURE_DISABLED_BY_DEFAULT};
+
+// Use Chrome's TabHelper system to deal with the life cycle of WebContent's
+// depending Autofill Assistant objects.
+const base::Feature kAutofillAssistantWithTabHelper{
+    "AutofillAssistantWithTabHelper", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // By default, proactive help is only offered if MSBB is turned on. This feature
 // flag allows disabling the link. Proactive help can still be offered to users
 // so long as no communication to a remote backend is required. Specifically,
@@ -43,13 +55,5 @@ const base::Feature kAutofillAssistantLoadDFMForTriggerScripts{
     "AutofillAssistantLoadDFMForTriggerScripts",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
-const base::Feature kAutofillAssistantProactiveHelp{
-    "AutofillAssistantProactiveHelp", base::FEATURE_DISABLED_BY_DEFAULT};
-
-// Use Chrome's TabHelper system to deal with the life cycle of WebContent's
-// depending Autofill Assistant objects.
-const base::Feature kAutofillAssistantWithTabHelper{
-    "AutofillAssistantWithTabHelper", base::FEATURE_DISABLED_BY_DEFAULT};
-
 }  // namespace features
 }  // namespace autofill_assistant

components/autofill_assistant/browser/features.h

@@ -19,6 +19,7 @@ extern const base::Feature kAutofillAssistantDialogOnboarding;
 extern const base::Feature kAutofillAssistantDirectActions;
 extern const base::Feature kAutofillAssistantDisableOnboardingFlow;
 extern const base::Feature kAutofillAssistantDisableProactiveHelpTiedToMSBB;
+extern const base::Feature kAutofillAssistantFeedbackChip;
 extern const base::Feature kAutofillAssistantLoadDFMForTriggerScripts;
 extern const base::Feature kAutofillAssistantProactiveHelp;
 extern const base::Feature kAutofillAssistantWithTabHelper;

components/autofill_assistant/browser/mock_controller_observer.h

@@ -62,6 +62,7 @@ class MockControllerObserver : public ControllerObserver {
   MOCK_METHOD1(OnGenericUserInterfaceChanged,
                void(const GenericUserInterfaceProto* generic_ui));
   MOCK_METHOD1(OnShouldShowOverlayChanged, void(bool should_show));
+  MOCK_METHOD0(OnFeedbackFormRequested, void());
 };
 
 }  // namespace autofill_assistant

components/autofill_assistant/browser/ui_delegate.h

@@ -179,6 +179,7 @@ class UiDelegate {
 
   // Reports a fatal error to Autofill Assistant, which should then stop.
   virtual void OnFatalError(const std::string& error_message,
+                            bool show_feedback_chip,
                             Metrics::DropOutReason reason) = 0;
 
   // Reports that Autofill Assistant should be Stopped.

components/autofill_assistant_strings.grdp

@@ -27,5 +27,8 @@
     <message name="IDS_AUTOFILL_ASSISTANT_STOPPED" desc="Text label that is shown when stopping the Autofill Assistant.">
       Google Assistant in Chrome stopping
     </message>
+    <message name="IDS_AUTOFILL_ASSISTANT_SEND_FEEDBACK" desc="Option shown in the menu when clicking the Autofill Assistant profile icon. Clicking this option will open a feedback sharing dialog.">
+      Send feedback
+    </message>
   </if>
 </grit-part>

components/autofill_assistant_strings_grdp/IDS_AUTOFILL_ASSISTANT_SEND_FEEDBACK.png.sha1

+65dfd26a9666766181fca7942b31e3c40d42b675
\ No newline at end of file

components/autofill_assistant_strings_grdp/OWNERS

+file://components/autofill_assistant/OWNERS