Autofill: Move the DownloadManager to AutofillHandler

No functionality change

Bug: 1151542
Change-Id: I5c3acb2dc12e84bf4c9d3a1d01fb5c4b7e64ab39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566336Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Michael Bai <michaelbai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832516}

components/autofill/core/browser/autofill_handler.cc

@@ -13,7 +13,9 @@
 #include "components/autofill/core/common/autofill_internals/log_message.h"
 #include "components/autofill/core/common/autofill_internals/logging_scope.h"
 #include "components/autofill/core/common/autofill_payments_features.h"
+#include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/autofill_tick_clock.h"
+#include "google_apis/google_api_keys.h"
 #include "ui/gfx/geometry/rect_f.h"
 
 namespace autofill {
@@ -55,13 +57,34 @@ bool CachedFormNeedsUpdate(const FormData& live_form,
   return false;
 }
 
+std::string GetAPIKeyForUrl(version_info::Channel channel) {
+  // First look if we can get API key from command line flag.
+  const base::CommandLine& command_line =
+      *base::CommandLine::ForCurrentProcess();
+  if (command_line.HasSwitch(switches::kAutofillAPIKey))
+    return command_line.GetSwitchValueASCII(switches::kAutofillAPIKey);
+
+  // Get the API key from Chrome baked keys.
+  if (channel == version_info::Channel::STABLE)
+    return google_apis::GetAPIKey();
+  return google_apis::GetNonStableAPIKey();
+}
+
 }  // namespace
 
 using base::TimeTicks;
 
-AutofillHandler::AutofillHandler(AutofillDriver* driver,
-                                 LogManager* log_manager)
-    : driver_(driver), log_manager_(log_manager) {}
+AutofillHandler::AutofillHandler(
+    AutofillDriver* driver,
+    LogManager* log_manager,
+    AutofillDownloadManagerState enable_download_manager,
+    version_info::Channel channel)
+    : driver_(driver), log_manager_(log_manager) {
+  if (enable_download_manager == ENABLE_AUTOFILL_DOWNLOAD_MANAGER) {
+    download_manager_ = std::make_unique<AutofillDownloadManager>(
+        driver, this, GetAPIKeyForUrl(channel), log_manager);
+  }
+}
 
 AutofillHandler::~AutofillHandler() = default;
 

components/autofill/core/browser/autofill_handler.h

@@ -20,6 +20,7 @@
 #include "components/autofill/core/common/mojom/autofill_types.mojom.h"
 #include "components/autofill/core/common/renderer_id.h"
 #include "components/autofill/core/common/signatures.h"
