Taking over issue 10006037: Moved WebDataService to ProfileKeyedService

chrome/browser/search_engines/template_url_service_test_util.cc

Index: chrome/browser/search_engines/template_url_service_test_util.cc
===================================================================
--- chrome/browser/search_engines/template_url_service_test_util.cc	(revision 136024)
+++ chrome/browser/search_engines/template_url_service_test_util.cc	(working copy)
@@ -8,10 +8,12 @@
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
 #include "chrome/browser/search_engines/search_terms_data.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/browser/webdata/web_data_service_factory.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service.h"
@@ -62,9 +64,8 @@
     io_thread_.StartIOThread();
   }
 
-  virtual WebDataService* GetWebDataService(ServiceAccessType access) {
-    return service_.get();
-  }
+  static scoped_refptr<RefcountedProfileKeyedService>
+      GetWebDataServiceForTemplateURLServiceTestingProfile(Profile* profile);
 
  private:
   scoped_refptr<WebDataService> service_;
@@ -73,6 +74,7 @@
   content::TestBrowserThread io_thread_;
 };
 
+
 // Trivial subclass of TemplateURLService that records the last invocation of
 // SetKeywordSearchTermsForURL.
 class TestingTemplateURLService : public TemplateURLService {
@@ -128,15 +130,31 @@
   io_thread_.Stop();
 
   // Clean up the test directory.
-  if (service_.get())
-    service_->Shutdown();
+  if (service_.get()) {
+    service_->ShutdownOnUIThread();
+    service_ = NULL;
+  }
   // Note that we must ensure the DB thread is stopped after WDS
   // shutdown (so it can commit pending transactions) but before
   // deleting the test profile directory, otherwise we may not be
   // able to delete it due to an open transaction.
+  // Schedule another task on the DB thread to notify us that it's safe to
+  // carry on with the test.
+  base::WaitableEvent done(false, false);
+  BrowserThread::PostTask(BrowserThread::DB, FROM_HERE,
+      base::Bind(&base::WaitableEvent::Signal, base::Unretained(&done)));
+  done.Wait();
   db_thread_.Stop();
 }
 
+scoped_refptr<RefcountedProfileKeyedService>
+TemplateURLServiceTestingProfile::
+    GetWebDataServiceForTemplateURLServiceTestingProfile(Profile* profile) {
+  TemplateURLServiceTestingProfile* test_profile =
+      reinterpret_cast<TemplateURLServiceTestingProfile*>(profile);
+  return test_profile->service_;

[Peter Kasting, 2012/05/11 22:29:32]

Nit: Should we move the temp dir/service init from SetUp() to here?  I'm not
sure this would work... I'm just looking for ways to make the ownership and
construction patterns here "more typical"

[rpetterson, 2012/05/11 23:09:10]

Yes, the way you describe makes more logical sense. However, they are doing some
unique things with the creation and deletion of the service that would require
more invasive changes to fix it. Seeing as this CL is already huge, I'd rather
wait to make those changes in a separate CL.

+}
+
 TemplateURLServiceTestUtil::TemplateURLServiceTestUtil()
     : ui_thread_(BrowserThread::UI, &message_loop_),
       changed_count_(0) {
@@ -147,6 +165,10 @@
 
 void TemplateURLServiceTestUtil::SetUp() {
   profile_.reset(new TemplateURLServiceTestingProfile());
+  WebDataServiceFactory::GetInstance()->SetTestingFactory(
+      profile_.get(), TemplateURLServiceTestingProfile::
+          GetWebDataServiceForTemplateURLServiceTestingProfile);
+
   profile_->SetUp();
   TemplateURLService* service = static_cast<TemplateURLService*>(
       TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse(
@@ -197,7 +219,8 @@
   model()->ChangeToLoadedState();
   // Initialize the web data service so that the database gets updated with
   // any changes made.
-  model()->service_ = profile_->GetWebDataService(Profile::EXPLICIT_ACCESS);
+  model()->service_ = WebDataServiceFactory::GetForProfile(
+      profile_.get(), Profile::EXPLICIT_ACCESS);
 }
 
 void TemplateURLServiceTestUtil::ClearModel() {