Have AutofillManagerDelegate supply the AutofillWebDataService to core code.

components/autofill/core/browser/personal_data_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/core/browser/personal_data_manager.h"
 
 #include <algorithm>
 #include <functional>
 #include <iterator>
 
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill-inl.h"
 #include "components/autofill/core/browser/autofill_country.h"
 #include "components/autofill/core/browser/autofill_field.h"
 #include "components/autofill/core/browser/autofill_metrics.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/personal_data_manager_observer.h"
 #include "components/autofill/core/browser/phone_number.h"
 #include "components/autofill/core/browser/phone_number_i18n.h"
 #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 {
 
 const base::string16::value_type kCreditCardPrefix[] = {'*', 0};
 
 template<typename T>
 class FormGroupMatchesByGUIDFunctor {
  public:
   explicit FormGroupMatchesByGUIDFunctor(const std::string& guid)
@@ skipping 101 matching lines @@
 
 // A helper function for finding the maximum value in a string->int map.
 static bool CompareVotes(const std::pair<std::string, int>& a,
                          const std::pair<std::string, int>& b) {
   return a.second < b.second;
 }
 
 }  // 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),
       app_locale_(app_locale),
       metric_logger_(new AutofillMetrics),
       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(
     WebDataServiceBase::Handle h,
     const WDTypedResult* result) {
   DCHECK(pending_profiles_query_ || pending_creditcards_query_);
 
   if (!result) {
     // Error from the web database.
     if (h == pending_creditcards_query_)
@@ skipping 185 matching lines @@
   if (is_off_the_record_)
     return;
 
   if (profile.IsEmpty(app_locale_))
     return;
 
   // Don't add an existing 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.
   if (FindByContents(web_profiles_, 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();
 }
 
 void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) {
   if (is_off_the_record_)
     return;
 
   AutofillProfile* existing_profile = GetProfileByGUID(profile.guid());
   if (!existing_profile)
     return;
 
   // Don't overwrite the origin for a profile that is already stored.
   if (existing_profile->Compare(profile) == 0)
     return;
 
   if (profile.IsEmpty(app_locale_)) {
     RemoveByGUID(profile.guid());
     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();
 }
 
 AutofillProfile* PersonalDataManager::GetProfileByGUID(
     const std::string& guid) {
   const std::vector<AutofillProfile*>& profiles = GetProfiles();
   std::vector<AutofillProfile*>::const_iterator iter =
       FindElementByGUID<AutofillProfile>(profiles, guid);
   return (iter != profiles.end()) ? *iter : NULL;
 }
 
 void PersonalDataManager::AddCreditCard(const CreditCard& credit_card) {
   if (is_off_the_record_)
     return;
 
   if (credit_card.IsEmpty(app_locale_))
     return;
 
   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.
   if (FindByContents(credit_cards_, 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();
 }
 
 void PersonalDataManager::UpdateCreditCard(const CreditCard& credit_card) {
   if (is_off_the_record_)
     return;
 
   CreditCard* existing_credit_card = GetCreditCardByGUID(credit_card.guid());
   if (!existing_credit_card)
     return;
 
   // Don't overwrite the origin for a credit card that is already stored.
   if (existing_credit_card->Compare(credit_card) == 0)
     return;
 
   if (credit_card.IsEmpty(app_locale_)) {
     RemoveByGUID(credit_card.guid());
     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();
 }
 
 void PersonalDataManager::RemoveByGUID(const std::string& guid) {
   if (is_off_the_record_)
     return;
 
   bool is_credit_card = FindByGUID<CreditCard>(credit_cards_, guid);
   bool is_profile = !is_credit_card &&
       FindByGUID<AutofillProfile>(web_profiles_, 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();
 }
 
 CreditCard* PersonalDataManager::GetCreditCardByGUID(const std::string& guid) {
   const std::vector<CreditCard*>& credit_cards = GetCreditCards();
   std::vector<CreditCard*>::const_iterator iter =
       FindElementByGUID<CreditCard>(credit_cards, guid);
   return (iter != credit_cards.end()) ? *iter : NULL;
@@ skipping 262 matching lines @@
 
   // Ensure that profile labels are up to date.  Currently, sync relies on
   // labels to identify a profile.
   // TODO(dhollowa): We need to deprecate labels and update the way sync
   // identifies profiles.
   std::vector<AutofillProfile*> profile_pointers(profiles->size());
   std::transform(profiles->begin(), profiles->end(), profile_pointers.begin(),
       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
   // the web database.
   for (std::vector<AutofillProfile*>::const_iterator iter =
            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.
   for (std::vector<AutofillProfile>::iterator iter = profiles->begin();
        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.
   web_profiles_.clear();
   for (std::vector<AutofillProfile>::iterator iter = profiles->begin();
        iter != profiles->end(); ++iter) {
     web_profiles_.push_back(new AutofillProfile(*iter));
   }
 
   // Refresh our local cache and send notifications to observers.
   Refresh();
 }
 
 void PersonalDataManager::SetCreditCards(
     std::vector<CreditCard>* credit_cards) {
   if (is_off_the_record_)
     return;
 
   // Remove empty credit cards from input.
   credit_cards->erase(std::remove_if(credit_cards->begin(), credit_cards->end(),
                                      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
   // removed.
   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.
   for (std::vector<CreditCard>::iterator iter = credit_cards->begin();
        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.
   credit_cards_.clear();
   for (std::vector<CreditCard>::iterator iter = credit_cards->begin();
        iter != credit_cards->end(); ++iter) {
     credit_cards_.push_back(new CreditCard(*iter));
   }
 
   // Refresh our local cache and send notifications to observers.
   Refresh();
 }
 
 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
 // fill in the contents of |auxiliary_profiles_|.
 #if !defined(OS_MACOSX) && !defined(OS_ANDROID)
 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,
                                                 const WDTypedResult* result) {
   DCHECK_EQ(pending_profiles_query_, h);
 
   pending_profiles_query_ = 0;
   web_profiles_.clear();
 
   const WDResult<std::vector<AutofillProfile*> >* r =
@@ skipping 21 matching lines @@
   std::vector<CreditCard*> credit_cards = r->GetValue();
   for (std::vector<CreditCard*>::iterator iter = credit_cards.begin();
        iter != credit_cards.end(); ++iter) {
     credit_cards_.push_back(*iter);
   }
 }
 
 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;
 }
 
 std::string PersonalDataManager::SaveImportedProfile(
     const AutofillProfile& imported_profile) {
   if (is_off_the_record_)
     return std::string();
 
   // Don't save a web profile if the data in the profile is a subset of an
@@ skipping 55 matching lines @@
 
 const AutofillMetrics* PersonalDataManager::metric_logger() const {
   return metric_logger_.get();
 }
 
 void PersonalDataManager::set_metric_logger(
     const AutofillMetrics* 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) {
   pref_service_ = pref_service;
 }
 
 std::string PersonalDataManager::MostCommonCountryCodeFromProfiles() const {
   // Count up country codes from existing profiles.
   std::map<std::string, int> votes;
   // TODO(estade): can we make this GetProfiles() instead? It seems to cause
@@ skipping 16 matching lines @@
   if (!votes.empty()) {
     std::map<std::string, int>::iterator iter =
         std::max_element(votes.begin(), votes.end(), CompareVotes);
     return iter->first;
   }
 
   return std::string();
 }
 
 }  // namespace autofill