[Autofill Offer] Fixed missing notification when offer data changed
The issue is the offer data is only updated when relaunch (or when
personal_data_manager::refresh() is invoked) and not updated when
new offer reaches client.
The reason is:
The full sync flow should be:
1. Offer sync bridge receive the data and populates to autofill table
if found difference.
2. Notify UI sequence (personal data manager) about the change.
3. Personal data manager sends query to autofill webdata service to
get data from autofill table.
4. Personal data manager gets notified by OnWebDataServiceRequestDone
and refreshes local cache.
5. Autofill Offer Manager being a personal data manager observer
gets notified.
6. Autofill Offer manager updates eligible domain calling
UpdateEligibleMerchantDomains.
Previously I thought the OnWebDataServiceRequestDone would be
automatically invoked when the sync is finished, so we miss step 2
and 3. Here in this CL fix it.
(cherry picked from commit c20eb6ac1db2e94b7975a763be0fb63f866ea2a9)
Bug: 1135790
Change-Id: I38ac61f3d5608eec1c8ac92538e457e566b3f538
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2444454
Reviewed-by: Marc Treib <[email protected]>
Commit-Queue: Siyu An <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#814321}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2459057
Reviewed-by: Siyu An <[email protected]>
Cr-Commit-Position: refs/branch-heads/4280@{#111}
Cr-Branched-From: ea420fb963f9658c9969b6513c56b8f47efa1a2a-refs/heads/master@{#812852}
diff --git a/components/autofill/core/browser/webdata/autofill_wallet_offer_sync_bridge.cc b/components/autofill/core/browser/webdata/autofill_wallet_offer_sync_bridge.cc
index 1cf3e41..c95389db 100644
--- a/components/autofill/core/browser/webdata/autofill_wallet_offer_sync_bridge.cc
+++ b/components/autofill/core/browser/webdata/autofill_wallet_offer_sync_bridge.cc
@@ -190,8 +190,10 @@
std::vector<std::unique_ptr<AutofillOfferData>> existing_offers;
table->GetCreditCardOffers(&existing_offers);
- if (AreAnyItemsDifferent(existing_offers, offer_data))
+ bool offer_data_changed = AreAnyItemsDifferent(existing_offers, offer_data);
+ if (offer_data_changed) {
table->SetCreditCardOffers(offer_data);
+ }
// Commit the transaction to make sure the data and the metadata with the
// new progress marker is written down (especially on Android where we
@@ -199,6 +201,11 @@
// even if the wallet data has not changed because the model type state incl.
// the progress marker always changes.
web_data_backend_->CommitChanges();
+
+ if (offer_data_changed) {
+ // TODO(crbug.com/1112095): Add enum to indicate what actually changed.
+ web_data_backend_->NotifyOfMultipleAutofillChanges();
+ }
}
AutofillTable* AutofillWalletOfferSyncBridge::GetAutofillTable() {
diff --git a/components/autofill/core/browser/webdata/autofill_wallet_offer_sync_bridge_unittest.cc b/components/autofill/core/browser/webdata/autofill_wallet_offer_sync_bridge_unittest.cc
index b7ac73d..3132670 100644
--- a/components/autofill/core/browser/webdata/autofill_wallet_offer_sync_bridge_unittest.cc
+++ b/components/autofill/core/browser/webdata/autofill_wallet_offer_sync_bridge_unittest.cc
@@ -244,6 +244,7 @@
&offer_specifics);
EXPECT_CALL(*backend(), CommitChanges());
+ EXPECT_CALL(*backend(), NotifyOfMultipleAutofillChanges());
StartSyncing({offer_specifics});
// Only the server offer should be present on the client.
@@ -259,6 +260,7 @@
table()->SetCreditCardOffers({client_data});
EXPECT_CALL(*backend(), CommitChanges());
+ EXPECT_CALL(*backend(), NotifyOfMultipleAutofillChanges());
StartSyncing({});
EXPECT_TRUE(GetAllLocalData().empty());
@@ -275,6 +277,7 @@
offer_specifics2.clear_id();
EXPECT_CALL(*backend(), CommitChanges());
+ EXPECT_CALL(*backend(), NotifyOfMultipleAutofillChanges());
base::HistogramTester histogram_tester;
StartSyncing({offer_specifics1, offer_specifics2});
@@ -292,6 +295,7 @@
table()->SetCreditCardOffers({client_data});
EXPECT_CALL(*backend(), CommitChanges());
+ EXPECT_CALL(*backend(), NotifyOfMultipleAutofillChanges());
// Passing in a non-null metadata change list indicates to the bridge that
// sync is stopping but the data type is not disabled.
@@ -311,6 +315,7 @@
// We do not write to DB at all, so we should not commit any changes.
EXPECT_CALL(*backend(), CommitChanges()).Times(0);
+ EXPECT_CALL(*backend(), NotifyOfMultipleAutofillChanges()).Times(0);
// Passing in a null metadata change list indicates to the bridge that
// sync is stopping and the data type is disabled.
