[Desktop] Pass AutofillProfile by const ref to the prompt callback.

ProfileImportProcess::SetUserDecision accepts the AutofillProfile as optional. This means that a copy of the AutofillProfile will be constructed from the data. Thus, these's no point in accepting the AutofillProfile by value is the decision callback. Fixed: 1486412 Change-Id: Icbc27e3c18e993c626c1f971d4dedd3fb765bd09 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4922294 Reviewed-by: Florian Leimgruber <fleimgruber@google.com> Reviewed-by: John Wu <jzw@chromium.org> Commit-Queue: Timofey Chudakov <tchudakov@google.com> Reviewed-by: Jan Keitel <jkeitel@google.com> Cr-Commit-Position: refs/heads/main@{#1209902}
chromium · Oct 15, 2023 · 89bf2e4 · 89bf2e4
1 parent 4f45d14
commit 89bf2e4
Show file tree

Hide file tree

Showing 32 changed files with 149 additions and 92 deletions.
diff --git a/chrome/browser/autofill/android/save_update_address_profile_message_controller.cc b/chrome/browser/autofill/android/save_update_address_profile_message_controller.cc
@@ -138,7 +138,7 @@ void SaveUpdateAddressProfileMessageController::DismissMessage() {
 
 void SaveUpdateAddressProfileMessageController::RunSaveAddressProfileCallback(
     AutofillClient::SaveAddressProfileOfferUserDecision decision) {
-  std::move(save_address_profile_callback_).Run(decision, profile_);
+  std::move(save_address_profile_callback_).Run(decision, std::nullopt);
   primary_action_callback_.Reset();
 }
 

diff --git a/chrome/browser/autofill/android/save_update_address_profile_message_controller_unittest.cc b/chrome/browser/autofill/android/save_update_address_profile_message_controller_unittest.cc
@@ -32,6 +32,8 @@
 namespace autofill {
 
 using testing::_;
+using profile_ref = base::optional_ref<const AutofillProfile>;
+using ::testing::Property;
 
 class SaveUpdateAddressProfileMessageControllerTest
     : public ChromeRenderViewHostTestHarness {
@@ -279,7 +281,8 @@ TEST_F(SaveUpdateAddressProfileMessageControllerTest,
   EXPECT_CALL(action_callback_, Run(_, profile_, nullptr, false, _));
   TriggerActionClick();
 
-  EXPECT_CALL(save_callback_, Run(_, profile_)).Times(0);
+  EXPECT_CALL(save_callback_, Run(_, Property(&profile_ref::has_value, false)))
+      .Times(0);
   TriggerMessageDismissedCallback(messages::DismissReason::PRIMARY_ACTION);
 }
 
@@ -293,7 +296,8 @@ TEST_F(SaveUpdateAddressProfileMessageControllerTest,
   EXPECT_CALL(action_callback_, Run(_, profile_, &original_profile_, _, _));
   TriggerActionClick();
 
-  EXPECT_CALL(save_callback_, Run(_, profile_)).Times(0);
+  EXPECT_CALL(save_callback_, Run(_, Property(&profile_ref::has_value, false)))
+      .Times(0);
   TriggerMessageDismissedCallback(messages::DismissReason::PRIMARY_ACTION);
 }
 
@@ -308,7 +312,7 @@ TEST_F(SaveUpdateAddressProfileMessageControllerTest,
   EXPECT_CALL(
       save_callback_,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kMessageDeclined,
-          profile_));
+          Property(&profile_ref::has_value, false)));
   TriggerMessageDismissedCallback(messages::DismissReason::GESTURE);
 }
 
@@ -323,7 +327,7 @@ TEST_F(SaveUpdateAddressProfileMessageControllerTest,
   EXPECT_CALL(
       save_callback_,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kMessageTimeout,
-          profile_));
+          Property(&profile_ref::has_value, false)));
   TriggerMessageDismissedCallback(messages::DismissReason::TIMER);
 }
 