+#include "components/version_info/channel.h"
 
 namespace gfx {
 class RectF;
@@ -143,6 +144,10 @@ class AutofillHandler : public AutofillDownloadManager::Observer {
 
   AutofillDriver* driver() { return driver_; }
 
+  AutofillDownloadManager* download_manager() {
+    return download_manager_.get();
+  }
+
 #ifdef UNIT_TEST
   // A public wrapper that calls |mutable_form_structures| for testing purposes
   // only.
@@ -158,7 +163,10 @@ class AutofillHandler : public AutofillDownloadManager::Observer {
 #endif
 
  protected:
-  AutofillHandler(AutofillDriver* driver, LogManager* log_manager);
+  AutofillHandler(AutofillDriver* driver,
+                  LogManager* log_manager,
+                  AutofillDownloadManagerState enable_download_manager,
+                  version_info::Channel channel);
 
   virtual void OnFormSubmittedImpl(const FormData& form,
                                    bool known_success,
@@ -221,6 +229,11 @@ class AutofillHandler : public AutofillDownloadManager::Observer {
     return &form_structures_;
   }
 
+  // Exposed for testing.
+  void set_download_manager(AutofillDownloadManager* manager) {
+    download_manager_.reset(manager);
+  }
+
  private:
   // AutofillDownloadManager::Observer:
   void OnLoadedServerPredictions(
@@ -239,6 +252,10 @@ class AutofillHandler : public AutofillDownloadManager::Observer {
   // Our copy of the form data.
   std::map<FormRendererId, std::unique_ptr<FormStructure>> form_structures_;
 
+  // Handles queries and uploads to Autofill servers. Will be nullptr if
+  // the download manager functionality is disabled.
+  std::unique_ptr<AutofillDownloadManager> download_manager_;
+
   // Will be not null only for |SaveCardBubbleViewsFullFormBrowserTest|.
   ObserverForTest* observer_for_testing_ = nullptr;
 

components/autofill/core/browser/autofill_handler_proxy.cc

@@ -13,7 +13,11 @@ using base::TimeTicks;
 AutofillHandlerProxy::AutofillHandlerProxy(AutofillDriver* driver,
                                            LogManager* log_manager,
                                            AutofillProvider* provider)
-    : AutofillHandler(driver, log_manager), provider_(provider) {}
+    : AutofillHandler(driver,
+                      log_manager,
+                      DISABLE_AUTOFILL_DOWNLOAD_MANAGER,
+                      version_info::Channel::UNKNOWN),
+      provider_(provider) {}
 
 AutofillHandlerProxy::~AutofillHandlerProxy() {}
 

components/autofill/core/browser/autofill_manager.cc

@@ -78,7 +78,6 @@
 #include "components/autofill/core/common/autofill_internals/logging_scope.h"
 #include "components/autofill/core/common/autofill_payments_features.h"
 #include "components/autofill/core/common/autofill_prefs.h"
-#include "components/autofill/core/common/autofill_switches.h"
 #include "components/autofill/core/common/autofill_tick_clock.h"
 #include "components/autofill/core/common/autofill_util.h"
 #include "components/autofill/core/common/form_data.h"
@@ -91,8 +90,6 @@
 #include "components/security_interstitials/core/pref_names.h"
 #include "components/security_state/core/security_state.h"
 #include "components/strings/grit/components_strings.h"
-#include "components/version_info/channel.h"
-#include "google_apis/google_api_keys.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/geometry/rect.h"
@@ -189,19 +186,6 @@ void LogDeveloperEngagementUkm(ukm::UkmRecorder* ukm_recorder,
   }
 }
 
-std::string GetAPIKeyForUrl(version_info::Channel channel) {
-  // First look if we can get API key from command line flag.
-  const base::CommandLine& command_line =
-      *base::CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kAutofillAPIKey))
-    return command_line.GetSwitchValueASCII(switches::kAutofillAPIKey);
-
-  // Get the API key from Chrome baked keys.
-  if (channel == version_info::Channel::STABLE)
-    return google_apis::GetAPIKey();
-  return google_apis::GetNonStableAPIKey();
-}
-
 ValuePatternsMetric GetValuePattern(const base::string16& value) {
   if (IsUPIVirtualPaymentAddress(value))
     return ValuePatternsMetric::kUpiVpa;
@@ -1561,7 +1545,7 @@ void AutofillManager::UploadFormDataAsyncCallback(
 
 void AutofillManager::UploadFormData(const FormStructure& submitted_form,
                                      bool observed_submission) {
-  if (!download_manager_)
+  if (!download_manager())
     return;
 
   // Check if the form is among the forms that were recently auto-filled.
@@ -1582,7 +1566,7 @@ void AutofillManager::UploadFormData(const FormStructure& submitted_form,
     non_empty_types.insert(CREDIT_CARD_VERIFICATION_CODE);
   }
 
-  download_manager_->StartUploadRequest(
+  download_manager()->StartUploadRequest(
       submitted_form, was_autofilled, non_empty_types,
       /*login_form_signature=*/std::string(), observed_submission,
       client_->GetPrefs());
@@ -1632,7 +1616,10 @@ AutofillManager::AutofillManager(
     const std::string app_locale,
     AutofillDownloadManagerState enable_download_manager,
     std::unique_ptr<CreditCardAccessManager> cc_access_manager)
-    : AutofillHandler(driver, client->GetLogManager()),
+    : AutofillHandler(driver,
+                      client->GetLogManager(),
+                      enable_download_manager,
+                      client->GetChannel()),
       client_(client),
       log_manager_(client_->GetLogManager()),
       app_locale_(app_locale),
@@ -1661,11 +1648,6 @@ AutofillManager::AutofillManager(
                                     : std::make_unique<CreditCardAccessManager>(
                                           driver, client_, personal_data_,
                                           credit_card_form_event_logger_.get());
-  if (enable_download_manager == ENABLE_AUTOFILL_DOWNLOAD_MANAGER) {
-    version_info::Channel channel = client_->GetChannel();
-    download_manager_.reset(new AutofillDownloadManager(
-        driver, this, GetAPIKeyForUrl(channel), client_->GetLogManager()));
-  }
   CountryNames::SetLocaleString(app_locale_);
   offer_manager_ = client_->GetAutofillOfferManager();
 }
@@ -2138,8 +2120,8 @@ void AutofillManager::OnFormsParsed(const std::vector<const FormData*>& forms) {
   LogAutofillTypePredictionsAvailable(log_manager_, queryable_forms);
 
   // Query the server if at least one of the forms was parsed.
-  if (!queryable_forms.empty() && download_manager_) {
-    download_manager_->StartQueryRequest(queryable_forms);
+  if (!queryable_forms.empty() && download_manager()) {
+    download_manager()->StartQueryRequest(queryable_forms);
   }
 }
 

components/autofill/core/browser/autofill_manager.h

@@ -177,10 +177,6 @@ class AutofillManager : public AutofillHandler,
 
   AutofillClient* client() { return client_; }
 
-  AutofillDownloadManager* download_manager() {
-    return download_manager_.get();
-  }
-
   CreditCardAccessManager* credit_card_access_manager() {
     return credit_card_access_manager_.get();
   }
@@ -376,11 +372,6 @@ class AutofillManager : public AutofillHandler,
     return form_interactions_ukm_logger_.get();
   }
 
-  // Exposed for testing.
-  void set_download_manager(AutofillDownloadManager* manager) {
-    download_manager_.reset(manager);
-  }
-
   // Exposed for testing.
   bool is_rich_query_enabled() const { return is_rich_query_enabled_; }
 
@@ -648,10 +639,6 @@ class AutofillManager : public AutofillHandler,
 
   base::circular_deque<std::string> autofilled_form_signatures_;
 
-  // Handles queries and uploads to Autofill servers. Will be NULL if
-  // the download manager functionality is disabled.
-  std::unique_ptr<AutofillDownloadManager> download_manager_;
-
   // Handles single-field autocomplete form data.
   // May be NULL.  NULL indicates OTR.
   base::WeakPtr<AutocompleteHistoryManager> autocomplete_history_manager_;

components/autofill/core/browser/autofill_manager_unittest.cc

@@ -7583,7 +7583,8 @@ TEST_P(AutofillManagerStructuredProfileTest,
         version_info::Channel::UNKNOWN}) {
     SCOPED_TRACE(::testing::Message()
                  << "Channel " << static_cast<int>(channel));
-    EXPECT_CALL(autofill_client_, GetChannel()).WillOnce(Return(channel));
+    // One more call is from TestAutofillManager constructor.
+    EXPECT_CALL(autofill_client_, GetChannel()).WillRepeatedly(Return(channel));
     TestAutofillManager test_instance(autofill_driver_.get(), &autofill_client_,
                                       &personal_data_,
                                       autocomplete_history_manager_.get());
@@ -7618,7 +7619,8 @@ TEST_P(AutofillManagerStructuredProfileTest,
     SCOPED_TRACE(::testing::Message()
                  << "Channel " << static_cast<int>(channel));
     EXPECT_FALSE(AutofillManager::IsRichQueryEnabled(channel));
-    EXPECT_CALL(autofill_client_, GetChannel()).WillOnce(Return(channel));
+    // One more call is from TestAutofillManager constructor.
+    EXPECT_CALL(autofill_client_, GetChannel()).WillRepeatedly(Return(channel));
     TestAutofillManager test_instance(autofill_driver_.get(), &autofill_client_,
                                       &personal_data_,
                                       autocomplete_history_manager_.get());