commit 40c757ff1a70344d7f77acd02d70f40909f0603b
author Stephane Zermatten <szermatt@chromium.org> Fri Feb 01 09:59:25 2019
committer Stephane Zermatten <szermatt@chromium.org> Fri Feb 01 09:59:25 2019
tree ebd39ff021ce1b37b01fc515ddf972ca9dc8d123
parent bf03dc89143d32520ca419c23c96316f35e801b5

This is the cherry pick merge for the M73 branch which was approved here: http://crbug.com/927205

[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-Original-Commit-Position: refs/heads/master@{#626978}(cherry picked from commit c5c8dabd64a57d29633843353d66e21c04a0589c)
Reviewed-on: https://chromium-review.googlesource.com/c/1449571
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#110}
Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896}

components/autofill_assistant/browser/actions/show_details_action.cc

diff --git a/components/autofill_assistant/browser/actions/show_details_action.cc b/components/autofill_assistant/browser/actions/show_details_action.cc
index 18b68e7..3568b6f 100644
--- a/components/autofill_assistant/browser/actions/show_details_action.cc
+++ b/components/autofill_assistant/browser/actions/show_details_action.cc
@@ -25,9 +25,11 @@
 
 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;
   }
 
@@ -37,7 +39,7 @@
   delegate->SetDetails(details);
 
   if (!details.changes.user_approval_required()) {
-    OnActionProcessed(std::move(callback), ACTION_APPLIED);
+    OnActionProcessed(ACTION_APPLIED);
     return;
   }
 
@@ -55,7 +57,7 @@
   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.
@@ -65,20 +67,19 @@
   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;
@@ -86,13 +87,14 @@
     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

diff --git a/components/autofill_assistant/browser/actions/show_details_action.h b/components/autofill_assistant/browser/actions/show_details_action.h
index f6324799..f29c3f1bd 100644
--- a/components/autofill_assistant/browser/actions/show_details_action.h
+++ b/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 @@
   // 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);