[Autofill Offer] Add validations for synced data before saving them

Bug: 1112095
Change-Id: Idf4553fd4367378a5117900b5ab75cbbe1dfeee1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418013
Commit-Queue: Siyu An <siyua@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808560}

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

@@ -313,6 +313,7 @@ void SetAutofillOfferSpecificsFromOfferData(
 
 AutofillOfferData AutofillOfferDataFromOfferSpecifics(
     const sync_pb::AutofillOfferSpecifics& offer_specifics) {
+  DCHECK(IsOfferSpecificsValid(offer_specifics));
   AutofillOfferData offer_data;
   offer_data.offer_id = offer_specifics.id();
   if (offer_specifics.has_percentage_reward()) {
@@ -493,4 +494,40 @@ template bool AreAnyItemsDifferent<>(
     const std::vector<std::unique_ptr<CreditCardCloudTokenData>>&,
     const std::vector<CreditCardCloudTokenData>&);
 
+bool IsOfferSpecificsValid(const sync_pb::AutofillOfferSpecifics specifics) {
+  // A valid offer has a non-empty id.
+  if (!specifics.has_id())
+    return false;
+
+  // A valid offer has a non-empty offer details url and the url must be valid.
+  if (!specifics.has_offer_details_url() ||
+      !GURL(specifics.offer_details_url()).is_valid()) {
+    return false;
+  }
+
+  // A valid offer has at least one merchant domain.
+  if (specifics.merchant_domain().size() == 0) {
+    return false;
+  }
+
+  // A valid offer has at least one linked card instrument id.
+  if (!specifics.has_card_linked_offer_data() ||
+      specifics.card_linked_offer_data().instrument_id().size() == 0) {
+    return false;
+  }
+
+  // A valid offer must have either a percentage reward or a fixed amount
+  // reward.
+  if (!specifics.has_percentage_reward() ||
+      !specifics.percentage_reward().has_percentage()) {
+    return specifics.has_fixed_amount_reward() &&
+           specifics.fixed_amount_reward().has_amount();
+  } else if (specifics.percentage_reward().percentage().find('%') ==
+             std::string::npos) {
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace autofill

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

@@ -63,6 +63,7 @@ void SetAutofillOfferSpecificsFromOfferData(
     sync_pb::AutofillOfferSpecifics* offer_specifics);
 
 // Creates an AutofillOfferData from the specified |offer_specifics|.
+// |offer_specifics| must be valid (as per IsOfferSpecificsValid()).
 AutofillOfferData AutofillOfferDataFromOfferSpecifics(
     const sync_pb::AutofillOfferSpecifics& offer_specifics);
 
@@ -97,6 +98,9 @@ template <class Item>
 bool AreAnyItemsDifferent(const std::vector<std::unique_ptr<Item>>& old_data,
                           const std::vector<Item>& new_data);
 
+// Returns whether the Wallet Offer |specifics| is valid data.
+bool IsOfferSpecificsValid(const sync_pb::AutofillOfferSpecifics specifics);
+
 }  // namespace autofill
 
 #endif  // COMPONENTS_AUTOFILL_CORE_BROWSER_WEBDATA_AUTOFILL_SYNC_BRIDGE_UTIL_H_

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

@@ -304,5 +304,54 @@ TEST_F(AutofillSyncBridgeUtilTest,
   EXPECT_TRUE(AreAnyItemsDifferent(old_offer_data, new_offer_data));
 }
 
+// Ensures that function IsOfferSpecificsValid is working correctly.
+TEST_F(AutofillSyncBridgeUtilTest, IsOfferSpecificsValid) {
+  sync_pb::AutofillOfferSpecifics specifics;
+  SetAutofillOfferSpecificsFromOfferData(test::GetCardLinkedOfferData1(),
+                                         &specifics);
+  // Expects default specifics is valid.
+  EXPECT_TRUE(IsOfferSpecificsValid(specifics));
+
+  specifics.clear_id();
+  // Expects specifics without id to be invalid.
+  EXPECT_FALSE(IsOfferSpecificsValid(specifics));
+
+  SetAutofillOfferSpecificsFromOfferData(test::GetCardLinkedOfferData1(),
+                                         &specifics);
+  specifics.set_offer_details_url("invalid url");
+  // Expects specifics with invalid offer_details_url to be invalid.
+  EXPECT_FALSE(IsOfferSpecificsValid(specifics));
+  specifics.clear_offer_details_url();
+  // Expects specifics without offer_details_url to be invalid.
+  EXPECT_FALSE(IsOfferSpecificsValid(specifics));
+
+  SetAutofillOfferSpecificsFromOfferData(test::GetCardLinkedOfferData1(),
+                                         &specifics);
+  specifics.clear_merchant_domain();
+  // Expects specifics without merchant domain to be invalid.
+  EXPECT_FALSE(IsOfferSpecificsValid(specifics));
+
+  SetAutofillOfferSpecificsFromOfferData(test::GetCardLinkedOfferData1(),
+                                         &specifics);
+  specifics.mutable_card_linked_offer_data()->clear_instrument_id();
+  // Expects specifics without linked card instrument id to be invalid.
+  EXPECT_FALSE(IsOfferSpecificsValid(specifics));
+  specifics.clear_card_linked_offer_data();
+  // Expects specifics without card linked offer data to be invalid.
+  EXPECT_FALSE(IsOfferSpecificsValid(specifics));
+
+  SetAutofillOfferSpecificsFromOfferData(test::GetCardLinkedOfferData1(),
+                                         &specifics);
+  specifics.mutable_percentage_reward()->set_percentage("5");
+  // Expects specifics without correct reward text to be invalid.
+  EXPECT_FALSE(IsOfferSpecificsValid(specifics));
+  specifics.clear_percentage_reward();
+  // Expects specifics without reward text to be invalid.
+  EXPECT_FALSE(IsOfferSpecificsValid(specifics));
+  specifics.mutable_fixed_amount_reward()->set_amount("$5");
+  // Expects specifics with only fixed amount reward text to be valid.
+  EXPECT_TRUE(IsOfferSpecificsValid(specifics));
+}
+
 }  // namespace
 }  // namespace autofill

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

@@ -173,9 +173,14 @@ void AutofillWalletOfferSyncBridge::MergeRemoteData(
   std::vector<AutofillOfferData> offer_data;
   for (const std::unique_ptr<syncer::EntityChange>& change : entity_data) {
     DCHECK(change->data().specifics.has_autofill_offer());
-    // TODO(crbug.com/1112095): Add offer data validation.
-    offer_data.push_back(AutofillOfferDataFromOfferSpecifics(
-        change->data().specifics.autofill_offer()));
+    const sync_pb::AutofillOfferSpecifics specifics =
+        change->data().specifics.autofill_offer();
+    if (IsOfferSpecificsValid(specifics)) {
+      offer_data.push_back(AutofillOfferDataFromOfferSpecifics(specifics));
+    } else {
+      // TODO(crbug.com/1112095): Add logging to record how often invalid data
+      // arrives.
+    }
   }
 
   AutofillTable* table = GetAutofillTable();