[Passwords] Stop uploading raw field metadata on Canary/Dev.

Despite metadata pipeline was launched some time ago, uploading raw
metadata (field names, ids & autocomplete attributes) on early
channels was not deprecated, so it should be deprecated now.

Bug: b/307891115
Change-Id: I14186a6aa544e7edf771aaa2819a10964cec7ef9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4979868
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Maria Kazinova <kazinova@google.com>
Reviewed-by: Christoph Schwering <schwering@google.com>
Cr-Commit-Position: refs/heads/main@{#1216895}

chrome/browser/autofill/autofill_server_browsertest.cc

@@ -292,14 +292,10 @@ IN_PROC_BROWSER_TEST_F(AutofillServerTest,
 
   // Enabling raw form data uploading (e.g., field name) is too complicated in
   // this test. So, don't expect it in the upload.
-  test::FillUploadField(upload->add_field(), 2594484045U, nullptr, nullptr,
-                        nullptr, 2U);
-  test::FillUploadField(upload->add_field(), 2750915947U, nullptr, nullptr,
-                        nullptr, 2U);
-  test::FillUploadField(upload->add_field(), 3494787134U, nullptr, nullptr,
-                        nullptr, 2U);
-  test::FillUploadField(upload->add_field(), 1236501728U, nullptr, nullptr,
-                        nullptr, 2U);
+  test::FillUploadField(upload->add_field(), 2594484045U, 2U);
+  test::FillUploadField(upload->add_field(), 2750915947U, 2U);
+  test::FillUploadField(upload->add_field(), 3494787134U, 2U);
+  test::FillUploadField(upload->add_field(), 1236501728U, 2U);
 
   WindowedNetworkObserver upload_network_observer(EqualsUploadProto(request));
   content::WebContents* web_contents =

components/autofill/core/browser/autofill_download_manager.cc

@@ -513,13 +513,6 @@ absl::optional<std::string> GetAPIQueryPayload(
   return std::move(payload);
 }
 
-// Raw metadata uploading enabled iff this Chrome instance is on Canary or Dev
-// channel.
-bool IsRawMetadataUploadingEnabled(version_info::Channel channel) {
-  return channel == version_info::Channel::CANARY ||
-         channel == version_info::Channel::DEV;
-}
-
 std::string GetAPIKeyForUrl(version_info::Channel channel) {
   // First look if we can get API key from command line flag.
   const base::CommandLine& command_line =
@@ -562,22 +555,18 @@ AutofillDownloadManager::AutofillDownloadManager(AutofillClient* client,
                                                  LogManager* log_manager)
     : AutofillDownloadManager(client,
                               GetAPIKeyForUrl(channel),
-                              IsRawMetadataUploadingEnabled(channel),
                               log_manager) {}
 
-AutofillDownloadManager::AutofillDownloadManager(
-    AutofillClient* client,
-    const std::string& api_key,
-    bool is_raw_metadata_uploading_enabled,
-    LogManager* log_manager)
+AutofillDownloadManager::AutofillDownloadManager(AutofillClient* client,
+                                                 const std::string& api_key,
+                                                 LogManager* log_manager)
     : client_(client),
       api_key_(api_key),
       log_manager_(log_manager),
       autofill_server_url_(GetAutofillServerURL()),
       throttle_reset_period_(GetThrottleResetPeriod()),
       max_form_cache_size_(kAutofillDownloadManagerMaxFormCacheSize),
-      loader_backoff_(&kAutofillBackoffPolicy),
-      is_raw_metadata_uploading_enabled_(is_raw_metadata_uploading_enabled) {}
+      loader_backoff_(&kAutofillBackoffPolicy) {}
 
 AutofillDownloadManager::~AutofillDownloadManager() = default;
 
@@ -717,9 +706,9 @@ bool AutofillDownloadManager::StartUploadRequest(
     return StartRequest(std::move(request_data));
   };
 
-  std::vector<AutofillUploadContents> uploads = form.EncodeUploadRequest(
-      available_field_types, form_was_autofilled, login_form_signature,
-      observed_submission, is_raw_metadata_uploading_enabled_);
+  std::vector<AutofillUploadContents> uploads =
+      form.EncodeUploadRequest(available_field_types, form_was_autofilled,
+                               login_form_signature, observed_submission);
   bool all_succeeded = !uploads.empty();
   for (AutofillUploadContents& upload : uploads)
     all_succeeded &= Upload(std::move(upload));

components/autofill/core/browser/autofill_download_manager.h

@@ -129,7 +129,6 @@ class AutofillDownloadManager {
  protected:
   AutofillDownloadManager(AutofillClient* client,
                           const std::string& api_key,
-                          bool is_raw_metadata_uploading_enabled,
                           LogManager* log_manager);
 
   // Gets the length of the payload from request data. Used to simulate
@@ -225,10 +224,6 @@ class AutofillDownloadManager {
   // Used for exponential backoff of requests.
   net::BackoffEntry loader_backoff_;
 
-  // Whether form data (e.g. form and field names) can be uploaded in clear
-  // text.
-  bool is_raw_metadata_uploading_enabled_ = false;
-
   base::WeakPtrFactory<AutofillDownloadManager> weak_factory_{this};
 };
 

components/autofill/core/browser/autofill_download_manager_unittest.cc

@@ -161,7 +161,6 @@ class AutofillDownloadManagerWithCustomPayloadSize
                                                size_t length)
       : AutofillDownloadManager(client,
                                 api_key,
-                                /*is_raw_metadata_uploading_enabled=*/false,
                                 /*log_manager=*/nullptr),
         length_(length) {}
   ~AutofillDownloadManagerWithCustomPayloadSize() override = default;
@@ -204,14 +203,10 @@ class AutofillDownloadManagerTest : public AutofillDownloadManager::Observer,
 
   class TestAutofillDownloadManager : public AutofillDownloadManager {
    public:
-    explicit TestAutofillDownloadManager(
-        AutofillClient* client,
-        std::string api_key = "",
-        bool is_raw_metadata_uploading_enabled = false)
+    explicit TestAutofillDownloadManager(AutofillClient* client,
+                                         std::string api_key = "")
         : AutofillDownloadManager(client,
                                   /*api_key=*/std::move(api_key),
-                                  /*is_raw_metadata_uploading_enabled=*/
-                                  is_raw_metadata_uploading_enabled,
                                   /*log_manager=*/nullptr) {}
   };
 
@@ -221,7 +216,6 @@ class AutofillDownloadManagerTest : public AutofillDownloadManager::Observer,
                 &test_url_loader_factory_)),
         download_manager_(&client_,
                           /*api_key=*/"",
-                          /*is_raw_metadata_uploading_enabled=*/false,
                           /*log_manager=*/nullptr),
         pref_service_(test::PrefServiceForTesting()) {
     client_.set_shared_url_loader_factory(test_shared_loader_factory_);
@@ -379,7 +373,6 @@ TEST_F(AutofillDownloadManagerTest, QueryAndUploadTest) {
   // Make download manager.
   AutofillDownloadManager download_manager(
       &client_, "dummykey",
-      /*is_raw_metadata_uploading_enabled=*/false,
       /*log_manager=*/nullptr);
 
   // Request with id 0.
@@ -805,82 +798,6 @@ TEST_F(AutofillDownloadManagerTest, UploadToAPITest) {
                               net::HTTP_OK, 1);
 }
 
-TEST_F(AutofillDownloadManagerTest, UploadWithRawMetadata) {
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitWithFeatures(
-      // Enabled
-      {},
-      // Disabled
-      // We don't want upload throttling for testing purpose.
-      {features::test::kAutofillUploadThrottling});
-
-  for (bool is_raw_metadata_uploading_enabled : {false, true}) {
-    SCOPED_TRACE(testing::Message() << "is_raw_metadata_uploading_enabled = "
-                                    << is_raw_metadata_uploading_enabled);
-    // Build the form structures that we want to upload.
-    FormData form;
-    form.name = u"form1";
-    FormFieldData field;
-
-    field.name = u"firstname";
-    field.form_control_type = FormControlType::kInputText;
-    form.fields.push_back(field);
-
-    field.name = u"lastname";
-    field.form_control_type = FormControlType::kInputText;
-    form.fields.push_back(field);
-    FormStructure form_structure(form);
-    form_structure.set_submission_source(SubmissionSource::FORM_SUBMISSION);
-    for (auto& fs_field : form_structure)
-      fs_field->host_form_signature = form_structure.form_signature();
-
-    std::unique_ptr<PrefService> pref_service = test::PrefServiceForTesting();
-    TestAutofillDownloadManager download_manager(
-        &client_, "dummykey",
-        /*is_raw_metadata_uploading_enabled=*/
-        is_raw_metadata_uploading_enabled);
-    EXPECT_TRUE(download_manager.StartUploadRequest(
-        form_structure, true, ServerFieldTypeSet(), "", true,
-        pref_service.get(), GetWeakPtr()));
-
-    // Inspect the request that the test URL loader sent.
-    ASSERT_EQ(1, test_url_loader_factory_.NumPending());
-    network::TestURLLoaderFactory::PendingRequest* request =
-        test_url_loader_factory_.GetPendingRequest(0);
-
-    // Assert some of the fields within the uploaded proto to make sure it was
-    // filled with something else than default data.
-    AutofillUploadRequest autofill_upload_request;
-    EXPECT_TRUE(
-        GetUploadRequestProtoFromRequest(request, &autofill_upload_request));
-    AutofillUploadContents upload = autofill_upload_request.upload();
-    EXPECT_GT(upload.client_version().size(), 0U);
-    EXPECT_EQ(FormSignature(upload.form_signature()),
-              form_structure.form_signature());
-    // Only a few strings are tested, full testing happens in FormStructure's
-    // tests.
-    ASSERT_EQ(is_raw_metadata_uploading_enabled, upload.has_form_name());
-    ASSERT_EQ(is_raw_metadata_uploading_enabled, upload.field()[0].has_name());
-    ASSERT_EQ(is_raw_metadata_uploading_enabled, upload.field()[1].has_type());
-    if (is_raw_metadata_uploading_enabled) {
-      EXPECT_EQ(form.name, UTF8ToUTF16(upload.form_name()));
-      EXPECT_EQ(form.fields[0].name, UTF8ToUTF16(upload.field()[0].name()));
-      EXPECT_EQ(FormControlTypeToString(form.fields[1].form_control_type),
-                upload.field()[1].type());
-    }
-
-    test_url_loader_factory_.SimulateResponseForPendingRequest(
-        request->request.url.spec(), "");
-    EXPECT_EQ(1U, responses_.size());
-    EXPECT_EQ(AutofillDownloadManagerTest::UPLOAD_SUCCESSFULL,
-              responses_.front().type_of_response);
-
-    ASSERT_EQ(0, test_url_loader_factory_.NumPending());
-    test_url_loader_factory_.ClearResponses();
-    responses_.clear();
-  }
-}
-
 TEST_F(AutofillDownloadManagerTest, BackoffLogic_Query) {
   FormData form;
   FormFieldData field;

components/autofill/core/browser/autofill_manager_unittest.cc

@@ -55,7 +55,6 @@ class MockAutofillDownloadManager : public AutofillDownloadManager {
   explicit MockAutofillDownloadManager(AutofillClient* client)
       : AutofillDownloadManager(client,
                                 /*api_key=*/"",
-                                /*is_raw_metadata_uploading_enabled=*/false,
                                 /*log_manager=*/nullptr) {}
 
   MockAutofillDownloadManager(const MockAutofillDownloadManager&) = delete;

components/autofill/core/browser/autofill_test_utils.cc

@@ -833,29 +833,11 @@ void InitializePossibleTypesAndValidities(
   }
 }
 
-void BasicFillUploadField(AutofillUploadContents::Field* field,
-                          unsigned signature,
-                          const char* name,
-                          const char* control_type,
-                          const char* autocomplete) {
-  field->set_signature(signature);
-  if (name)
-    field->set_name(name);
-  if (control_type)
-    field->set_type(control_type);
-  if (autocomplete)
-    field->set_autocomplete(autocomplete);
-}
-
 void FillUploadField(AutofillUploadContents::Field* field,
                      unsigned signature,
-                     const char* name,
-                     const char* control_type,
-                     const char* autocomplete,
                      unsigned autofill_type,
                      unsigned validity_state) {
-  BasicFillUploadField(field, signature, name, control_type, autocomplete);
-
+  field->set_signature(signature);
   field->add_autofill_type(autofill_type);
 
   auto* type_validities = field->add_autofill_type_validities();
@@ -865,12 +847,9 @@ void FillUploadField(AutofillUploadContents::Field* field,
 
 void FillUploadField(AutofillUploadContents::Field* field,
                      unsigned signature,
-                     const char* name,
-                     const char* control_type,
-                     const char* autocomplete,
                      const std::vector<unsigned>& autofill_types,
                      const std::vector<unsigned>& validity_states) {
-  BasicFillUploadField(field, signature, name, control_type, autocomplete);
+  field->set_signature(signature);
 
   for (unsigned i = 0; i < autofill_types.size(); ++i) {
     field->add_autofill_type(autofill_types[i]);
@@ -887,14 +866,11 @@ void FillUploadField(AutofillUploadContents::Field* field,
 
 void FillUploadField(AutofillUploadContents::Field* field,
                      unsigned signature,
-                     const char* name,
-                     const char* control_type,
-                     const char* autocomplete,
                      unsigned autofill_type,
                      const std::vector<unsigned>& validity_states) {
-  BasicFillUploadField(field, signature, name, control_type, autocomplete);
-
+  field->set_signature(signature);
   field->add_autofill_type(autofill_type);
+
   auto* type_validities = field->add_autofill_type_validities();
   type_validities->set_type(autofill_type);
   for (unsigned i = 0; i < validity_states.size(); ++i)

components/autofill/core/browser/autofill_test_utils.h

@@ -309,30 +309,19 @@ void InitializePossibleTypesAndValidities(
     const std::vector<ServerFieldType>& possible_type,
     const std::vector<AutofillDataModel::ValidityState>& validity_state = {});
 
-// Fills the upload |field| with the information passed by parameter. If the
-// value of a const char* parameter is NULL, the corresponding attribute won't
-// be set at all, as opposed to being set to empty string.
+// Fills the upload |field| with the information passed by parameter.
 void FillUploadField(AutofillUploadContents::Field* field,
                      unsigned signature,
-                     const char* name,
-                     const char* control_type,
-                     const char* autocomplete,
                      unsigned autofill_type,
                      unsigned validity_state = 0);
 
 void FillUploadField(AutofillUploadContents::Field* field,
                      unsigned signature,
-                     const char* name,
-                     const char* control_type,
-                     const char* autocomplete,
                      const std::vector<unsigned>& autofill_type,
                      const std::vector<unsigned>& validity_state = {});
 
 void FillUploadField(AutofillUploadContents::Field* field,
                      unsigned signature,
-                     const char* name,
-                     const char* control_type,
-                     const char* autocomplete,
                      unsigned autofill_type,
                      const std::vector<unsigned>& validity_states);
 

components/autofill/core/browser/browser_autofill_manager_unittest.cc

@@ -296,7 +296,6 @@ class MockAutofillDownloadManager : public AutofillDownloadManager {
   explicit MockAutofillDownloadManager(AutofillClient* client)
       : AutofillDownloadManager(client,
                                 /*api_key=*/"",
-                                /*is_raw_metadata_uploading_enabled=*/false,
                                 /*log_manager=*/nullptr) {}
 
   MockAutofillDownloadManager(const MockAutofillDownloadManager&) = delete;