@@ -340,7 +344,7 @@ TEST_F(SaveUpdateAddressProfileMessageControllerTest, OnlyOnePromptAtATime) {
       another_action_callback;
   EXPECT_CALL(save_callback_,
               Run(AutofillClient::SaveAddressProfileOfferUserDecision::kIgnored,
-                  profile_));
+                  Property(&profile_ref::has_value, false)));
   ExpectDismissMessageCall();
   EnqueueSaveMessage(another_profile, /*is_migration_to_account=*/false,
                      another_save_callback.Get(),

diff --git a/chrome/browser/autofill/android/save_update_address_profile_prompt_controller.cc b/chrome/browser/autofill/android/save_update_address_profile_prompt_controller.cc
@@ -264,7 +264,12 @@ void SaveUpdateAddressProfilePromptController::OnPromptDismissed(
 
 void SaveUpdateAddressProfilePromptController::RunSaveAddressProfileCallback(
     AutofillClient::SaveAddressProfileOfferUserDecision decision) {
-  std::move(decision_callback_).Run(decision, profile_);
+  std::move(decision_callback_)
+      .Run(decision,
+           decision == AutofillClient::SaveAddressProfileOfferUserDecision::
+                           kEditAccepted
+               ? base::optional_ref(profile_)
+               : std::nullopt);
 }
 
 }  // namespace autofill
diff --git a/chrome/browser/autofill/android/save_update_address_profile_prompt_controller_unittest.cc b/chrome/browser/autofill/android/save_update_address_profile_prompt_controller_unittest.cc
@@ -33,6 +33,11 @@
 namespace autofill {
 
 namespace {
+
+using profile_ref = base::optional_ref<const AutofillProfile>;
+using ::testing::AllOf;
+using ::testing::Property;
+
 std::unique_ptr<KeyedService> CreateTestSyncService(
     content::BrowserContext* context) {
   return std::make_unique<syncer::TestSyncService>();
@@ -176,7 +181,7 @@ TEST_F(SaveUpdateAddressProfilePromptControllerTest,
   EXPECT_CALL(
       decision_callback_,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kAccepted,
-          profile_));
+          Property(&profile_ref::has_value, false)));
   controller_->OnUserAccepted(env_, mock_caller_);
 }
 
@@ -188,7 +193,7 @@ TEST_F(SaveUpdateAddressProfilePromptControllerTest,
   EXPECT_CALL(
       decision_callback_,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kDeclined,
-          profile_));
+          Property(&profile_ref::has_value, false)));
   controller_->OnUserDeclined(env_, mock_caller_);
 }
 
@@ -200,7 +205,7 @@ TEST_F(SaveUpdateAddressProfilePromptControllerTest,
 
   EXPECT_CALL(decision_callback_,
               Run(AutofillClient::SaveAddressProfileOfferUserDecision::kNever,
-                  profile_));
+                  Property(&profile_ref::has_value, false)));
   controller_->OnUserDeclined(env_, mock_caller_);
 }
 
@@ -213,7 +218,8 @@ TEST_F(SaveUpdateAddressProfilePromptControllerTest,
   EXPECT_CALL(
       decision_callback_,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kEditAccepted,
-          edited_profile));
+          AllOf(Property(&profile_ref::has_value, true),
+                Property(&profile_ref::value, edited_profile))));
   base::android::ScopedJavaLocalRef<jobject> edited_profile_java =
       edited_profile.CreateJavaObject(
           g_browser_process->GetApplicationLocale());
@@ -239,7 +245,7 @@ TEST_F(SaveUpdateAddressProfilePromptControllerTest,
 
   EXPECT_CALL(decision_callback_,
               Run(AutofillClient::SaveAddressProfileOfferUserDecision::kIgnored,
-                  profile_));
+                  Property(&profile_ref::has_value, false)));
   controller_.reset();
 }
 

diff --git a/chrome/browser/ui/android/autofill/save_update_address_profile_flow_manager.cc b/chrome/browser/ui/android/autofill/save_update_address_profile_flow_manager.cc
@@ -34,7 +34,7 @@ void SaveUpdateAddressProfileFlowManager::OfferSave(
       save_update_address_profile_prompt_controller_) {
     std::move(callback).Run(
         AutofillClient::SaveAddressProfileOfferUserDecision::kAutoDeclined,
-        profile);
+        std::nullopt);
     return;
   }
 
