[AF Wallet] Notify changes in PaymentsCustomerData to WebDataBackend

AutofillWalletSyncBridge did not notify changes in PaymentsCustomerData
to WebDataBackend. This was a bug because PersonalDataManager relies on
such notifications.

This CL fixes the bug (and also aligns the behaviour of the bridge to
to other wallet data so that it writes changes only when needed).

Bug: 878697
Change-Id: Id69c0520f192e401380c1eb2dbd55d28f25770b6
Reviewed-on: https://chromium-review.googlesource.com/1196382Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587552}

components/autofill/core/browser/payments/payments_customer_data.h

@@ -17,6 +17,9 @@ struct PaymentsCustomerData {
   bool operator==(const PaymentsCustomerData& other) const {
     return customer_id == other.customer_id;
   }
+  bool operator!=(const PaymentsCustomerData& other) const {
+    return !(*this == other);
+  }
 
   // The identifier by which a Google Payments account is identified.
   std::string customer_id;
@@ -24,4 +27,4 @@ struct PaymentsCustomerData {
 
 }  // namespace autofill
 
-#endif  // COMPONENTS_AUTOFILL_CORE_BROWSER_PAYMENTS_PAYMENTS_CUSTOMER_DATA_H_
+#endif  // COMPONENTS_AUTOFILL_CORE_BROWSER_PAYMENTS_PAYMENTS_CUSTOMER_DATA_H_
\ No newline at end of file

components/autofill/core/browser/webdata/autofill_wallet_sync_bridge.cc

@@ -208,21 +208,7 @@ void AutofillWalletSyncBridge::SetSyncData(
 
   // In both cases, we need to update wallet cards and payments customer data.
   wallet_data_changed |= SetWalletCards(std::move(wallet_cards));
-
-  // Changes to payments customer data do not have to be notified to the backend
-  // so we do not need to load the previous value before writing.
-  if (customer_data.empty()) {
-    // Clear the data in the DB.
-    GetAutofillTable()->SetPaymentsCustomerData(nullptr);
-  } else {
-    // In case there were multiple entries (and there shouldn't!), we take the
-    // pointer to the first entry in the vector.
-    GetAutofillTable()->SetPaymentsCustomerData(customer_data.data());
-    if (customer_data.size() > 1) {
-      DLOG(WARNING) << "Sync wallet_data update has " << customer_data.size()
-                    << " payments-customer-data entries; expected 0 or 1.";
-    }
-  }
+  wallet_data_changed |= SetPaymentsCustormerData(std::move(customer_data));
 
   if (web_data_backend_ && wallet_data_changed)
     web_data_backend_->NotifyOfMultipleAutofillChanges();
@@ -291,6 +277,40 @@ bool AutofillWalletSyncBridge::SetWalletAddresses(
   return false;
 }
 
+bool AutofillWalletSyncBridge::SetPaymentsCustormerData(
+    std::vector<PaymentsCustomerData> customer_data) {
+  // In the common case, the database won't have changed. Committing an update
+  // to the database will require at least one DB page write and will schedule
+  // a fsync. To avoid this I/O, it should be more efficient to do a read and
+  // only do the writes if something changed.
+  AutofillTable* table = GetAutofillTable();
+  std::unique_ptr<PaymentsCustomerData> existing_entry;
+  table->GetPaymentsCustomerData(&existing_entry);
+
+  // In case there were multiple entries (and there shouldn't!), we take the
+  // pointer to the first entry in the vector.
+  PaymentsCustomerData* new_entry =
+      customer_data.empty() ? nullptr : customer_data.data();
+
+#if DCHECK_IS_ON()
+  if (customer_data.size() > 1) {
+    DLOG(WARNING) << "Sync wallet_data update has " << customer_data.size()
+                  << " payments-customer-data entries; expected 0 or 1.";
+  }
+#endif  // DCHECK_IS_ON()
+
+  if (!new_entry && existing_entry) {
+    // Clear the existing entry in the DB.
+    GetAutofillTable()->SetPaymentsCustomerData(nullptr);
+    return true;
+  } else if (new_entry && (!existing_entry || *new_entry != *existing_entry)) {
+    // Write the new entry in the DB as it differs from the existing one.
+    GetAutofillTable()->SetPaymentsCustomerData(new_entry);
+    return true;
+  }
+  return false;
+}
+
 // static
 template <class Item>
 AutofillWalletSyncBridge::AutofillWalletDiff

components/autofill/core/browser/webdata/autofill_wallet_sync_bridge.h

@@ -24,6 +24,7 @@ class AutofillTable;
 class AutofillWebDataBackend;
 class AutofillWebDataService;
 class CreditCard;
+struct PaymentsCustomerData;
 
 // Sync bridge responsible for propagating local changes to the processor and
 // applying remote changes to the local database.
@@ -85,6 +86,11 @@ class AutofillWalletSyncBridge : public base::SupportsUserData::Data,
   // |wallet_addresses| was different from local data).
   bool SetWalletAddresses(std::vector<AutofillProfile> wallet_addresses);
 
+  // Sets |customer_data| to this client and returns whether any change has been
+  // applied (i.e., whether |customer_data| was different from local data).
+  bool SetPaymentsCustormerData(
+      std::vector<PaymentsCustomerData> customer_data);
+
   // Computes a "diff" (items added, items removed) of two vectors of items,
   // which should be either CreditCard or AutofillProfile. This is used for two
   // purposes: