Un-refcount AutofillWebData and TokenWebData

depends on https://codereview.chromium.org/15927029/

BUG=230920

Review URL: https://chromiumcodereview.appspot.com/16154031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@205038 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 91 insertions, 117 deletions

diff --git a/components/autofill/browser/autocomplete_history_manager.cc b/components/autofill/browser/autocomplete_history_manager.cc
index f3f6f3f..7ad0cc3 100644
--- a/components/autofill/browser/autocomplete_history_manager.cc
+++ b/components/autofill/browser/autocomplete_history_manager.cc
@@ -117,7 +117,7 @@ void AutocompleteHistoryManager::OnGetAutocompleteSuggestions(
     return;
   }
 
-  if (autofill_data_.get()) {
+  if (autofill_data_) {
     pending_query_handle_ = autofill_data_->GetFormValuesForElementName(
         name, prefix, kMaxAutocompleteMenuItems, this);
   }
@@ -153,13 +153,13 @@ void AutocompleteHistoryManager::OnFormSubmitted(const FormData& form) {
     }
   }
 
-  if (!values.empty() && autofill_data_.get())
+  if (!values.empty() && autofill_data_)
     autofill_data_->AddFormFields(values);
 }
 
 void AutocompleteHistoryManager::OnRemoveAutocompleteEntry(
     const base::string16& name, const base::string16& value) {
-  if (autofill_data_.get())
+  if (autofill_data_)
     autofill_data_->RemoveFormValueForElementName(name, value);
 }
 