@@ -46,7 +46,7 @@ void SaveUpdateAddressProfileFlowManager::OfferSave(
     // Fallback to the default behavior without confirmation.
     std::move(callback).Run(
         AutofillClient::SaveAddressProfileOfferUserDecision::kUserNotAsked,
-        profile);
+        std::nullopt);
   }
 }
 

diff --git a/chrome/browser/ui/android/autofill/save_update_address_profile_flow_manager_browsertest.cc b/chrome/browser/ui/android/autofill/save_update_address_profile_flow_manager_browsertest.cc
@@ -16,6 +16,9 @@
 
 namespace autofill {
 
+using ::testing::Property;
+using profile_ref = base::optional_ref<const AutofillProfile>;
+
 class SaveUpdateAddressProfileFlowManagerBrowserTest
     : public AndroidBrowserTest {
  public:
@@ -74,7 +77,7 @@ IN_PROC_BROWSER_TEST_F(SaveUpdateAddressProfileFlowManagerBrowserTest,
   EXPECT_CALL(
       another_save_callback,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kAutoDeclined,
-          another_profile));
+          Property(&profile_ref::has_value, false)));
   flow_manager_->OfferSave(GetWebContents(), another_profile,
                            /*original_profile=*/nullptr, kNotMigrationToAccount,
                            another_save_callback.Get());
@@ -98,7 +101,7 @@ IN_PROC_BROWSER_TEST_F(SaveUpdateAddressProfileFlowManagerBrowserTest,
   EXPECT_CALL(
       another_save_callback,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kAutoDeclined,
-          another_profile));
+          Property(&profile_ref::has_value, false)));
   flow_manager_->OfferSave(GetWebContents(), another_profile,
                            /*original_profile=*/nullptr, kNotMigrationToAccount,
                            another_save_callback.Get());

diff --git a/chrome/browser/ui/autofill/edit_address_profile_dialog_controller.h b/chrome/browser/ui/autofill/edit_address_profile_dialog_controller.h
@@ -25,7 +25,7 @@ class EditAddressProfileDialogController {
   // including the edits performed by the user.
   virtual void OnDialogClosed(
       AutofillClient::SaveAddressProfileOfferUserDecision decision,
-      const AutofillProfile& profile_with_edits) = 0;
+      base::optional_ref<const AutofillProfile> profile_with_edits) = 0;
 };
 
 }  // namespace autofill

diff --git a/chrome/browser/ui/autofill/edit_address_profile_dialog_controller_impl.cc b/chrome/browser/ui/autofill/edit_address_profile_dialog_controller_impl.cc
@@ -91,7 +91,7 @@ bool EditAddressProfileDialogControllerImpl::GetIsValidatable() const {
 
 void EditAddressProfileDialogControllerImpl::OnDialogClosed(
     AutofillClient::SaveAddressProfileOfferUserDecision decision,
-    const AutofillProfile& profile_with_edits) {
+    base::optional_ref<const AutofillProfile> profile_with_edits) {
   std::move(on_user_decision_callback_).Run(decision, profile_with_edits);
   dialog_view_ = nullptr;
 }

diff --git a/chrome/browser/ui/autofill/edit_address_profile_dialog_controller_impl.h b/chrome/browser/ui/autofill/edit_address_profile_dialog_controller_impl.h
@@ -54,7 +54,7 @@ class EditAddressProfileDialogControllerImpl
   bool GetIsValidatable() const override;
   void OnDialogClosed(
       AutofillClient::SaveAddressProfileOfferUserDecision decision,
-      const AutofillProfile& profile_with_edits) override;
+      base::optional_ref<const AutofillProfile> profile_with_edits) override;
 
   // content::WebContentsObserver:
   void WebContentsDestroyed() override;

