Reland "[Autofill] Populate ChromeUserContext in Payments requests"

This reverts commit ca672c42.

Reason for revert: Root cause of trybot failures is still unclear.  Creating this revert to run trybots.

Original change's description:
> Revert "[Autofill] Populate ChromeUserContext in Payments requests"
> 
> This reverts commit d9183af5.
> 
> Reason for revert: suspected of causing https://bugs.chromium.org/p/chromium/issues/detail?id=881420
> 
> Original change's description:
> > [Autofill] Populate ChromeUserContext in Payments requests
> > 
> > This change adds |chrome_user_context| and |full_sync_enabled| values to
> > Unmask, GetDetails, Upload, and MigrateCards requests, as long as either
> > of the appropriate flags are enabled.
> > 
> > Bug: 840703
> > Change-Id: Ieb3baf980b68add43a3111c0902a03b574b1adcf
> > Reviewed-on: https://chromium-review.googlesource.com/1208434
> > Commit-Queue: Jared Saul <jsaul@google.com>
> > Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
> > Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#589060}
> 
> TBR=sebsg@chromium.org,jsaul@google.com,dlkumar@google.com
> 
> Change-Id: Id88b0a194dad400be5ba95a36f11d8ebb5ccd1ec
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 840703
> Reviewed-on: https://chromium-review.googlesource.com/1211282
> Reviewed-by: John Budorick <jbudorick@chromium.org>
> Commit-Queue: John Budorick <jbudorick@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589215}

TBR=sebsg@chromium.org,jsaul@google.com,jbudorick@chromium.org,dlkumar@google.com

Change-Id: Ibaab351513bef9c23a4da09cfd8c72514d49731a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 840703
Reviewed-on: https://chromium-review.googlesource.com/1211728Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Commit-Queue: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#589596}

components/autofill/core/browser/payments/full_card_request_unittest.cc

@@ -61,6 +61,7 @@ class MockPersonalDataManager : public PersonalDataManager {
  public:
   MockPersonalDataManager() : PersonalDataManager("en-US") {}
   ~MockPersonalDataManager() override {}
+  MOCK_CONST_METHOD0(IsSyncFeatureEnabled, bool());
   MOCK_METHOD1(UpdateCreditCard, void(const CreditCard& credit_card));
   MOCK_METHOD1(UpdateServerCreditCard, void(const CreditCard& credit_card));
 };

components/autofill/core/browser/payments/payments_client.cc

