[Autofill Assistant] Fix bug in ShowDetailsAction.

Before this patch, users selecting "Go Back" when asked to confirm
inconsistent details caused a crash. This was caused by std::move being
called twice on the same callback.

This patch gets rid of the crash by keeping the callback in the action
instance instead of the individual callbacks.

Bug: 806868
Change-Id: I6f8520e4f3b97bed40369cc95f19c5d2e763f95f
Reviewed-on: https://chromium-review.googlesource.com/c/1442651
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626978}

components/autofill_assistant/browser/actions/show_details_action.cc

@@ -26,9 +26,11 @@ ShowDetailsAction::~ShowDetailsAction() {}
 
 void ShowDetailsAction::InternalProcessAction(ActionDelegate* delegate,
                                               ProcessActionCallback callback) {
+  callback_ = std::move(callback);
+
   if (!proto_.show_details().has_details()) {
     delegate->ClearDetails();
-    OnActionProcessed(std::move(callback), ACTION_APPLIED);
+    OnActionProcessed(ACTION_APPLIED);
     return;
   }
 
@@ -38,7 +40,7 @@ void ShowDetailsAction::InternalProcessAction(ActionDelegate* delegate,
   delegate->SetDetails(details);
 
   if (!details.changes.user_approval_required()) {
-    OnActionProcessed(std::move(callback), ACTION_APPLIED);
+    OnActionProcessed(ACTION_APPLIED);
     return;
   }
 
@@ -56,7 +58,7 @@ void ShowDetailsAction::InternalProcessAction(ActionDelegate* delegate,
   chips->back().type = Chip::Type::BUTTON_FILLED_BLUE;
   chips->back().callback = base::BindOnce(
       &ShowDetailsAction::OnUserResponse, weak_ptr_factory_.GetWeakPtr(),
-      std::move(callback), base::Unretained(delegate), previous_status_message,
+      base::Unretained(delegate), previous_status_message,
       /* success= */ true);
 
   // Go back button.
@@ -66,34 +68,34 @@ void ShowDetailsAction::InternalProcessAction(ActionDelegate* delegate,
   chips->back().type = Chip::Type::BUTTON_TEXT;
   chips->back().callback = base::BindOnce(
       &ShowDetailsAction::OnUserResponse, weak_ptr_factory_.GetWeakPtr(),
-      std::move(callback), base::Unretained(delegate), previous_status_message,
+      base::Unretained(delegate), previous_status_message,
       /* success= */ false);
 
   delegate->Prompt(std::move(chips));
 }
 
 void ShowDetailsAction::OnUserResponse(
-    ProcessActionCallback callback,
     ActionDelegate* delegate,
     const std::string& previous_status_message,
     bool can_continue) {
   if (!can_continue) {
     delegate->Close();
-    OnActionProcessed(std::move(callback), MANUAL_FALLBACK);
+    OnActionProcessed(MANUAL_FALLBACK);
   } else {
     // Same details, without highlights.
     Details details;
     details.proto = proto_.show_details().details();
     delegate->SetDetails(details);
     // Restore status message
     delegate->SetStatusMessage(previous_status_message);
-    OnActionProcessed(std::move(callback), ACTION_APPLIED);
+    OnActionProcessed(ACTION_APPLIED);
   }
 }
 
-void ShowDetailsAction::OnActionProcessed(ProcessActionCallback callback,
-                                          ProcessedActionStatusProto status) {
+void ShowDetailsAction::OnActionProcessed(ProcessedActionStatusProto status) {
+  DCHECK(callback_);
+
   UpdateProcessedAction(status);
-  std::move(callback).Run(std::move(processed_action_proto_));
+  std::move(callback_).Run(std::move(processed_action_proto_));
 }
 }  // namespace autofill_assistant

components/autofill_assistant/browser/actions/show_details_action.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "components/autofill_assistant/browser/actions/action.h"
@@ -22,13 +23,12 @@ class ShowDetailsAction : public Action {
   // Overrides Action:
   void InternalProcessAction(ActionDelegate* delegate,
                              ProcessActionCallback callback) override;
-  void OnUserResponse(ProcessActionCallback callback,
-                      ActionDelegate* delegate,
+  void OnUserResponse(ActionDelegate* delegate,
                       const std::string& old_status_message,
                       bool can_continue);
-  void OnActionProcessed(ProcessActionCallback callback,
-                         ProcessedActionStatusProto status);
+  void OnActionProcessed(ProcessedActionStatusProto status);
 
+  ProcessActionCallback callback_;
   base::WeakPtrFactory<ShowDetailsAction> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ShowDetailsAction);