Have AutofillManagerDelegate supply the AutofillWebDataService to core code.

components/autofill/core/browser/personal_data_manager.cc

Index: components/autofill/core/browser/personal_data_manager.cc
diff --git a/components/autofill/core/browser/personal_data_manager.cc b/components/autofill/core/browser/personal_data_manager.cc
index 243da5edca1d370726feb58e14132949b093c2f8..77138430691a37e32dff16401599d164a4c7558f 100644
--- a/components/autofill/core/browser/personal_data_manager.cc
+++ b/components/autofill/core/browser/personal_data_manager.cc
@@ -25,7 +25,6 @@
 #include "components/autofill/core/browser/validation.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
-#include "content/public/browser/browser_context.h"
 
 namespace autofill {
 namespace {
@@ -147,7 +146,7 @@ static bool CompareVotes(const std::pair<std::string, int>& a,
 }  // namespace
 
 PersonalDataManager::PersonalDataManager(const std::string& app_locale)
-    : browser_context_(NULL),
+    : autofill_webdata_(NULL),
       is_data_loaded_(false),
       pending_profiles_query_(0),
       pending_creditcards_query_(0),
@@ -156,40 +155,33 @@ PersonalDataManager::PersonalDataManager(const std::string& app_locale)
       is_off_the_record_(false),
       has_logged_profile_count_(false) {}
 
-void PersonalDataManager::Init(content::BrowserContext* browser_context,
-                               PrefService* pref_service,
-                               bool is_off_the_record) {
-  browser_context_ = browser_context;
+void PersonalDataManager::Init(
+    scoped_refptr<AutofillWebDataService> autofill_webdata,
+    PrefService* pref_service,
+    bool is_off_the_record) {
+  autofill_webdata_ = autofill_webdata;
   pref_service_ = pref_service;
   is_off_the_record_ = is_off_the_record;
 
   if (!is_off_the_record_)
     metric_logger_->LogIsAutofillEnabledAtStartup(IsAutofillEnabled());
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-
   // WebDataService may not be available in tests.

[Ilya Sherman, 2013/11/14 02:22:02]

Out of curiousity, do you know whether this is common?  I'd love to fix this if
possible, so that the production code doesn't need to be aware of how test code
might be playing with its dependencies.

[blundell, 2013/11/14 16:46:30]

Found the offenders (at least in unit_tests) by putting an
EXPECT_TRUE(autofill_webdata) in PersonalDataManager::Init(). Filed
crbug.com/319329.

On 2013/11/14 02:22:02, Ilya Sherman wrote:

-  if (!autofill_data.get())
+  if (!autofill_webdata_.get())
     return;
 
   LoadProfiles();
   LoadCreditCards();
 
-  autofill_data->AddObserver(this);
+  autofill_webdata_->AddObserver(this);
 }
 
 PersonalDataManager::~PersonalDataManager() {
   CancelPendingQuery(&pending_profiles_query_);
   CancelPendingQuery(&pending_creditcards_query_);
 
-  if (!browser_context_)
-    return;
-
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (autofill_data.get())
-    autofill_data->RemoveObserver(this);
+  if (autofill_webdata_.get())
+    autofill_webdata_->RemoveObserver(this);
 }
 
 void PersonalDataManager::OnWebDataServiceRequestDone(
@@ -395,9 +387,7 @@ void PersonalDataManager::AddProfile(const AutofillProfile& profile) {
   if (FindByGUID<AutofillProfile>(web_profiles_, profile.guid()))
     return;
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  if (!autofill_webdata_.get())
     return;
 
   // Don't add a duplicate.
@@ -405,7 +395,7 @@ void PersonalDataManager::AddProfile(const AutofillProfile& profile) {
     return;
 
   // Add the new profile to the web database.
-  autofill_data->AddAutofillProfile(profile);
+  autofill_webdata_->AddAutofillProfile(profile);
 
   // Refresh our local cache and send notifications to observers.
   Refresh();
@@ -428,13 +418,11 @@ void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) {
     return;
   }
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  if (!autofill_webdata_.get())
     return;
 
   // Make the update.
-  autofill_data->UpdateAutofillProfile(profile);
+  autofill_webdata_->UpdateAutofillProfile(profile);
 
   // Refresh our local cache and send notifications to observers.
   Refresh();
@@ -458,9 +446,7 @@ void PersonalDataManager::AddCreditCard(const CreditCard& credit_card) {
   if (FindByGUID<CreditCard>(credit_cards_, credit_card.guid()))
     return;
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  if (!autofill_webdata_.get())
     return;
 
   // Don't add a duplicate.
@@ -468,7 +454,7 @@ void PersonalDataManager::AddCreditCard(const CreditCard& credit_card) {
     return;
 
   // Add the new credit card to the web database.
-  autofill_data->AddCreditCard(credit_card);
+  autofill_webdata_->AddCreditCard(credit_card);
 
   // Refresh our local cache and send notifications to observers.
   Refresh();
@@ -491,13 +477,11 @@ void PersonalDataManager::UpdateCreditCard(const CreditCard& credit_card) {
     return;
   }
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  if (!autofill_webdata_.get())
     return;
 
   // Make the update.
-  autofill_data->UpdateCreditCard(credit_card);
+  autofill_webdata_->UpdateCreditCard(credit_card);
 
   // Refresh our local cache and send notifications to observers.
   Refresh();
@@ -513,15 +497,13 @@ void PersonalDataManager::RemoveByGUID(const std::string& guid) {
   if (!is_credit_card && !is_profile)
     return;
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  if (!autofill_webdata_.get())
     return;
 
   if (is_credit_card)
-    autofill_data->RemoveCreditCard(guid);
+    autofill_webdata_->RemoveCreditCard(guid);
   else
-    autofill_data->RemoveAutofillProfile(guid);
+    autofill_webdata_->RemoveAutofillProfile(guid);
 
   // Refresh our local cache and send notifications to observers.
   Refresh();
@@ -804,9 +786,7 @@ void PersonalDataManager::SetProfiles(std::vector<AutofillProfile>* profiles) {
       address_of<AutofillProfile>);
   AutofillProfile::AdjustInferredLabels(&profile_pointers);
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  if (!autofill_webdata_.get())
     return;
 
   // Any profiles that are not in the new profile list should be removed from
@@ -815,14 +795,14 @@ void PersonalDataManager::SetProfiles(std::vector<AutofillProfile>* profiles) {
            web_profiles_.begin();
        iter != web_profiles_.end(); ++iter) {
     if (!FindByGUID<AutofillProfile>(*profiles, (*iter)->guid()))
-      autofill_data->RemoveAutofillProfile((*iter)->guid());
+      autofill_webdata_->RemoveAutofillProfile((*iter)->guid());
   }
 
   // Update the web database with the existing profiles.
   for (std::vector<AutofillProfile>::iterator iter = profiles->begin();
        iter != profiles->end(); ++iter) {
     if (FindByGUID<AutofillProfile>(web_profiles_, iter->guid()))
-      autofill_data->UpdateAutofillProfile(*iter);
+      autofill_webdata_->UpdateAutofillProfile(*iter);
   }
 
   // Add the new profiles to the web database.  Don't add a duplicate.
@@ -830,7 +810,7 @@ void PersonalDataManager::SetProfiles(std::vector<AutofillProfile>* profiles) {
        iter != profiles->end(); ++iter) {
     if (!FindByGUID<AutofillProfile>(web_profiles_, iter->guid()) &&
         !FindByContents(web_profiles_, *iter))
-      autofill_data->AddAutofillProfile(*iter);
+      autofill_webdata_->AddAutofillProfile(*iter);
   }
 
   // Copy in the new profiles.
@@ -854,9 +834,7 @@ void PersonalDataManager::SetCreditCards(
                                      IsEmptyFunctor<CreditCard>(app_locale_)),
                       credit_cards->end());
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  if (!autofill_webdata_.get())
     return;
 
   // Any credit cards that are not in the new credit card list should be
@@ -864,14 +842,14 @@ void PersonalDataManager::SetCreditCards(
   for (std::vector<CreditCard*>::const_iterator iter = credit_cards_.begin();
        iter != credit_cards_.end(); ++iter) {
     if (!FindByGUID<CreditCard>(*credit_cards, (*iter)->guid()))
-      autofill_data->RemoveCreditCard((*iter)->guid());
+      autofill_webdata_->RemoveCreditCard((*iter)->guid());
   }
 
   // Update the web database with the existing credit cards.
   for (std::vector<CreditCard>::iterator iter = credit_cards->begin();
        iter != credit_cards->end(); ++iter) {
     if (FindByGUID<CreditCard>(credit_cards_, iter->guid()))
-      autofill_data->UpdateCreditCard(*iter);
+      autofill_webdata_->UpdateCreditCard(*iter);
   }
 
   // Add the new credit cards to the web database.  Don't add a duplicate.
@@ -879,7 +857,7 @@ void PersonalDataManager::SetCreditCards(
        iter != credit_cards->end(); ++iter) {
     if (!FindByGUID<CreditCard>(credit_cards_, iter->guid()) &&
         !FindByContents(credit_cards_, *iter))
-      autofill_data->AddCreditCard(*iter);
+      autofill_webdata_->AddCreditCard(*iter);
   }
 
   // Copy in the new credit cards.
@@ -894,16 +872,14 @@ void PersonalDataManager::SetCreditCards(
 }
 
 void PersonalDataManager::LoadProfiles() {
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get()) {
+  if (!autofill_webdata_.get()) {
     NOTREACHED();
     return;
   }
 
   CancelPendingQuery(&pending_profiles_query_);
 
-  pending_profiles_query_ = autofill_data->GetAutofillProfiles(this);
+  pending_profiles_query_ = autofill_webdata_->GetAutofillProfiles(this);
 }
 
 // Win and Linux implementations do nothing. Mac and Android implementations
@@ -914,16 +890,14 @@ void PersonalDataManager::LoadAuxiliaryProfiles() const {
 #endif
 
 void PersonalDataManager::LoadCreditCards() {
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get()) {
+  if (!autofill_webdata_.get()) {
     NOTREACHED();
     return;
   }
 
   CancelPendingQuery(&pending_creditcards_query_);
 
-  pending_creditcards_query_ = autofill_data->GetCreditCards(this);
+  pending_creditcards_query_ = autofill_webdata_->GetCreditCards(this);
 }
 
 void PersonalDataManager::ReceiveLoadedProfiles(WebDataServiceBase::Handle h,
@@ -965,13 +939,11 @@ void PersonalDataManager::ReceiveLoadedCreditCards(
 void PersonalDataManager::CancelPendingQuery(
     WebDataServiceBase::Handle* handle) {
   if (*handle) {
-    scoped_refptr<AutofillWebDataService> autofill_data(
-        AutofillWebDataService::FromBrowserContext(browser_context_));
-    if (!autofill_data.get()) {
+    if (!autofill_webdata_.get()) {
       NOTREACHED();
       return;
     }
-    autofill_data->CancelRequest(*handle);
+    autofill_webdata_->CancelRequest(*handle);
   }
   *handle = 0;
 }
@@ -1047,9 +1019,9 @@ void PersonalDataManager::set_metric_logger(
   metric_logger_.reset(metric_logger);
 }
 
-void PersonalDataManager::set_browser_context(
-    content::BrowserContext* context) {
-  browser_context_ = context;
+void PersonalDataManager::set_autofill_webdata_service(
+    scoped_refptr<AutofillWebDataService> webdata) {
+  autofill_webdata_ = webdata;
 }
 
 void PersonalDataManager::set_pref_service(PrefService* pref_service) {