Have AutofillManagerDelegate supply the AutofillWebDataService to core code.

components/autofill/core/browser/personal_data_manager_unittest.cc

Index: components/autofill/core/browser/personal_data_manager_unittest.cc
diff --git a/components/autofill/core/browser/personal_data_manager_unittest.cc b/components/autofill/core/browser/personal_data_manager_unittest.cc
index 6757d505afc8ee02fb75ffa66c1433fd6aa5e61b..bd91ae48e7e9bbfd9be342a630a52a05e38933d7 100644
--- a/components/autofill/core/browser/personal_data_manager_unittest.cc
+++ b/components/autofill/core/browser/personal_data_manager_unittest.cc
@@ -17,8 +17,11 @@
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/personal_data_manager_observer.h"
+#include "components/autofill/core/browser/webdata/autofill_table.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
 #include "components/autofill/core/common/form_data.h"
+#include "components/webdata/common/web_data_service_base.h"
+#include "components/webdata/common/web_database_service.h"
 #include "components/webdata/encryptor/encryptor.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -63,6 +66,21 @@ class PersonalDataManagerTest : public testing::Test {
   virtual void SetUp() {
     db_thread_.Start();
 
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    base::FilePath path = temp_dir_.path().AppendASCII("TestWebDB");
+    wdbs_ = new WebDatabaseService(
+        path,
+        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
+        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB));
+    wdbs_->AddTable(scoped_ptr<WebDatabaseTable>(new AutofillTable("en-US")));
+    wdbs_->LoadDatabase();
+    wds_ = new AutofillWebDataService(
+        wdbs_,
+        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
+        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::DB),
+        WebDataServiceBase::ProfileErrorCallback());
+    wds_->Init();
+
     profile_.reset(new TestingProfile);
     profile_->CreateWebDataService();
 
@@ -75,6 +93,11 @@ class PersonalDataManagerTest : public testing::Test {
     personal_data_.reset(NULL);
     profile_.reset(NULL);
 
+    wds_->ShutdownOnUIThread();
+    wdbs_->ShutdownDatabase();
+    wds_ = NULL;
+    wdbs_ = NULL;
+
     // Schedule another task on the DB thread to notify us that it's safe to
     // stop the thread.
     base::WaitableEvent done(false, false);
@@ -89,7 +112,7 @@ class PersonalDataManagerTest : public testing::Test {
 
   void ResetPersonalDataManager() {
     personal_data_.reset(new PersonalDataManager("en-US"));
-    personal_data_->Init(profile_.get(),
+    personal_data_->Init(scoped_refptr<AutofillWebDataService>(wds_),
                          profile_->GetPrefs(),
                          profile_->IsOffTheRecord());
     personal_data_->AddObserver(&personal_data_observer_);
@@ -110,6 +133,9 @@ class PersonalDataManagerTest : public testing::Test {
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread db_thread_;

[Ilya Sherman, 2013/11/14 23:45:55]

We should really use a TestBrowserThreadBundle rather than carefully pumping
these.  IMO we should just use the profile's AutofillWebDataService for now; and
then clean up the test in a separate, more focused change.

[blundell, 2013/11/15 16:04:54]

I looked at using the //chrome-level concepts for now. I would need to create a
TabAutofillManagerDelegate, for which this test would need to become a
ChromeTestRenderViewHostHarness so that I had a WebContents. Then the test would
have a TestBrowserThreadBundle, so I would need to eliminate the manually-setup
MessageLoops used here. I'd rather not do those changes, because it's just
taking this test in the wrong direction from eventually having it be able to be
shared by iOS (even the usage of TestBrowserThreadBundle, since that's a
//content-level concept).

What do you think?

On 2013/11/14 23:45:55, Ilya Sherman wrote:

[Jói, 2013/11/15 16:51:33]

I agree, don't go back to concepts only available in //chrome.

The ideal would be if you can get rid of the TestingProfile and have this built
in the components_unittests target instead of the unit_tests target in //chrome.

   scoped_ptr<TestingProfile> profile_;
+  scoped_refptr<AutofillWebDataService> wds_;
+  scoped_refptr<WebDatabaseService> wdbs_;

[Ilya Sherman, 2013/11/14 23:45:55]

Please give these more descriptive names, e.g. "autofill_database_service" and
"web_database".  (Unlike everywhere else that uses AutofillWebDataService, I do
think it's useful to include the "autofill_" prefix here, to contrast vs. the
"web_" prefix for the other one.)

[blundell, 2013/11/15 16:04:54]

Done.

+  base::ScopedTempDir temp_dir_;
   scoped_ptr<PersonalDataManager> personal_data_;
   PersonalDataLoadedObserverMock personal_data_observer_;
 };
@@ -535,10 +561,7 @@ TEST_F(PersonalDataManagerTest, Refresh) {
   profile_pointers.push_back(&profile2);
   AutofillProfile::AdjustInferredLabels(&profile_pointers);
 
-  scoped_refptr<AutofillWebDataService> wds =
-      AutofillWebDataService::FromBrowserContext(profile_.get());
-  ASSERT_TRUE(wds.get());
-  wds->AddAutofillProfile(profile2);
+  wds_->AddAutofillProfile(profile2);
 
   personal_data_->Refresh();
 
@@ -553,8 +576,8 @@ TEST_F(PersonalDataManagerTest, Refresh) {
   EXPECT_EQ(profile1, *results2[1]);
   EXPECT_EQ(profile2, *results2[2]);
 
-  wds->RemoveAutofillProfile(profile1.guid());
-  wds->RemoveAutofillProfile(profile2.guid());
+  wds_->RemoveAutofillProfile(profile1.guid());
+  wds_->RemoveAutofillProfile(profile2.guid());
 
   // Before telling the PDM to refresh, simulate an edit to one of the deleted
   // profiles via a SetProfile update (this would happen if the Autofill window