diff --git a/chrome/browser/ui/autofill/edit_address_profile_dialog_controller_impl_interactive_uitest.cc b/chrome/browser/ui/autofill/edit_address_profile_dialog_controller_impl_interactive_uitest.cc
@@ -43,19 +43,26 @@ class EditAddressProfileDialogControllerImplTest
 
   void OnUserDecision(
       AutofillClient::SaveAddressProfileOfferUserDecision decision,
-      AutofillProfile edited_profile) {
+      base::optional_ref<const AutofillProfile> edited_profile) {
     user_decision_ = decision;
-    edited_profile_ = edited_profile;
+    if (edited_profile.has_value()) {
+      edited_profile_ = edited_profile.value();
+    }
   }
 
   auto EnsureClosedWithDecisionAndProfile(
       AutofillClient::SaveAddressProfileOfferUserDecision
           expected_user_decision,
-      const AutofillProfile& expected_profile) {
+      base::optional_ref<const AutofillProfile> expected_profile) {
     return Steps(
         CheckResult([this]() { return user_decision_; },
                     expected_user_decision),
-        CheckResult([this]() { return edited_profile_; }, expected_profile));
+        Do([this, expected_profile]() {
+          ASSERT_EQ(edited_profile_.has_value(), expected_profile.has_value());
+          if (expected_profile.has_value()) {
+            EXPECT_EQ(edited_profile_.value(), expected_profile.value());
+          }
+        }));
   }
 
   auto ShowEditor(const AutofillProfile& profile,
@@ -88,7 +95,7 @@ class EditAddressProfileDialogControllerImplTest
   // prompt.
   AutofillClient::SaveAddressProfileOfferUserDecision user_decision_;
   AutofillProfile local_profile_;
-  AutofillProfile edited_profile_;
+  std::optional<AutofillProfile> edited_profile_;
 };
 
 IN_PROC_BROWSER_TEST_F(EditAddressProfileDialogControllerImplTest,

diff --git a/chrome/browser/ui/autofill/edit_address_profile_dialog_controller_impl_unittest.cc b/chrome/browser/ui/autofill/edit_address_profile_dialog_controller_impl_unittest.cc
@@ -19,6 +19,9 @@
 namespace autofill {
 
 using ::testing::_;
+using ::testing::AllOf;
+using ::testing::Property;
+using profile_ref = base::optional_ref<const AutofillProfile>;
 
 class EditAddressProfileDialogControllerImplTest
     : public BrowserWithTestWindowTest {
@@ -59,7 +62,7 @@ class EditAddressProfileDialogControllerImplTest
   TestAutofillBubble autofill_bubble_;
   base::MockOnceCallback<void(
       AutofillClient::SaveAddressProfileOfferUserDecision,
-      AutofillProfile profile)>
+      profile_ref profile)>
       save_callback_;
 };
 
@@ -76,30 +79,32 @@ TEST_F(EditAddressProfileDialogControllerImplTest,
        IgnoreDialog_CancelCallbackInvoked) {
   EXPECT_CALL(save_callback_,
               Run(AutofillClient::SaveAddressProfileOfferUserDecision::kIgnored,
-                  profile_));
+                  Property(&profile_ref::has_value, false)));
 
   controller()->OnDialogClosed(
-      AutofillClient::SaveAddressProfileOfferUserDecision::kIgnored, profile_);
+      AutofillClient::SaveAddressProfileOfferUserDecision::kIgnored,
+      std::nullopt);
 }
 
 TEST_F(EditAddressProfileDialogControllerImplTest,
        CancelEditing_CancelCallbackInvoked) {
   EXPECT_CALL(
       save_callback_,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kEditDeclined,
-          profile_));
+          Property(&profile_ref::has_value, false)));
 
   controller()->OnDialogClosed(
       AutofillClient::SaveAddressProfileOfferUserDecision::kEditDeclined,
-      profile_);
+      std::nullopt);
 }
 
 TEST_F(EditAddressProfileDialogControllerImplTest,
        SaveAddress_SaveCallbackInvoked) {
   EXPECT_CALL(
       save_callback_,
       Run(AutofillClient::SaveAddressProfileOfferUserDecision::kEditAccepted,
-          profile_));
+          AllOf(Property(&profile_ref::has_value, true),
+                Property(&profile_ref::value, profile_))));
 
   controller()->OnDialogClosed(
       AutofillClient::SaveAddressProfileOfferUserDecision::kEditAccepted,

diff --git a/chrome/browser/ui/autofill/save_update_address_profile_bubble_controller.h b/chrome/browser/ui/autofill/save_update_address_profile_bubble_controller.h
@@ -37,7 +37,7 @@ class SaveUpdateAddressProfileBubbleController {
   virtual const AutofillProfile* GetOriginalProfile() const = 0;
   virtual void OnUserDecision(
       AutofillClient::SaveAddressProfileOfferUserDecision decision,
-      AutofillProfile profile) = 0;
+      base::optional_ref<const AutofillProfile> profile) = 0;
   virtual void OnEditButtonClicked() = 0;
   virtual void OnBubbleClosed() = 0;
 };

diff --git a/chrome/browser/ui/autofill/save_update_address_profile_bubble_controller_impl.cc b/chrome/browser/ui/autofill/save_update_address_profile_bubble_controller_impl.cc
@@ -140,7 +140,7 @@ void SaveUpdateAddressProfileBubbleControllerImpl::OfferSave(
   if (bubble_view()) {
     std::move(address_profile_save_prompt_callback)
         .Run(AutofillClient::SaveAddressProfileOfferUserDecision::kAutoDeclined,
-             profile);
+             std::nullopt);
     return;
   }
   // If the user closed the bubble of the previous import process using the
@@ -153,7 +153,7 @@ void SaveUpdateAddressProfileBubbleControllerImpl::OfferSave(
   if (address_profile_save_prompt_callback_) {
     std::move(address_profile_save_prompt_callback_)
         .Run(AutofillClient::SaveAddressProfileOfferUserDecision::kIgnored,
-             address_profile_);
+             std::nullopt);
   }
 
   address_profile_ = profile;
@@ -325,7 +325,7 @@ SaveUpdateAddressProfileBubbleControllerImpl::GetOriginalProfile() const {
 
 void SaveUpdateAddressProfileBubbleControllerImpl::OnUserDecision(
     AutofillClient::SaveAddressProfileOfferUserDecision decision,
-    AutofillProfile profile) {
+    base::optional_ref<const AutofillProfile> profile) {
   if (decision ==
       AutofillClient::SaveAddressProfileOfferUserDecision::kEditDeclined) {
     // Reopen this bubble if the user canceled editing.
@@ -385,7 +385,7 @@ void SaveUpdateAddressProfileBubbleControllerImpl::WebContentsDestroyed() {
   AutofillBubbleControllerBase::WebContentsDestroyed();
 
   OnUserDecision(AutofillClient::SaveAddressProfileOfferUserDecision::kIgnored,
-                 address_profile_);
+                 std::nullopt);
 }
 
 PageActionIconType

diff --git a/chrome/browser/ui/autofill/save_update_address_profile_bubble_controller_impl.h b/chrome/browser/ui/autofill/save_update_address_profile_bubble_controller_impl.h
@@ -58,7 +58,7 @@ class SaveUpdateAddressProfileBubbleControllerImpl
   const AutofillProfile* GetOriginalProfile() const override;
   void OnUserDecision(
       AutofillClient::SaveAddressProfileOfferUserDecision decision,
-      AutofillProfile profile) override;
+      base::optional_ref<const AutofillProfile> profile) override;
   void OnEditButtonClicked() override;
   void OnBubbleClosed() override;
 

diff --git a/...wser/ui/autofill/save_update_address_profile_bubble_controller_impl_interactive_uitest.cc b/...wser/ui/autofill/save_update_address_profile_bubble_controller_impl_interactive_uitest.cc
@@ -51,7 +51,7 @@ class BaseSaveUpdateAddressProfileBubbleControllerImplTest
 
   void OnUserDecision(
       AutofillClient::SaveAddressProfileOfferUserDecision decision,
-      AutofillProfile profile) {
+      base::optional_ref<const AutofillProfile> profile) {
     user_decision_ = decision;
   }