@@ -170,7 +170,7 @@ void AutocompleteHistoryManager::SetExternalDelegate(
 
 void AutocompleteHistoryManager::CancelPendingQuery() {
   if (pending_query_handle_) {
-    if (autofill_data_.get())
+    if (autofill_data_)
       autofill_data_->CancelRequest(pending_query_handle_);
     pending_query_handle_ = 0;
   }
diff --git a/components/autofill/browser/autocomplete_history_manager.h b/components/autofill/browser/autocomplete_history_manager.h
index 9a55fa4..4bba83a 100644
--- a/components/autofill/browser/autocomplete_history_manager.h
+++ b/components/autofill/browser/autocomplete_history_manager.h
@@ -69,7 +69,7 @@ class AutocompleteHistoryManager : public content::WebContentsObserver,
   void CancelPendingQuery();
 
   content::BrowserContext* browser_context_;
-  scoped_refptr<AutofillWebDataService> autofill_data_;
+  AutofillWebDataService* autofill_data_;
 
   BooleanPrefMember autofill_enabled_;
 
diff --git a/components/autofill/browser/autocomplete_history_manager_unittest.cc b/components/autofill/browser/autocomplete_history_manager_unittest.cc
index 57da55a..cc6707f 100644
--- a/components/autofill/browser/autocomplete_history_manager_unittest.cc
+++ b/components/autofill/browser/autocomplete_history_manager_unittest.cc
@@ -38,43 +38,11 @@ class MockWebDataService : public AutofillWebDataService {
  public:
   MockWebDataService()
       : AutofillWebDataService() {
-    current_mock_web_data_service_ = this;
   }
 
   MOCK_METHOD1(AddFormFields, void(const std::vector<FormFieldData>&));
 
-  static scoped_refptr<MockWebDataService> GetCurrent() {
-    if (!current_mock_web_data_service_) {
-      return new MockWebDataService();
-    }
-    return current_mock_web_data_service_;
-  }
-
- protected:
   virtual ~MockWebDataService() {}
-
- private:
-  // Keep track of the most recently created instance, so that it can be
-  // associated with the current profile when Build() is called.
-  static MockWebDataService* current_mock_web_data_service_;
-};
-
-MockWebDataService* MockWebDataService::current_mock_web_data_service_ = NULL;
-
-class MockWebDataServiceWrapperCurrent : public MockWebDataServiceWrapperBase {
- public:
-  static BrowserContextKeyedService* Build(content::BrowserContext* profile) {
-    return new MockWebDataServiceWrapperCurrent();
-  }
-
-  MockWebDataServiceWrapperCurrent() {}
-
-  virtual scoped_refptr<AutofillWebDataService> GetAutofillWebData() OVERRIDE {
-    return MockWebDataService::GetCurrent();
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MockWebDataServiceWrapperCurrent);
 };
 
 class MockAutofillManagerDelegate
@@ -90,25 +58,37 @@ class MockAutofillManagerDelegate
   DISALLOW_COPY_AND_ASSIGN(MockAutofillManagerDelegate);
 };
 
+BrowserContextKeyedService* BuildMockWebDataServiceWrapper(
+    content::BrowserContext* profile) {
+  return new MockWebDataServiceWrapper(
+      NULL,
+      new MockWebDataService(),
+      NULL);
+}
+
 }  // namespace
 
 class AutocompleteHistoryManagerTest : public ChromeRenderViewHostTestHarness {
  protected:
   virtual void SetUp() OVERRIDE {
     ChromeRenderViewHostTestHarness::SetUp();
-    web_data_service_ = new MockWebDataService();
-    WebDataServiceFactory::GetInstance()->SetTestingFactory(
-        profile(), MockWebDataServiceWrapperCurrent::Build);
+    MockWebDataServiceWrapper* wrapper =
+        static_cast<MockWebDataServiceWrapper*>(
+            WebDataServiceFactory::GetInstance()->SetTestingFactoryAndUse(
+                profile(), BuildMockWebDataServiceWrapper));
+    web_data_service_ =
+        static_cast<MockWebDataService*>(wrapper->GetAutofillWebData());
     autocomplete_manager_.reset(new AutocompleteHistoryManager(web_contents()));
   }
 
   virtual void TearDown() OVERRIDE {
     autocomplete_manager_.reset();
+    web_data_service_->ShutdownOnUIThread();
     web_data_service_ = NULL;
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
-  scoped_refptr<MockWebDataService> web_data_service_;
+  MockWebDataService* web_data_service_;
   scoped_ptr<AutocompleteHistoryManager> autocomplete_manager_;
   MockAutofillManagerDelegate manager_delegate;
 };
@@ -130,7 +110,7 @@ TEST_F(AutocompleteHistoryManagerTest, CreditCardNumberValue) {
   valid_cc.form_control_type = "text";
   form.fields.push_back(valid_cc);
 
-  EXPECT_CALL(*web_data_service_.get(), AddFormFields(_)).Times(0);
+  EXPECT_CALL(*web_data_service_, AddFormFields(_)).Times(0);
   autocomplete_manager_->OnFormSubmitted(form);
 }
 
@@ -153,7 +133,7 @@ TEST_F(AutocompleteHistoryManagerTest, NonCreditCardNumberValue) {
   invalid_cc.form_control_type = "text";
   form.fields.push_back(invalid_cc);
 
-  EXPECT_CALL(*(web_data_service_.get()), AddFormFields(_)).Times(1);
+  EXPECT_CALL(*(web_data_service_), AddFormFields(_)).Times(1);
   autocomplete_manager_->OnFormSubmitted(form);
 }
 
@@ -173,7 +153,7 @@ TEST_F(AutocompleteHistoryManagerTest, SSNValue) {
   ssn.form_control_type = "text";
   form.fields.push_back(ssn);
 
-  EXPECT_CALL(*web_data_service_.get(), AddFormFields(_)).Times(0);
+  EXPECT_CALL(*web_data_service_, AddFormFields(_)).Times(0);
   autocomplete_manager_->OnFormSubmitted(form);
 }
 
@@ -194,7 +174,7 @@ TEST_F(AutocompleteHistoryManagerTest, SearchField) {
   search_field.form_control_type = "search";
   form.fields.push_back(search_field);
 
-  EXPECT_CALL(*(web_data_service_.get()), AddFormFields(_)).Times(1);
+  EXPECT_CALL(*(web_data_service_), AddFormFields(_)).Times(1);
   autocomplete_manager_->OnFormSubmitted(form);
 }
 
diff --git a/components/autofill/browser/personal_data_manager.cc b/components/autofill/browser/personal_data_manager.cc
index a399548..e2afca4 100644
--- a/components/autofill/browser/personal_data_manager.cc
+++ b/components/autofill/browser/personal_data_manager.cc
@@ -146,11 +146,11 @@ void PersonalDataManager::Init(BrowserContext* browser_context) {
   if (!browser_context_->IsOffTheRecord())
     metric_logger_->LogIsAutofillEnabledAtStartup(IsAutofillEnabled());
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
 
   // WebDataService may not be available in tests.
-  if (!autofill_data.get())
+  if (!autofill_data)
     return;
 
   LoadProfiles();
@@ -166,9 +166,9 @@ PersonalDataManager::~PersonalDataManager() {
   if (!browser_context_)
     return;
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (autofill_data.get())
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (autofill_data)
     autofill_data->RemoveObserver(this);
 }
 
@@ -375,9 +375,9 @@ 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())
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data)
     return;
 
   // Don't add a duplicate.