@@ -237,9 +237,12 @@ bool ShouldUseActiveSignedInAccount() {
 class UnmaskCardRequest : public PaymentsRequest {
  public:
   UnmaskCardRequest(const PaymentsClient::UnmaskRequestDetails& request_details,
+                    const bool full_sync_enabled,
                     base::OnceCallback<void(AutofillClient::PaymentsRpcResult,
                                             const std::string&)> callback)
-      : request_details_(request_details), callback_(std::move(callback)) {
+      : request_details_(request_details),
+        full_sync_enabled_(full_sync_enabled),
+        callback_(std::move(callback)) {
     DCHECK(
         CreditCard::MASKED_SERVER_CARD == request_details.card.record_type() ||
         CreditCard::FULL_SERVER_CARD == request_details.card.record_type());
@@ -267,6 +270,13 @@ class UnmaskCardRequest : public PaymentsRequest {
     }
     request_dict.Set("context", std::move(context));
 
+    if (ShouldUseActiveSignedInAccount()) {
+      std::unique_ptr<base::DictionaryValue> chrome_user_context(
+          new base::DictionaryValue());
+      chrome_user_context->SetBoolean("full_sync_enabled", full_sync_enabled_);
+      request_dict.Set("chrome_user_context", std::move(chrome_user_context));
+    }
+
     int value = 0;
     if (base::StringToInt(request_details_.user_response.exp_month, &value))
       request_dict.SetInteger("expiration_month", value);
@@ -297,6 +307,7 @@ class UnmaskCardRequest : public PaymentsRequest {
 
  private:
   PaymentsClient::UnmaskRequestDetails request_details_;
+  const bool full_sync_enabled_;
   base::OnceCallback<void(AutofillClient::PaymentsRpcResult,
                           const std::string&)>
       callback_;
@@ -309,6 +320,7 @@ class GetUploadDetailsRequest : public PaymentsRequest {
       const std::vector<AutofillProfile>& addresses,
       const int detected_values,
       const std::vector<const char*>& active_experiments,
+      const bool full_sync_enabled,
       const std::string& app_locale,
       base::OnceCallback<void(AutofillClient::PaymentsRpcResult,
                               const base::string16&,
@@ -317,6 +329,7 @@ class GetUploadDetailsRequest : public PaymentsRequest {
       : addresses_(addresses),
         detected_values_(detected_values),
         active_experiments_(active_experiments),
+        full_sync_enabled_(full_sync_enabled),
         app_locale_(app_locale),
         callback_(std::move(callback)),
         billable_service_number_(billable_service_number) {}
@@ -335,6 +348,13 @@ class GetUploadDetailsRequest : public PaymentsRequest {
     context->SetInteger("billable_service", billable_service_number_);
     request_dict.Set("context", std::move(context));
 
+    if (ShouldUseActiveSignedInAccount()) {
+      std::unique_ptr<base::DictionaryValue> chrome_user_context(
+          new base::DictionaryValue());
+      chrome_user_context->SetBoolean("full_sync_enabled", full_sync_enabled_);
+      request_dict.Set("chrome_user_context", std::move(chrome_user_context));
+    }
+
     std::unique_ptr<base::ListValue> addresses(new base::ListValue());
     for (const AutofillProfile& profile : addresses_) {
       // These addresses are used by Payments to (1) accurately determine the
@@ -380,6 +400,7 @@ class GetUploadDetailsRequest : public PaymentsRequest {
   const std::vector<AutofillProfile> addresses_;
   const int detected_values_;
   const std::vector<const char*> active_experiments_;
+  const bool full_sync_enabled_;
   std::string app_locale_;
   base::OnceCallback<void(AutofillClient::PaymentsRpcResult,
                           const base::string16&,
@@ -393,9 +414,12 @@ class GetUploadDetailsRequest : public PaymentsRequest {
 class UploadCardRequest : public PaymentsRequest {
  public:
   UploadCardRequest(const PaymentsClient::UploadRequestDetails& request_details,
+                    const bool full_sync_enabled,
                     base::OnceCallback<void(AutofillClient::PaymentsRpcResult,
                                             const std::string&)> callback)
-      : request_details_(request_details), callback_(std::move(callback)) {}
+      : request_details_(request_details),
+        full_sync_enabled_(full_sync_enabled),
+        callback_(std::move(callback)) {}
   ~UploadCardRequest() override {}
 
   std::string GetRequestUrlPath() override { return kUploadCardRequestPath; }
@@ -423,6 +447,13 @@ class UploadCardRequest : public PaymentsRequest {
     }
     request_dict.Set("context", std::move(context));
 
+    if (ShouldUseActiveSignedInAccount()) {
+      std::unique_ptr<base::DictionaryValue> chrome_user_context(
+          new base::DictionaryValue());
+      chrome_user_context->SetBoolean("full_sync_enabled", full_sync_enabled_);
+      request_dict.Set("chrome_user_context", std::move(chrome_user_context));
+    }
+
     SetStringIfNotEmpty(request_details_.card, CREDIT_CARD_NAME_FULL,
                         app_locale, "cardholder_name", &request_dict);
 
@@ -481,6 +512,7 @@ class UploadCardRequest : public PaymentsRequest {
 
  private:
   const PaymentsClient::UploadRequestDetails request_details_;
+  const bool full_sync_enabled_;
   base::OnceCallback<void(AutofillClient::PaymentsRpcResult,
                           const std::string&)>
       callback_;
@@ -492,9 +524,11 @@ class MigrateCardsRequest : public PaymentsRequest {
   MigrateCardsRequest(
       const PaymentsClient::MigrationRequestDetails& request_details,
       const std::vector<MigratableCreditCard>& migratable_credit_cards,
+      const bool full_sync_enabled,
       MigrateCardsCallback callback)
       : request_details_(request_details),
         migratable_credit_cards_(migratable_credit_cards),
+        full_sync_enabled_(full_sync_enabled),
         callback_(std::move(callback)) {}
   ~MigrateCardsRequest() override {}
 
@@ -522,6 +556,13 @@ class MigrateCardsRequest : public PaymentsRequest {
     }
     request_dict.Set("context", std::move(context));
 
+    if (ShouldUseActiveSignedInAccount()) {
+      std::unique_ptr<base::DictionaryValue> chrome_user_context(
+          new base::DictionaryValue());
+      chrome_user_context->SetBoolean("full_sync_enabled", full_sync_enabled_);
+      request_dict.Set("chrome_user_context", std::move(chrome_user_context));
+    }
+
     request_dict.SetString("context_token", request_details_.context_token);
 
     std::string all_pans_data = std::string();
@@ -597,6 +638,7 @@ class MigrateCardsRequest : public PaymentsRequest {
 
   const PaymentsClient::MigrationRequestDetails request_details_;
   const std::vector<MigratableCreditCard>& migratable_credit_cards_;
+  const bool full_sync_enabled_;
   MigrateCardsCallback callback_;
   std::unique_ptr<std::unordered_map<std::string, std::string>> save_result_;
   std::string display_text_;
@@ -652,7 +694,9 @@ void PaymentsClient::UnmaskCard(
     base::OnceCallback<void(AutofillClient::PaymentsRpcResult,
                             const std::string&)> callback) {
   IssueRequest(
-      std::make_unique<UnmaskCardRequest>(request_details, std::move(callback)),
+      std::make_unique<UnmaskCardRequest>(
+          request_details, account_info_getter_->IsSyncFeatureEnabled(),
+          std::move(callback)),
       true);
 }
 
@@ -666,7 +710,8 @@ void PaymentsClient::GetUploadDetails(
                             std::unique_ptr<base::DictionaryValue>)> callback,
     const int billable_service_number) {
   IssueRequest(std::make_unique<GetUploadDetailsRequest>(
-                   addresses, detected_values, active_experiments, app_locale,
+                   addresses, detected_values, active_experiments,
+                   account_info_getter_->IsSyncFeatureEnabled(), app_locale,
                    std::move(callback), billable_service_number),
                false);
 }
@@ -676,7 +721,9 @@ void PaymentsClient::UploadCard(
     base::OnceCallback<void(AutofillClient::PaymentsRpcResult,
                             const std::string&)> callback) {
   IssueRequest(
-      std::make_unique<UploadCardRequest>(request_details, std::move(callback)),
+      std::make_unique<UploadCardRequest>(
+          request_details, account_info_getter_->IsSyncFeatureEnabled(),
+          std::move(callback)),
       true);
 }
 
@@ -686,7 +733,8 @@ void PaymentsClient::MigrateCards(
     MigrateCardsCallback callback) {
   IssueRequest(
       std::make_unique<MigrateCardsRequest>(
-          request_details, migratable_credit_cards, std::move(callback)),
+          request_details, migratable_credit_cards,
+          account_info_getter_->IsSyncFeatureEnabled(), std::move(callback)),
       /*authenticate=*/true);
 }
 

components/autofill/core/browser/payments/payments_client_unittest.cc

@@ -321,6 +321,51 @@ TEST_F(PaymentsClientTest, UnmaskSuccessAccountFromSyncTest) {
   EXPECT_EQ("1234", real_pan_);
 }
 
+TEST_F(PaymentsClientTest, UnmaskIncludesChromeUserContext) {
+  scoped_feature_list_.InitWithFeatures(
+      {features::kAutofillGetPaymentsIdentityFromSync},  // Enabled
+      {features::kAutofillEnableAccountWalletStorage});  // Disabled
+
+  StartUnmasking();
+  IssueOAuthToken();
+  ReturnResponse(net::HTTP_OK, "{}");
+
+  // ChromeUserContext was set.
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest,
+       UnmaskIncludesChromeUserContextIfWalletStorageFlagEnabled) {
+  scoped_feature_list_.InitWithFeatures(
+      {features::kAutofillEnableAccountWalletStorage},    // Enabled
+      {features::kAutofillGetPaymentsIdentityFromSync});  // Disabled
+
+  StartUnmasking();
+  IssueOAuthToken();
+  ReturnResponse(net::HTTP_OK, "{}");
+
+  // ChromeUserContext was set.
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest, UnmaskExcludesChromeUserContextIfExperimentsOff) {
+  scoped_feature_list_.InitWithFeatures(
+      {},  // Enabled
+      {features::kAutofillEnableAccountWalletStorage,
+       features::kAutofillGetPaymentsIdentityFromSync});  // Disabled
+
+  StartUnmasking();
+  IssueOAuthToken();
+  ReturnResponse(net::HTTP_OK, "{}");
+
+  // ChromeUserContext was not set.
+  EXPECT_TRUE(!GetUploadData().empty());
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") == std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") == std::string::npos);
+}
+
 TEST_F(PaymentsClientTest, GetDetailsSuccess) {
   StartGettingUploadDetails();
   ReturnResponse(
@@ -363,6 +408,46 @@ TEST_F(PaymentsClientTest, GetDetailsIncludesDetectedValuesInRequest) {
               std::string::npos);
 }
 
+TEST_F(PaymentsClientTest, GetDetailsIncludesChromeUserContext) {
+  scoped_feature_list_.InitWithFeatures(
+      {features::kAutofillGetPaymentsIdentityFromSync},  // Enabled
+      {features::kAutofillEnableAccountWalletStorage});  // Disabled
+
+  StartGettingUploadDetails();
+
+  // ChromeUserContext was set.
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest,
+       GetDetailsIncludesChromeUserContextIfWalletStorageFlagEnabled) {
+  scoped_feature_list_.InitWithFeatures(
+      {features::kAutofillEnableAccountWalletStorage},    // Enabled
+      {features::kAutofillGetPaymentsIdentityFromSync});  // Disabled
+
+  StartGettingUploadDetails();
+
+  // ChromeUserContext was set.
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest,
+       GetDetailsExcludesChromeUserContextIfExperimentsOff) {
+  scoped_feature_list_.InitWithFeatures(
+      {},  // Enabled
+      {features::kAutofillEnableAccountWalletStorage,
+       features::kAutofillGetPaymentsIdentityFromSync});  // Disabled
+
+  StartGettingUploadDetails();
+
+  // ChromeUserContext was not set.
+  EXPECT_TRUE(!GetUploadData().empty());
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") == std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") == std::string::npos);
+}
+
 TEST_F(PaymentsClientTest, GetUploadAccountFromSyncTest) {
   EnableAutofillGetPaymentsIdentityFromSync();
   // Set up a different account.
@@ -600,6 +685,48 @@ TEST_F(PaymentsClientTest, UploadIncludesCvcInRequestIfProvided) {
   EXPECT_TRUE(GetUploadData().find("&s7e_13_cvc=") != std::string::npos);
 }
 
+TEST_F(PaymentsClientTest, UploadIncludesChromeUserContext) {
+  scoped_feature_list_.InitWithFeatures(
+      {features::kAutofillGetPaymentsIdentityFromSync},  // Enabled
+      {features::kAutofillEnableAccountWalletStorage});  // Disabled
+
+  StartUploading(/*include_cvc=*/true);
+  IssueOAuthToken();
+
+  // ChromeUserContext was set.
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest,
+       UploadIncludesChromeUserContextIfWalletStorageFlagEnabled) {
+  scoped_feature_list_.InitWithFeatures(
+      {features::kAutofillEnableAccountWalletStorage},    // Enabled
+      {features::kAutofillGetPaymentsIdentityFromSync});  // Disabled
+
+  StartUploading(/*include_cvc=*/true);
+  IssueOAuthToken();
+
+  // ChromeUserContext was set.
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest, UploadExcludesChromeUserContextIfExperimentsOff) {
+  scoped_feature_list_.InitWithFeatures(
+      {},  // Enabled
+      {features::kAutofillEnableAccountWalletStorage,
+       features::kAutofillGetPaymentsIdentityFromSync});  // Disabled
+
+  StartUploading(/*include_cvc=*/true);
+  IssueOAuthToken();
+
+  // ChromeUserContext was not set.
+  EXPECT_TRUE(!GetUploadData().empty());
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") == std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") == std::string::npos);
+}
+
 TEST_F(PaymentsClientTest, UploadDoesNotIncludeCvcInRequestIfNotProvided) {
   StartUploading(/*include_cvc=*/false);
   IssueOAuthToken();
@@ -675,6 +802,49 @@ TEST_F(PaymentsClientTest,
   EXPECT_TRUE(GetUploadData().find("cardholder_name") == std::string::npos);
 }
 
+TEST_F(PaymentsClientTest, MigrationRequestIncludesChromeUserContext) {
+  scoped_feature_list_.InitWithFeatures(
+      {features::kAutofillGetPaymentsIdentityFromSync},  // Enabled
+      {features::kAutofillEnableAccountWalletStorage});  // Disabled
+
+  StartMigrating(/*uncheck_last_card=*/false, /*has_cardholder_name=*/true);
+  IssueOAuthToken();
+
+  // ChromeUserContext was set.
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest,
+       MigrationRequestIncludesChromeUserContextIfWalletStorageFlagEnabled) {
+  scoped_feature_list_.InitWithFeatures(
+      {features::kAutofillEnableAccountWalletStorage},    // Enabled
+      {features::kAutofillGetPaymentsIdentityFromSync});  // Disabled
+
+  StartMigrating(/*uncheck_last_card=*/false, /*has_cardholder_name=*/true);
+  IssueOAuthToken();
+
+  // ChromeUserContext was set.
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") != std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") != std::string::npos);
+}
+
+TEST_F(PaymentsClientTest,
+       MigrationRequestExcludesChromeUserContextIfExperimentsOff) {
+  scoped_feature_list_.InitWithFeatures(
+      {},  // Enabled
+      {features::kAutofillEnableAccountWalletStorage,
+       features::kAutofillGetPaymentsIdentityFromSync});  // Disabled
+
+  StartMigrating(/*uncheck_last_card=*/false, /*has_cardholder_name=*/true);
+  IssueOAuthToken();
+
+  // ChromeUserContext was not set.
+  EXPECT_TRUE(!GetUploadData().empty());
+  EXPECT_TRUE(GetUploadData().find("chrome_user_context") == std::string::npos);
+  EXPECT_TRUE(GetUploadData().find("full_sync_enabled") == std::string::npos);
+}
+
 TEST_F(PaymentsClientTest, MigrationSuccessWithSaveResult) {
   StartMigrating(/*uncheck_last_card=*/false, /*has_cardholder_name=*/true);
   IssueOAuthToken();