[Autofill Assistant] Forward client status from WebController.

Before this change, the reason behind an error that happened in
WebController was always lost, as all methods would return a boolean,
either true or false. This makes it hard to debug errors, as many
different errors end up being reported as OTHER.

With this change, the WebController methods can return a
ProcessedActionStatusProto, which ends up being included into the RPC
response, in most case, allowing WebController to let the server know
whether the error failed because the element was not found, or for other
reasons.

This change also add new client status code, which correspond to errors
in WebController.

This change introduces a ClientStatus C++ type, which is, for now, just
a wrapper for ProcessedActionStatusProto. This prepares the way for
adding more debugging information into the status, which can be
forwarded to the server, such as details about clicks.

BUG=b/129387787

Change-Id: I9df154f51401aa52a84d6b8c80c6c7ad02dc61c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1543772
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#646241}

components/autofill_assistant/browser/BUILD.gn

@@ -78,6 +78,8 @@ jumbo_static_library("browser") {
     "client.h",
     "client_memory.cc",
     "client_memory.h",
+    "client_status.cc",
+    "client_status.h",
     "controller.cc",
     "controller.h",
     "details.cc",

components/autofill_assistant/browser/actions/action.cc

@@ -20,10 +20,14 @@ void Action::ProcessAction(ActionDelegate* delegate,
   InternalProcessAction(delegate, std::move(callback));
 }
 
-void Action::UpdateProcessedAction(ProcessedActionStatusProto status) {
+void Action::UpdateProcessedAction(ProcessedActionStatusProto status_proto) {
+  UpdateProcessedAction(ClientStatus(status_proto));
+}
+
+void Action::UpdateProcessedAction(const ClientStatus& status) {
   // Safety check in case process action is run twice.
   *processed_action_proto_->mutable_action() = proto_;
-  processed_action_proto_->set_status(status);
+  status.FillProto(processed_action_proto_.get());
 }
 
 // static

components/autofill_assistant/browser/actions/action.h

@@ -17,6 +17,7 @@
 namespace autofill_assistant {
 
 class ActionDelegate;
+class ClientStatus;
 
 // An action that performs a single step of a script on the website.
 class Action {
@@ -45,6 +46,7 @@ class Action {
       const google::protobuf::RepeatedPtrField<std::string>& repeated_strings);
 
   void UpdateProcessedAction(ProcessedActionStatusProto status);
+  void UpdateProcessedAction(const ClientStatus& status);
 
   // Intended for debugging. Writes a string representation of |action| to
   // |out|.

components/autofill_assistant/browser/actions/action_delegate.h

@@ -31,6 +31,7 @@ class WebContents;
 
 namespace autofill_assistant {
 class ClientMemory;
+class ClientStatus;
 
 // Action delegate called when processing actions.
 class ActionDelegate {
@@ -74,8 +75,9 @@ class ActionDelegate {
       base::OnceCallback<void(ProcessedActionStatusProto)> callback) = 0;
 
   // Click or tap the element given by |selector| on the web page.
-  virtual void ClickOrTapElement(const Selector& selector,
-                                 base::OnceCallback<void(bool)> callback) = 0;
+  virtual void ClickOrTapElement(
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Ask user to select one of the given suggestions.
   //
@@ -108,26 +110,30 @@ class ActionDelegate {
 
   // Fill the address form given by |selector| with the given address
   // |profile|. |profile| cannot be nullptr.
-  virtual void FillAddressForm(const autofill::AutofillProfile* profile,
-                               const Selector& selector,
-                               base::OnceCallback<void(bool)> callback) = 0;
+  virtual void FillAddressForm(
+      const autofill::AutofillProfile* profile,
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Fill the card form given by |selector| with the given |card| and its
   // |cvc|. Return result asynchronously through |callback|.
-  virtual void FillCardForm(std::unique_ptr<autofill::CreditCard> card,
-                            const base::string16& cvc,
-                            const Selector& selector,
-                            base::OnceCallback<void(bool)> callback) = 0;
+  virtual void FillCardForm(
+      std::unique_ptr<autofill::CreditCard> card,
+      const base::string16& cvc,
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Select the option given by |selector| and the value of the option to be
   // picked.
-  virtual void SelectOption(const Selector& selector,
-                            const std::string& selected_option,
-                            base::OnceCallback<void(bool)> callback) = 0;
+  virtual void SelectOption(
+      const Selector& selector,
+      const std::string& selected_option,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Focus on the element given by |selector|.
-  virtual void FocusElement(const Selector& selector,
-                            base::OnceCallback<void(bool)> callback) = 0;
+  virtual void FocusElement(
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Sets selector of areas that can be manipulated:
   // - after the end of the script and before the beginning of the next script.
@@ -137,34 +143,39 @@ class ActionDelegate {
       const ElementAreaProto& touchable_element_area) = 0;
 
   // Highlight the element given by |selector|.
-  virtual void HighlightElement(const Selector& selector,
-                                base::OnceCallback<void(bool)> callback) = 0;
+  virtual void HighlightElement(
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Set the |value| of field |selector| and return the result through
   // |callback|. If |simulate_key_presses| is true, the value will be set by
   // clicking the field and then simulating key presses, otherwise the `value`
   // attribute will be set directly.
-  virtual void SetFieldValue(const Selector& selector,
-                             const std::string& value,
-                             bool simulate_key_presses,
-                             base::OnceCallback<void(bool)> callback) = 0;
+  virtual void SetFieldValue(
+      const Selector& selector,
+      const std::string& value,
+      bool simulate_key_presses,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Set the |value| of the |attribute| of the element given by |selector|.
-  virtual void SetAttribute(const Selector& selector,
-                            const std::vector<std::string>& attribute,
-                            const std::string& value,
-                            base::OnceCallback<void(bool)> callback) = 0;
+  virtual void SetAttribute(
+      const Selector& selector,
+      const std::vector<std::string>& attribute,
+      const std::string& value,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Sets the keyboard focus to |selector| and inputs the specified text parts.
   // Returns the result through |callback|.
-  virtual void SendKeyboardInput(const Selector& selector,
-                                 const std::vector<std::string>& text_parts,
-                                 base::OnceCallback<void(bool)> callback) = 0;
+  virtual void SendKeyboardInput(
+      const Selector& selector,
+      const std::vector<std::string>& text_parts,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
   // Return the outerHTML of an element given by |selector|.
   virtual void GetOuterHtml(
       const Selector& selector,
-      base::OnceCallback<void(bool, const std::string&)> callback) = 0;
+      base::OnceCallback<void(const ClientStatus&, const std::string&)>
+          callback) = 0;
 
   // Load |url| in the current tab. Returns immediately, before the new page has
   // been loaded.

components/autofill_assistant/browser/actions/autofill_action.cc

@@ -16,6 +16,7 @@
 #include "components/autofill_assistant/browser/actions/action_delegate.h"
 #include "components/autofill_assistant/browser/batch_element_checker.h"
 #include "components/autofill_assistant/browser/client_memory.h"
+#include "components/autofill_assistant/browser/client_status.h"
 
 namespace autofill_assistant {
 
@@ -64,6 +65,10 @@ void AutofillAction::InternalProcessAction(
 }
 
 void AutofillAction::EndAction(ProcessedActionStatusProto status) {
+  EndAction(ClientStatus(status));
+}
+
+void AutofillAction::EndAction(const ClientStatus& status) {
   UpdateProcessedAction(status);
   std::move(process_action_callback_).Run(std::move(processed_action_proto_));
 }
@@ -114,17 +119,16 @@ void AutofillAction::OnGetFullCard(ActionDelegate* delegate,
                                         weak_ptr_factory_.GetWeakPtr()));
 }
 
-void AutofillAction::OnCardFormFilled(bool successful) {
+void AutofillAction::OnCardFormFilled(const ClientStatus& status) {
   // TODO(crbug.com/806868): Implement required fields checking for cards.
-  EndAction(successful ? ACTION_APPLIED : OTHER_ACTION_STATUS);
-  return;
+  EndAction(status);
 }
 
 void AutofillAction::OnAddressFormFilled(ActionDelegate* delegate,
-                                         bool successful) {
+                                         const ClientStatus& status) {
   // In case Autofill failed, we fail the action.
-  if (!successful) {
-    EndAction(OTHER_ACTION_STATUS);
+  if (!status.ok()) {
+    EndAction(status);
     return;
   }
 
@@ -257,8 +261,8 @@ void AutofillAction::SetFallbackFieldValuesSequentially(
 
 void AutofillAction::OnSetFallbackFieldValue(ActionDelegate* delegate,
                                              int required_fields_index,
-                                             bool successful) {
-  if (!successful) {
+                                             const ClientStatus& status) {
+  if (!status.ok()) {
     // Fallback failed: we stop the script without checking the fields.
     EndAction(MANUAL_FALLBACK);
     return;

components/autofill_assistant/browser/actions/autofill_action.h

@@ -21,6 +21,8 @@ class CreditCard;
 }  // namespace autofill
 
 namespace autofill_assistant {
+class ClientStatus;
+
 // An action to autofill a form using a local address or credit card.
 class AutofillAction : public Action {
  public:
@@ -35,6 +37,7 @@ class AutofillAction : public Action {
                              ProcessActionCallback callback) override;
 
   void EndAction(ProcessedActionStatusProto status);
+  void EndAction(const ClientStatus& status);
 
   // Fill the form using data in client memory. Return whether filling succeeded
   // or not through OnAddressFormFilled or OnCardFormFilled.
@@ -47,10 +50,11 @@ class AutofillAction : public Action {
                      const base::string16& cvc);
 
   // Called when the credit card form has been filled.
-  void OnCardFormFilled(bool successful);
+  void OnCardFormFilled(const ClientStatus& status);
 
   // Called when the address form has been filled.
-  void OnAddressFormFilled(ActionDelegate* delegate, bool successful);
+  void OnAddressFormFilled(ActionDelegate* delegate,
+                           const ClientStatus& status);
 
   // Check whether all required fields have a non-empty value. If it is the
   // case, finish the action successfully. If it's not and |allow_fallback|
@@ -86,7 +90,7 @@ class AutofillAction : public Action {
   // after failed validation.
   void OnSetFallbackFieldValue(ActionDelegate* delegate,
                                int required_fields_index,
-                               bool successful);
+                               const ClientStatus& status);
 
   // Usage of the autofilled address. Ignored if autofilling a card.
   std::string name_;

components/autofill_assistant/browser/actions/autofill_action_unittest.cc

@@ -14,6 +14,7 @@
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill_assistant/browser/actions/mock_action_delegate.h"
 #include "components/autofill_assistant/browser/client_memory.h"
+#include "components/autofill_assistant/browser/client_status.h"
 #include "components/autofill_assistant/browser/mock_run_once_callback.h"
 #include "components/autofill_assistant/browser/mock_web_controller.h"
 #include "components/autofill_assistant/browser/service.pb.h"
@@ -209,7 +210,7 @@ TEST_F(AutofillActionTest, ValidationSucceeds) {
   // Autofill succeeds.
   EXPECT_CALL(mock_action_delegate_,
               OnFillAddressForm(NotNull(), Eq(Selector({kFakeSelector})), _))
-      .WillOnce(RunOnceCallback<2>(true));
+      .WillOnce(RunOnceCallback<2>(OkClientStatus()));
 
   // Validation succeeds.
   ON_CALL(mock_web_controller_, OnGetFieldValue(_, _))
@@ -233,7 +234,7 @@ TEST_F(AutofillActionTest, FallbackFails) {
   // Autofill succeeds.
   EXPECT_CALL(mock_action_delegate_,
               OnFillAddressForm(NotNull(), Eq(Selector({kFakeSelector})), _))
-      .WillOnce(RunOnceCallback<2>(true));
+      .WillOnce(RunOnceCallback<2>(OkClientStatus()));
 
   // Validation fails when getting FIRST_NAME.
   EXPECT_CALL(mock_web_controller_,
@@ -249,7 +250,7 @@ TEST_F(AutofillActionTest, FallbackFails) {
   // Fallback fails.
   EXPECT_CALL(mock_action_delegate_,
               OnSetFieldValue(Eq(Selector({"#first_name"})), kFirstName, _))
-      .WillOnce(RunOnceCallback<2>(false));
+      .WillOnce(RunOnceCallback<2>(ClientStatus(OTHER_ACTION_STATUS)));
 
   EXPECT_EQ(ProcessedActionStatusProto::MANUAL_FALLBACK,
             ProcessAction(action_proto));
@@ -269,7 +270,7 @@ TEST_F(AutofillActionTest, FallbackSucceeds) {
   // Autofill succeeds.
   EXPECT_CALL(mock_action_delegate_,
               OnFillAddressForm(NotNull(), Eq(Selector({kFakeSelector})), _))
-      .WillOnce(RunOnceCallback<2>(true));
+      .WillOnce(RunOnceCallback<2>(OkClientStatus()));
 
   {
     InSequence seq;
@@ -288,7 +289,7 @@ TEST_F(AutofillActionTest, FallbackSucceeds) {
     // Fallback succeeds.
     EXPECT_CALL(mock_action_delegate_,
                 OnSetFieldValue(Eq(Selector({"#first_name"})), kFirstName, _))
-        .WillOnce(RunOnceCallback<2>(true));
+        .WillOnce(RunOnceCallback<2>(OkClientStatus()));
 
     // Second validation succeeds.
     EXPECT_CALL(mock_web_controller_, OnGetFieldValue(_, _))

components/autofill_assistant/browser/actions/click_action.cc

@@ -44,8 +44,9 @@ void ClickAction::OnWaitForElement(ActionDelegate* delegate,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void ClickAction::OnClick(ProcessActionCallback callback, bool status) {
-  UpdateProcessedAction(status ? ACTION_APPLIED : OTHER_ACTION_STATUS);
+void ClickAction::OnClick(ProcessActionCallback callback,
+                          const ClientStatus& status) {
+  UpdateProcessedAction(status);
   std::move(callback).Run(std::move(processed_action_proto_));
 }
 

components/autofill_assistant/browser/actions/click_action.h

@@ -13,6 +13,8 @@
 #include "components/autofill_assistant/browser/actions/action.h"
 
 namespace autofill_assistant {
+class ClientStatus;
+
 // An action to perform a mouse left button click on a given element on Web,
 // which is implemented as a touch tap on Mobile.
 class ClickAction : public Action {
@@ -28,7 +30,7 @@ class ClickAction : public Action {
   void OnWaitForElement(ActionDelegate* delegate,
                         ProcessActionCallback callback,
                         bool element_found);
-  void OnClick(ProcessActionCallback callback, bool status);
+  void OnClick(ProcessActionCallback callback, const ClientStatus& status);
 
   base::WeakPtrFactory<ClickAction> weak_ptr_factory_;
 

components/autofill_assistant/browser/actions/focus_element_action.cc

@@ -54,10 +54,10 @@ void FocusElementAction::OnWaitForElement(ActionDelegate* delegate,
 
 void FocusElementAction::OnFocusElement(ActionDelegate* delegate,
                                         ProcessActionCallback callback,
-                                        bool status) {
+                                        const ClientStatus& status) {
   delegate->SetTouchableElementArea(
       proto().focus_element().touchable_element_area());
-  UpdateProcessedAction(status ? ACTION_APPLIED : OTHER_ACTION_STATUS);
+  UpdateProcessedAction(status);
   std::move(callback).Run(std::move(processed_action_proto_));
 }
 

components/autofill_assistant/browser/actions/focus_element_action.h

@@ -27,7 +27,7 @@ class FocusElementAction : public Action {
                         bool element_found);
   void OnFocusElement(ActionDelegate* delegate,
                       ProcessActionCallback callback,
-                      bool status);
+                      const ClientStatus& status);
 
   base::WeakPtrFactory<FocusElementAction> weak_ptr_factory_;
 

components/autofill_assistant/browser/actions/highlight_element_action.cc

@@ -46,8 +46,8 @@ void HighlightElementAction::OnWaitForElement(ActionDelegate* delegate,
 }
 
 void HighlightElementAction::OnHighlightElement(ProcessActionCallback callback,
-                                                bool status) {
-  UpdateProcessedAction(status ? ACTION_APPLIED : OTHER_ACTION_STATUS);
+                                                const ClientStatus& status) {
+  UpdateProcessedAction(status);
   std::move(callback).Run(std::move(processed_action_proto_));
 }
 

components/autofill_assistant/browser/actions/highlight_element_action.h

@@ -29,7 +29,8 @@ class HighlightElementAction : public Action {
   void OnWaitForElement(ActionDelegate* delegate,
                         ProcessActionCallback callback,
                         bool element_found);
-  void OnHighlightElement(ProcessActionCallback callback, bool status);
+  void OnHighlightElement(ProcessActionCallback callback,
+                          const ClientStatus& status);
 
   base::WeakPtrFactory<HighlightElementAction> weak_ptr_factory_;
 

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -52,48 +52,50 @@ class MockActionDelegate : public ActionDelegate {
   MOCK_METHOD0(GetStatusMessage, std::string());
   MOCK_METHOD2(ClickOrTapElement,
                void(const Selector& selector,
-                    base::OnceCallback<void(bool)> callback));
+                    base::OnceCallback<void(const ClientStatus&)> callback));
 
   MOCK_METHOD2(Prompt,
                void(std::unique_ptr<std::vector<Chip>> chips,
                     base::OnceCallback<void()> on_terminate));
   MOCK_METHOD0(CancelPrompt, void());
 
-  void FillAddressForm(const autofill::AutofillProfile* profile,
-                       const Selector& selector,
-                       base::OnceCallback<void(bool)> callback) override {
+  void FillAddressForm(
+      const autofill::AutofillProfile* profile,
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override {
     OnFillAddressForm(profile, selector, callback);
   }
 
   MOCK_METHOD3(OnFillAddressForm,
                void(const autofill::AutofillProfile* profile,
                     const Selector& selector,
-                    base::OnceCallback<void(bool)>& callback));
+                    base::OnceCallback<void(const ClientStatus&)>& callback));
 
-  void FillCardForm(std::unique_ptr<autofill::CreditCard> card,
-                    const base::string16& cvc,
-                    const Selector& selector,
-                    base::OnceCallback<void(bool)> callback) override {
+  void FillCardForm(
+      std::unique_ptr<autofill::CreditCard> card,
+      const base::string16& cvc,
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override {
     OnFillCardForm(card->guid(), selector, callback);
   }
 
   MOCK_METHOD3(OnFillCardForm,
                void(const std::string& guid,
                     const Selector& selector,
-                    base::OnceCallback<void(bool)>& callback));
+                    base::OnceCallback<void(const ClientStatus&)>& callback));
   MOCK_METHOD3(SelectOption,
                void(const Selector& selector,
                     const std::string& selected_option,
-                    base::OnceCallback<void(bool)> callback));
+                    base::OnceCallback<void(const ClientStatus&)> callback));
   MOCK_METHOD2(FocusElement,
                void(const Selector& selector,
-                    base::OnceCallback<void(bool)> callback));
+                    base::OnceCallback<void(const ClientStatus&)> callback));
   MOCK_METHOD1(SetTouchableElementArea,
                void(const ElementAreaProto& touchable_element_area));
 
   MOCK_METHOD2(HighlightElement,
                void(const Selector& selector,
-                    base::OnceCallback<void(bool)> callback));
+                    base::OnceCallback<void(const ClientStatus&)> callback));
 
   MOCK_METHOD1(GetPaymentInformation,
                void(std::unique_ptr<PaymentRequestOptions> options));
@@ -103,29 +105,29 @@ class MockActionDelegate : public ActionDelegate {
   void SetFieldValue(const Selector& selector,
                      const std::string& value,
                      bool ignored_simulate_key_presses,
-                     base::OnceCallback<void(bool)> callback) {
+                     base::OnceCallback<void(const ClientStatus&)> callback) {
     OnSetFieldValue(selector, value, callback);
   }
 
   MOCK_METHOD3(OnSetFieldValue,
                void(const Selector& selector,
                     const std::string& value,
-                    base::OnceCallback<void(bool)>& callback));
+                    base::OnceCallback<void(const ClientStatus&)>& callback));
 
   MOCK_METHOD4(SetAttribute,
                void(const Selector& selector,
                     const std::vector<std::string>& attribute,
                     const std::string& value,
-                    base::OnceCallback<void(bool)> callback));
+                    base::OnceCallback<void(const ClientStatus&)> callback));
 
   MOCK_METHOD3(SendKeyboardInput,
                void(const Selector& selector,
                     const std::vector<std::string>& text_parts,
-                    base::OnceCallback<void(bool)> callback));
-  MOCK_METHOD2(
-      GetOuterHtml,
-      void(const Selector& selector,
-           base::OnceCallback<void(bool, const std::string&)> callback));
+                    base::OnceCallback<void(const ClientStatus&)> callback));
+  MOCK_METHOD2(GetOuterHtml,
+               void(const Selector& selector,
+                    base::OnceCallback<void(const ClientStatus&,
+                                            const std::string&)> callback));
   MOCK_METHOD1(LoadURL, void(const GURL& url));
   MOCK_METHOD0(Shutdown, void());
   MOCK_METHOD0(Close, void());

components/autofill_assistant/browser/actions/select_option_action.cc

@@ -51,8 +51,8 @@ void SelectOptionAction::OnWaitForElement(ActionDelegate* delegate,
 }
 
 void SelectOptionAction::OnSelectOption(ProcessActionCallback callback,
-                                        bool status) {
-  UpdateProcessedAction(status ? ACTION_APPLIED : OTHER_ACTION_STATUS);
+                                        const ClientStatus& status) {
+  UpdateProcessedAction(status);
   std::move(callback).Run(std::move(processed_action_proto_));
 }
 

components/autofill_assistant/browser/actions/select_option_action.h

@@ -27,7 +27,8 @@ class SelectOptionAction : public Action {
   void OnWaitForElement(ActionDelegate* delegate,
                         ProcessActionCallback callback,
                         bool element_found);
-  void OnSelectOption(ProcessActionCallback callback, bool status);
+  void OnSelectOption(ProcessActionCallback callback,
+                      const ClientStatus& status);
 
   base::WeakPtrFactory<SelectOptionAction> weak_ptr_factory_;
 

components/autofill_assistant/browser/actions/set_attribute_action.cc

@@ -47,8 +47,8 @@ void SetAttributeAction::OnWaitForElement(ActionDelegate* delegate,
 }
 
 void SetAttributeAction::OnSetAttribute(ProcessActionCallback callback,
-                                        bool status) {
-  UpdateProcessedAction(status ? ACTION_APPLIED : OTHER_ACTION_STATUS);
+                                        const ClientStatus& status) {
+  UpdateProcessedAction(status);
   std::move(callback).Run(std::move(processed_action_proto_));
 }
 

components/autofill_assistant/browser/actions/set_attribute_action.h

@@ -27,7 +27,8 @@ class SetAttributeAction : public Action {
   void OnWaitForElement(ActionDelegate* delegate,
                         ProcessActionCallback callback,
                         bool element_found);
-  void OnSetAttribute(ProcessActionCallback callback, bool status);
+  void OnSetAttribute(ProcessActionCallback callback,
+                      const ClientStatus& status);
 
   base::WeakPtrFactory<SetAttributeAction> weak_ptr_factory_;
 

components/autofill_assistant/browser/actions/set_form_field_value_action.cc

@@ -46,16 +46,16 @@ void SetFormFieldValueAction::OnWaitForElement(ActionDelegate* delegate,
 
   // Start with first value, then call OnSetFieldValue() recursively until done.
   OnSetFieldValue(delegate, std::move(callback), /* next = */ 0,
-                  /* status= */ true);
+                  OkClientStatus());
 }
 
 void SetFormFieldValueAction::OnSetFieldValue(ActionDelegate* delegate,
                                               ProcessActionCallback callback,
                                               int next,
-                                              bool status) {
+                                              const ClientStatus& status) {
   // If something went wrong or we are out of values: finish
-  if (!status || next >= proto_.set_form_value().value_size()) {
-    UpdateProcessedAction(status ? ACTION_APPLIED : OTHER_ACTION_STATUS);
+  if (!status.ok() || next >= proto_.set_form_value().value_size()) {
+    UpdateProcessedAction(status);
     std::move(callback).Run(std::move(processed_action_proto_));
     return;
   }
@@ -88,7 +88,7 @@ void SetFormFieldValueAction::OnSetFieldValue(ActionDelegate* delegate,
             << "and only supports US-ASCII values (encountered "
             << key_field.keycode() << "). Use field `key' instead.";
         OnSetFieldValue(delegate, std::move(callback), next,
-                        /* status= */ false);
+                        ClientStatus(INVALID_ACTION));
       }
       break;
     case SetFormFieldValueProto_KeyPress::kKeyboardInput:
@@ -100,7 +100,8 @@ void SetFormFieldValueAction::OnSetFieldValue(ActionDelegate* delegate,
       break;
     default:
       DVLOG(1) << "Unrecognized field for SetFormFieldValueProto_KeyPress";
-      OnSetFieldValue(delegate, std::move(callback), next, /* status= */ false);
+      OnSetFieldValue(delegate, std::move(callback), next,
+                      ClientStatus(INVALID_ACTION));
       break;
   }
 }

components/autofill_assistant/browser/actions/set_form_field_value_action.h

@@ -30,7 +30,7 @@ class SetFormFieldValueAction : public Action {
   void OnSetFieldValue(ActionDelegate* delegate,
                        ProcessActionCallback callback,
                        int next,
-                       bool status);
+                       const ClientStatus& status);
 
   base::WeakPtrFactory<SetFormFieldValueAction> weak_ptr_factory_;
 

components/autofill_assistant/browser/actions/upload_dom_action.cc

@@ -45,10 +45,10 @@ void UploadDomAction::OnWaitForElement(ActionDelegate* delegate,
 }
 
 void UploadDomAction::OnGetOuterHtml(ProcessActionCallback callback,
-                                     bool successful,
+                                     const ClientStatus& status,
                                      const std::string& outer_html) {
-  if (!successful) {
-    UpdateProcessedAction(OTHER_ACTION_STATUS);
+  if (!status.ok()) {
+    UpdateProcessedAction(status);
     std::move(callback).Run(std::move(processed_action_proto_));
     return;
   }

components/autofill_assistant/browser/actions/upload_dom_action.h

@@ -27,7 +27,7 @@ class UploadDomAction : public Action {
                         ProcessActionCallback callback,
                         bool element_found);
   void OnGetOuterHtml(ProcessActionCallback callback,
-                      bool successful,
+                      const ClientStatus& status,
                       const std::string& outer_html);
 
   base::WeakPtrFactory<UploadDomAction> weak_ptr_factory_;

components/autofill_assistant/browser/client_status.cc

+// Copyright 2019 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/client_status.h"
+
+#include "base/no_destructor.h"
+
+namespace autofill_assistant {
+
+ClientStatus::ClientStatus() : status_(UNKNOWN_ACTION_STATUS) {}
+ClientStatus::ClientStatus(ProcessedActionStatusProto status)
+    : status_(status) {}
+ClientStatus::~ClientStatus() = default;
+
+void ClientStatus::FillProto(ProcessedActionProto* proto) const {
+  proto->set_status(status_);
+  // TODO(b/129387787): Fill extra debugging information collected in the
+  // ClientStatus.
+}
+
+std::ostream& operator<<(std::ostream& out, const ClientStatus& status) {
+  return out << status.status_;
+}
+
+const ClientStatus& OkClientStatus() {
+  static base::NoDestructor<ClientStatus> ok_(ACTION_APPLIED);
+  return *ok_.get();
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         const ProcessedActionStatusProto& status) {
+#ifdef NDEBUG
+  out << static_cast<int>(status);
+  return out;
+#else
+  switch (status) {
+    case ProcessedActionStatusProto::UNKNOWN_ACTION_STATUS:
+      out << "UNKNOWN_ACTION_STATUS";
+      break;
+    case ProcessedActionStatusProto::ELEMENT_RESOLUTION_FAILED:
+      out << "ELEMENT_RESOLUTION_FAILED";
+      break;
+    case ProcessedActionStatusProto::ACTION_APPLIED:
+      out << "ACTION_APPLIED";
+      break;
+    case ProcessedActionStatusProto::OTHER_ACTION_STATUS:
+      out << "OTHER_ACTION_STATUS";
+      break;
+    case ProcessedActionStatusProto::PAYMENT_REQUEST_ERROR:
+      out << "PAYMENT_REQUEST_ERROR";
+      break;
+    case ProcessedActionStatusProto::UNSUPPORTED_ACTION:
+      out << "UNSUPPORTED_ACTION";
+      break;
+    case ProcessedActionStatusProto::MANUAL_FALLBACK:
+      out << "MANUAL_FALLBACK";
+      break;
+    case ProcessedActionStatusProto::INTERRUPT_FAILED:
+      out << "INTERRUPT_FAILED";
+      break;
+    case ProcessedActionStatusProto::USER_ABORTED_ACTION:
+      out << "USER_ABORTED_ACTION";
+      break;
+
+    case ProcessedActionStatusProto::GET_FULL_CARD_FAILED:
+      out << "GET_FULL_CARD_FAILED";
+      break;
+
+    case ProcessedActionStatusProto::PRECONDITION_FAILED:
+      out << "PRECONDITION_FAILED";
+      break;
+
+    case ProcessedActionStatusProto::INVALID_ACTION:
+      out << "INVALID_ACTION";
+      break;
+
+    case ProcessedActionStatusProto::UNSUPPORTED:
+      out << "UNSUPPORTED";
+      break;
+
+    case ProcessedActionStatusProto::TIMED_OUT:
+      out << "TIMED_OUT";
+      break;
+
+    case ProcessedActionStatusProto::ELEMENT_UNSTABLE:
+      out << "ELEMENT_UNSTABLE";
+      break;
+
+      // Intentionally no default case to make compilation fail if a new value
+      // was added to the enum but not to this list.
+  }
+  return out;
+#endif  // NDEBUG
+}
+
+}  // namespace autofill_assistant

components/autofill_assistant/browser/client_status.h

+// Copyright 2019 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_CLIENT_STATUS_H_
+#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_CLIENT_STATUS_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "components/autofill_assistant/browser/service.pb.h"
+
+namespace autofill_assistant {
+
+class ClientStatus {
+ public:
+  ClientStatus();
+  explicit ClientStatus(ProcessedActionStatusProto status);
+  ~ClientStatus();
+
+  // Returns true if this is an OK status.
+  bool ok() const { return status_ == ACTION_APPLIED; }
+
+  // Fills a ProcessedActionProto as appropriate for the current status.
+  void FillProto(ProcessedActionProto* proto) const;
+
+  // Returns the corresponding proto status.
+  ProcessedActionStatusProto proto_status() const { return status_; }
+
+  // Modifies the corresponding proto status.
+  void set_proto_status(ProcessedActionStatusProto status) { status_ = status; }
+
+  // The output operator, for logging.
+  friend std::ostream& operator<<(std::ostream& out,
+                                  const ClientStatus& status);
+
+ private:
+  ProcessedActionStatusProto status_;
+  // TODO(b/129387787): Add more information, to be reported to
+  // ProcessedActionProto
+};
+
+// An OK status.
+const ClientStatus& OkClientStatus();
+
+// Intended for debugging and test error output. Writes a string representation
+// of the status to |out|.
+std::ostream& operator<<(std::ostream& out,
+                         const ProcessedActionStatusProto& status);
+
+}  // namespace autofill_assistant
+
+#endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_CLIENT_STATUS_H_

components/autofill_assistant/browser/mock_web_controller.h

@@ -22,23 +22,25 @@ class MockWebController : public WebController {
 
   MOCK_METHOD1(LoadURL, void(const GURL&));
 
-  void ClickOrTapElement(const Selector& selector,
-                         base::OnceCallback<void(bool)> callback) override {
+  void ClickOrTapElement(
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override {
     // Transforming callback into a references allows using RunOnceCallback on
     // the argument.
     OnClickOrTapElement(selector, callback);
   }
   MOCK_METHOD2(OnClickOrTapElement,
                void(const Selector& selector,
-                    base::OnceCallback<void(bool)>& callback));
+                    base::OnceCallback<void(const ClientStatus&)>& callback));
 
-  void FocusElement(const Selector& selector,
-                    base::OnceCallback<void(bool)> callback) override {
+  void FocusElement(
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override {
     OnFocusElement(selector, callback);
   }
   MOCK_METHOD2(OnFocusElement,
                void(const Selector& selector,
-                    base::OnceCallback<void(bool)>& callback));
+                    base::OnceCallback<void(const ClientStatus&)>& callback));
 
   void ElementCheck(ElementCheckType check_type,
                     const Selector& selector,

components/autofill_assistant/browser/script_executor.cc

@@ -18,6 +18,7 @@
 #include "components/autofill_assistant/browser/actions/action.h"
 #include "components/autofill_assistant/browser/batch_element_checker.h"
 #include "components/autofill_assistant/browser/client_memory.h"
+#include "components/autofill_assistant/browser/client_status.h"
 #include "components/autofill_assistant/browser/protocol_utils.h"
 #include "components/autofill_assistant/browser/self_delete_full_card_requester.h"
 #include "components/autofill_assistant/browser/service.h"
@@ -34,58 +35,6 @@ namespace {
 constexpr base::TimeDelta kShortWaitForElementDeadline =
     base::TimeDelta::FromSeconds(2);
 
-// Intended for debugging. Writes a string representation of the status to
-// |out|.
-std::ostream& operator<<(std::ostream& out,
-                         const ProcessedActionStatusProto& status) {
-#ifdef NDEBUG
-  out << static_cast<int>(status);
-  return out;
-#else
-  switch (status) {
-    case ProcessedActionStatusProto::UNKNOWN_ACTION_STATUS:
-      out << "UNKNOWN_ACTION_STATUS";
-      break;
-    case ProcessedActionStatusProto::ELEMENT_RESOLUTION_FAILED:
-      out << "ELEMENT_RESOLUTION_FAILED";
-      break;
-    case ProcessedActionStatusProto::ACTION_APPLIED:
-      out << "ACTION_APPLIED";
-      break;
-    case ProcessedActionStatusProto::OTHER_ACTION_STATUS:
-      out << "OTHER_ACTION_STATUS";
-      break;
-    case ProcessedActionStatusProto::PAYMENT_REQUEST_ERROR:
-      out << "PAYMENT_REQUEST_ERROR";
-      break;
-    case ProcessedActionStatusProto::UNSUPPORTED_ACTION:
-      out << "UNSUPPORTED_ACTION";
-      break;
-    case ProcessedActionStatusProto::MANUAL_FALLBACK:
-      out << "MANUAL_FALLBACK";
-      break;
-    case ProcessedActionStatusProto::INTERRUPT_FAILED:
-      out << "INTERRUPT_FAILED";
-      break;
-    case ProcessedActionStatusProto::USER_ABORTED_ACTION:
-      out << "USER_ABORTED_ACTION";
-      break;
-
-    case ProcessedActionStatusProto::GET_FULL_CARD_FAILED:
-      out << "GET_FULL_CARD_FAILED";
-      break;
-
-    case ProcessedActionStatusProto::PRECONDITION_FAILED:
-      out << "PRECONDITION_FAILED";
-      break;
-
-      // Intentionally no default case to make compilation fail if a new value
-      // was added to the enum but not to this list.
-  }
-  return out;
-#endif  // NDEBUG
-}
-
 std::ostream& operator<<(std::ostream& out,
                          const ScriptExecutor::AtEnd& at_end) {
 #ifdef NDEBUG
@@ -206,7 +155,7 @@ std::string ScriptExecutor::GetStatusMessage() {
 
 void ScriptExecutor::ClickOrTapElement(
     const Selector& selector,
-    base::OnceCallback<void(bool)> callback) {
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   delegate_->GetWebController()->ClickOrTapElement(selector,
                                                    std::move(callback));
 }
@@ -298,36 +247,41 @@ void ScriptExecutor::OnChosen(base::OnceClosure callback) {
   std::move(callback).Run();
 }
 
-void ScriptExecutor::FillAddressForm(const autofill::AutofillProfile* profile,
-                                     const Selector& selector,
-                                     base::OnceCallback<void(bool)> callback) {
+void ScriptExecutor::FillAddressForm(
+    const autofill::AutofillProfile* profile,
+    const Selector& selector,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   delegate_->GetWebController()->FillAddressForm(profile, selector,
                                                  std::move(callback));
 }
 
-void ScriptExecutor::FillCardForm(std::unique_ptr<autofill::CreditCard> card,
-                                  const base::string16& cvc,
-                                  const Selector& selector,
-                                  base::OnceCallback<void(bool)> callback) {
+void ScriptExecutor::FillCardForm(
+    std::unique_ptr<autofill::CreditCard> card,
+    const base::string16& cvc,
+    const Selector& selector,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   delegate_->GetWebController()->FillCardForm(std::move(card), cvc, selector,
                                               std::move(callback));
 }
 
-void ScriptExecutor::SelectOption(const Selector& selector,
-                                  const std::string& selected_option,
-                                  base::OnceCallback<void(bool)> callback) {
+void ScriptExecutor::SelectOption(
+    const Selector& selector,
+    const std::string& selected_option,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   delegate_->GetWebController()->SelectOption(selector, selected_option,
                                               std::move(callback));
 }
 
-void ScriptExecutor::HighlightElement(const Selector& selector,
-                                      base::OnceCallback<void(bool)> callback) {
+void ScriptExecutor::HighlightElement(
+    const Selector& selector,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   delegate_->GetWebController()->HighlightElement(selector,
                                                   std::move(callback));
 }
 
-void ScriptExecutor::FocusElement(const Selector& selector,
-                                  base::OnceCallback<void(bool)> callback) {
+void ScriptExecutor::FocusElement(
+    const Selector& selector,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   last_focused_element_selector_ = selector;
   delegate_->GetWebController()->FocusElement(selector, std::move(callback));
 }
@@ -346,18 +300,20 @@ void ScriptExecutor::SetProgressVisible(bool visible) {
   delegate_->SetProgressVisible(visible);
 }
 
-void ScriptExecutor::SetFieldValue(const Selector& selector,
-                                   const std::string& value,
-                                   bool simulate_key_presses,
-                                   base::OnceCallback<void(bool)> callback) {
+void ScriptExecutor::SetFieldValue(
+    const Selector& selector,
+    const std::string& value,
+    bool simulate_key_presses,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   delegate_->GetWebController()->SetFieldValue(
       selector, value, simulate_key_presses, std::move(callback));
 }
 
-void ScriptExecutor::SetAttribute(const Selector& selector,
-                                  const std::vector<std::string>& attribute,
-                                  const std::string& value,
-                                  base::OnceCallback<void(bool)> callback) {
+void ScriptExecutor::SetAttribute(
+    const Selector& selector,
+    const std::vector<std::string>& attribute,
+    const std::string& value,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   delegate_->GetWebController()->SetAttribute(selector, attribute, value,
                                               std::move(callback));
 }
@@ -365,14 +321,15 @@ void ScriptExecutor::SetAttribute(const Selector& selector,
 void ScriptExecutor::SendKeyboardInput(
     const Selector& selector,
     const std::vector<std::string>& text_parts,
-    base::OnceCallback<void(bool)> callback) {
+    base::OnceCallback<void(const ClientStatus&)> callback) {
   delegate_->GetWebController()->SendKeyboardInput(selector, text_parts,
                                                    std::move(callback));
 }
 
 void ScriptExecutor::GetOuterHtml(
     const Selector& selector,
-    base::OnceCallback<void(bool, const std::string&)> callback) {
+    base::OnceCallback<void(const ClientStatus&, const std::string&)>
+        callback) {
   delegate_->GetWebController()->GetOuterHtml(selector, std::move(callback));
 }
 

components/autofill_assistant/browser/script_executor.h

@@ -112,44 +112,54 @@ class ScriptExecutor : public ActionDelegate {
       base::OnceCallback<void(ProcessedActionStatusProto)> callback) override;
   void SetStatusMessage(const std::string& message) override;
   std::string GetStatusMessage() override;
-  void ClickOrTapElement(const Selector& selector,
-                         base::OnceCallback<void(bool)> callback) override;
+  void ClickOrTapElement(
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
   void GetPaymentInformation(
       std::unique_ptr<PaymentRequestOptions> options) override;
   void GetFullCard(GetFullCardCallback callback) override;
   void Prompt(std::unique_ptr<std::vector<Chip>> chips,
               base::OnceCallback<void()> on_terminate) override;
   void CancelPrompt() override;
-  void FillAddressForm(const autofill::AutofillProfile* profile,
-                       const Selector& selector,
-                       base::OnceCallback<void(bool)> callback) override;
-  void FillCardForm(std::unique_ptr<autofill::CreditCard> card,
-                    const base::string16& cvc,
-                    const Selector& selector,
-                    base::OnceCallback<void(bool)> callback) override;
-  void SelectOption(const Selector& selector,
-                    const std::string& selected_option,
-                    base::OnceCallback<void(bool)> callback) override;
-  void HighlightElement(const Selector& selector,
-                        base::OnceCallback<void(bool)> callback) override;
-  void FocusElement(const Selector& selector,
-                    base::OnceCallback<void(bool)> callback) override;
+  void FillAddressForm(
+      const autofill::AutofillProfile* profile,
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
+  void FillCardForm(
+      std::unique_ptr<autofill::CreditCard> card,
+      const base::string16& cvc,
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
+  void SelectOption(
+      const Selector& selector,
+      const std::string& selected_option,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
+  void HighlightElement(
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
+  void FocusElement(
+      const Selector& selector,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
   void SetTouchableElementArea(
       const ElementAreaProto& touchable_element_area) override;
-  void SetFieldValue(const Selector& selector,
-                     const std::string& value,
-                     bool simulate_key_presses,
-                     base::OnceCallback<void(bool)> callback) override;
-  void SetAttribute(const Selector& selector,
-                    const std::vector<std::string>& attribute,
-                    const std::string& value,
-                    base::OnceCallback<void(bool)> callback) override;
-  void SendKeyboardInput(const Selector& selector,
-                         const std::vector<std::string>& text_parts,
-                         base::OnceCallback<void(bool)> callback) override;
+  void SetFieldValue(
+      const Selector& selector,
+      const std::string& value,
+      bool simulate_key_presses,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
+  void SetAttribute(
+      const Selector& selector,
+      const std::vector<std::string>& attribute,
+      const std::string& value,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
+  void SendKeyboardInput(
+      const Selector& selector,
+      const std::vector<std::string>& text_parts,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
   void GetOuterHtml(
       const Selector& selector,
-      base::OnceCallback<void(bool, const std::string&)> callback) override;
+      base::OnceCallback<void(const ClientStatus&, const std::string&)>
+          callback) override;
   void LoadURL(const GURL& url) override;
   void Shutdown() override;
   void Close() override;

components/autofill_assistant/browser/script_executor_unittest.cc

@@ -63,12 +63,12 @@ class ScriptExecutorTest : public testing::Test,
     // In this test, "tell" actions always succeed and "click" actions always
     // fail. The following makes a click action fail immediately
     ON_CALL(mock_web_controller_, OnClickOrTapElement(_, _))
-        .WillByDefault(RunOnceCallback<1>(false));
+        .WillByDefault(RunOnceCallback<1>(ClientStatus(OTHER_ACTION_STATUS)));
 
     ON_CALL(mock_web_controller_, OnElementCheck(_, _, _))
         .WillByDefault(RunOnceCallback<2>(true));
     ON_CALL(mock_web_controller_, OnFocusElement(_, _))
-        .WillByDefault(RunOnceCallback<1>(true));
+        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
   }
 
  protected:

components/autofill_assistant/browser/service.proto

@@ -404,6 +404,18 @@ enum ProcessedActionStatusProto {
   // failed.
   //
   PRECONDITION_FAILED = 10;
+
+  // The action definition returned by the server was rejected.
+  INVALID_ACTION = 11;
+
+  // Executing the action as defined is unsupported.
+  UNSUPPORTED = 12;
+
+  // Timed out waiting for the document to load.
+  TIMED_OUT = 13;
+
+  // Failed to get a stable position for the element, usually to click on it.
+  ELEMENT_UNSTABLE = 14;
 }
 
 // The pseudo type values come from