@@ -408,9 +408,9 @@ void PersonalDataManager::UpdateProfile(const AutofillProfile& profile) {
     return;
   }
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data)
     return;
 
   // Make the update.
@@ -441,9 +441,9 @@ 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())
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data)
     return;
 
   // Don't add a duplicate.
@@ -474,9 +474,9 @@ void PersonalDataManager::UpdateCreditCard(const CreditCard& credit_card) {
     return;
   }
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data)
     return;
 
   // Make the update.
@@ -496,9 +496,9 @@ 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())
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data)
     return;
 
   if (is_credit_card)
@@ -784,9 +784,9 @@ 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())
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data)
     return;
 
   // Any profiles that are not in the new profile list should be removed from
@@ -838,9 +838,9 @@ void PersonalDataManager::SetCreditCards(
       it++;
   }
 
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get())
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data)
     return;
 
   // Any credit cards that are not in the new credit card list should be
@@ -878,9 +878,9 @@ void PersonalDataManager::SetCreditCards(
 }
 
 void PersonalDataManager::LoadProfiles() {
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get()) {
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data) {
     NOTREACHED();
     return;
   }
@@ -898,9 +898,9 @@ void PersonalDataManager::LoadAuxiliaryProfiles() {
 #endif
 
 void PersonalDataManager::LoadCreditCards() {
-  scoped_refptr<AutofillWebDataService> autofill_data(
-      AutofillWebDataService::FromBrowserContext(browser_context_));
-  if (!autofill_data.get()) {
+  AutofillWebDataService* autofill_data =
+      AutofillWebDataService::FromBrowserContext(browser_context_);
+  if (!autofill_data) {
     NOTREACHED();
     return;
   }
@@ -949,9 +949,9 @@ void PersonalDataManager::ReceiveLoadedCreditCards(
 void PersonalDataManager::CancelPendingQuery(
     WebDataServiceBase::Handle* handle) {
   if (*handle) {
-    scoped_refptr<AutofillWebDataService> autofill_data(
-        AutofillWebDataService::FromBrowserContext(browser_context_));
-    if (!autofill_data.get()) {
+    AutofillWebDataService* autofill_data =
+        AutofillWebDataService::FromBrowserContext(browser_context_);
+    if (!autofill_data) {
       NOTREACHED();
       return;
     }
diff --git a/components/autofill/browser/personal_data_manager_unittest.cc b/components/autofill/browser/personal_data_manager_unittest.cc
index 6c20883..1a4ffd5 100644
--- a/components/autofill/browser/personal_data_manager_unittest.cc
+++ b/components/autofill/browser/personal_data_manager_unittest.cc
@@ -539,9 +539,9 @@ TEST_F(PersonalDataManagerTest, Refresh) {
   profile_pointers.push_back(&profile2);
   AutofillProfile::AdjustInferredLabels(&profile_pointers);
 
-  scoped_refptr<AutofillWebDataService> wds =
+  AutofillWebDataService* wds =
       AutofillWebDataService::FromBrowserContext(profile_.get());
-  ASSERT_TRUE(wds.get());
+  ASSERT_TRUE(wds);
   wds->AddAutofillProfile(profile2);
 
   personal_data_->Refresh();
diff --git a/components/autofill/browser/webdata/autofill_webdata_service.h b/components/autofill/browser/webdata/autofill_webdata_service.h
index b861c64..a85896d 100644
--- a/components/autofill/browser/webdata/autofill_webdata_service.h
+++ b/components/autofill/browser/webdata/autofill_webdata_service.h
@@ -43,9 +43,11 @@ class AutofillWebDataService : public AutofillWebData,
   AutofillWebDataService(scoped_refptr<WebDatabaseService> wdbs,
                          const ProfileErrorCallback& callback);
 
+  virtual ~AutofillWebDataService();
+
   // Retrieve an AutofillWebDataService for the given context.
   // Can return NULL in some contexts.
-  static scoped_refptr<AutofillWebDataService> FromBrowserContext(
+  static AutofillWebDataService* FromBrowserContext(
       content::BrowserContext* context);
 
   // WebDataServiceBase implementation.
@@ -101,8 +103,6 @@ class AutofillWebDataService : public AutofillWebData,
       const base::Callback<void(AutofillWebDataBackend*)>& callback);
 
  protected:
-  virtual ~AutofillWebDataService();
-
   virtual void NotifyAutofillMultipleChangedOnUIThread();
 
   base::WeakPtr<AutofillWebDataService> AsWeakPtr() {
diff --git a/components/autofill/browser/webdata/web_data_service_unittest.cc b/components/autofill/browser/webdata/web_data_service_unittest.cc
index 128ac1c..b12326b 100644
--- a/components/autofill/browser/webdata/web_data_service_unittest.cc
+++ b/components/autofill/browser/webdata/web_data_service_unittest.cc
@@ -76,15 +76,15 @@ class WebDataServiceTest : public testing::Test {
     wdbs_->AddTable(scoped_ptr<WebDatabaseTable>(new AutofillTable("en-US")));
     wdbs_->LoadDatabase();
 
-    wds_ = new AutofillWebDataService(
-        wdbs_, WebDataServiceBase::ProfileErrorCallback());
+    wds_.reset(new AutofillWebDataService(
+        wdbs_, WebDataServiceBase::ProfileErrorCallback()));
     wds_->Init();
   }
 
   virtual void TearDown() {
     wds_->ShutdownOnUIThread();
     wdbs_->ShutdownDatabase();
-    wds_ = NULL;
+    wds_.reset();
     wdbs_ = NULL;
     WaitForDatabaseThread();
 
@@ -107,7 +107,7 @@ class WebDataServiceTest : public testing::Test {
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread db_thread_;
   base::FilePath profile_dir_;
-  scoped_refptr<AutofillWebDataService> wds_;
+  scoped_ptr<AutofillWebDataService> wds_;
   scoped_refptr<WebDatabaseService> wdbs_;
   base::ScopedTempDir temp_dir_;
 };
@@ -135,7 +135,8 @@ class WebDataServiceAutofillTest : public WebDataServiceTest {
     BrowserThread::PostTask(
         BrowserThread::DB,
         FROM_HERE,
-        base::Bind(add_observer_func, wds_, &observer_));
+        base::Bind(add_observer_func,
+                   base::Unretained(wds_.get()), &observer_));
     WaitForDatabaseThread();
   }
 
@@ -146,7 +147,8 @@ class WebDataServiceAutofillTest : public WebDataServiceTest {
     BrowserThread::PostTask(
         BrowserThread::DB,
         FROM_HERE,
-        base::Bind(remove_observer_func, wds_, &observer_));
+        base::Bind(remove_observer_func,
+                   base::Unretained(wds_.get()), &observer_));
     WaitForDatabaseThread();
 
     WebDataServiceTest::TearDown();
diff --git a/components/webdata/common/web_data_service_base.h b/components/webdata/common/web_data_service_base.h
index 11d5d3e..0510184 100644
--- a/components/webdata/common/web_data_service_base.h
+++ b/components/webdata/common/web_data_service_base.h
@@ -23,9 +23,7 @@ class Thread;
 }
 
 // Base for WebDataService class hierarchy.
-class WEBDATA_EXPORT WebDataServiceBase
-    : public base::RefCountedThreadSafe<WebDataServiceBase,
-          content::BrowserThread::DeleteOnUIThread> {
+class WEBDATA_EXPORT WebDataServiceBase {
  public:
   // All requests return an opaque handle of the following type.
   typedef int Handle;
@@ -48,6 +46,8 @@ class WEBDATA_EXPORT WebDataServiceBase
   WebDataServiceBase(scoped_refptr<WebDatabaseService> wdbs,
                      const ProfileErrorCallback& callback);
 
+  virtual ~WebDataServiceBase();
+
   // Cancel any pending request. You need to call this method if your
   // WebDataServiceConsumer is about to be deleted.
   virtual void CancelRequest(Handle h);
@@ -88,20 +88,13 @@ class WEBDATA_EXPORT WebDataServiceBase
   virtual WebDatabase* GetDatabase();
 
  protected:
-  virtual ~WebDataServiceBase();
-
   // Our database service.
   scoped_refptr<WebDatabaseService> wdbs_;
 
  private:
-  friend struct content::BrowserThread::DeleteOnThread<
-      content::BrowserThread::UI>;
-  friend class base::DeleteHelper<WebDataServiceBase>;
-  // We have to friend RCTS<> so WIN shared-lib build is happy (crbug/112250).
-  friend class base::RefCountedThreadSafe<WebDataServiceBase,
-      content::BrowserThread::DeleteOnUIThread>;
-
   ProfileErrorCallback profile_error_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebDataServiceBase);
 };
 
 #endif  // COMPONENTS_WEBDATA_COMMON_WEB_DATA_SERVICE_BASE_H_
diff --git a/components/webdata/common/web_data_service_test_util.cc b/components/webdata/common/web_data_service_test_util.cc
index 2720721..0ece6a4 100644
--- a/components/webdata/common/web_data_service_test_util.cc
+++ b/components/webdata/common/web_data_service_test_util.cc
@@ -22,8 +22,8 @@ void MockWebDataServiceWrapperBase::Shutdown() {
 // all the webdatas in.
 MockWebDataServiceWrapper::MockWebDataServiceWrapper(
     scoped_refptr<WebDataService> fake_service,
-    scoped_refptr<AutofillWebDataService> fake_autofill,
-    scoped_refptr<TokenWebData> fake_token)
+    AutofillWebDataService* fake_autofill,
+    TokenWebData* fake_token)
     : fake_autofill_web_data_(fake_autofill),
       fake_token_web_data_(fake_token),
       fake_web_data_(fake_service) {
@@ -32,13 +32,13 @@ MockWebDataServiceWrapper::MockWebDataServiceWrapper(
 MockWebDataServiceWrapper::~MockWebDataServiceWrapper() {
 }
 
-scoped_refptr<AutofillWebDataService>
+AutofillWebDataService*
     MockWebDataServiceWrapper::GetAutofillWebData() {
-  return fake_autofill_web_data_;
+  return fake_autofill_web_data_.get();
 }
 
-scoped_refptr<TokenWebData> MockWebDataServiceWrapper::GetTokenWebData() {
-  return fake_token_web_data_;
+TokenWebData* MockWebDataServiceWrapper::GetTokenWebData() {
+  return fake_token_web_data_.get();
 }
 
 scoped_refptr<WebDataService> MockWebDataServiceWrapper::GetWebData() {
diff --git a/components/webdata/common/web_data_service_test_util.h b/components/webdata/common/web_data_service_test_util.h
index 4f0f6e5..6fb3b63 100644
--- a/components/webdata/common/web_data_service_test_util.h
+++ b/components/webdata/common/web_data_service_test_util.h
@@ -58,21 +58,20 @@ class MockWebDataServiceWrapper : public MockWebDataServiceWrapperBase {
  public:
   MockWebDataServiceWrapper(
       scoped_refptr<WebDataService> fake_service,
-      scoped_refptr<autofill::AutofillWebDataService> fake_autofill,
-      scoped_refptr<TokenWebData> fake_token);
+      autofill::AutofillWebDataService* fake_autofill,
+      TokenWebData* fake_token);
 
   virtual ~MockWebDataServiceWrapper();
 
-  virtual scoped_refptr<autofill::AutofillWebDataService>
-      GetAutofillWebData() OVERRIDE;
+  virtual autofill::AutofillWebDataService* GetAutofillWebData() OVERRIDE;
 
-  virtual scoped_refptr<TokenWebData> GetTokenWebData() OVERRIDE;
+  virtual TokenWebData* GetTokenWebData() OVERRIDE;
 
   virtual scoped_refptr<WebDataService> GetWebData() OVERRIDE;
 
  protected:
-  scoped_refptr<autofill::AutofillWebDataService> fake_autofill_web_data_;
-  scoped_refptr<TokenWebData> fake_token_web_data_;
+  scoped_ptr<autofill::AutofillWebDataService> fake_autofill_web_data_;
+  scoped_ptr<TokenWebData> fake_token_web_data_;
   scoped_refptr<WebDataService> fake_web_data_;
 
  private: