Revert 35339 - Change WDS to use the DB thread rather than its own thread.

This cleanup was requested by brettw and was started to make it easier for the sync service to post tasks to the WDS thread (now the DB thread).  This simplifies the WDS a bit since it no longer has to manage its own thread, and can assume that the DB thread is running throughout its lifetime.

One change in behavior that is significant is that previous to this change, the WDS worker thread would always be joined when Shutdown() was called from Profile::~Profile().  Now the Shutdown() method schedules a task that can extend the lifetime of the WDS past the lifetime of the Profile instance.

Review URL: http://codereview.chromium.org/524003

TBR=skrul@chromium.org
Review URL: http://codereview.chromium.org/521013

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

3 files changed, 43 insertions, 21 deletions

diff --git a/chrome/browser/webdata/web_data_service.cc b/chrome/browser/webdata/web_data_service.cc
index d25492b..4407cdd 100644
--- a/chrome/browser/webdata/web_data_service.cc
+++ b/chrome/browser/webdata/web_data_service.cc
@@ -29,7 +29,7 @@ using webkit_glue::FormField;
 using webkit_glue::PasswordForm;
 
 WebDataService::WebDataService()
-  : is_running_(false),
+  : thread_(NULL),
     db_(NULL),
     failed_init_(false),
     should_commit_(false),
@@ -38,8 +38,8 @@ WebDataService::WebDataService()
 }
 
 WebDataService::~WebDataService() {
-  if (is_running_ && db_) {
-    DLOG_ASSERT("WebDataService dtor called without Shutdown");
+  if (thread_) {
+    Shutdown();
   }
 }
 
@@ -51,18 +51,47 @@ bool WebDataService::Init(const FilePath& profile_path) {
 
 bool WebDataService::InitWithPath(const FilePath& path) {
   path_ = path;
-  is_running_ = true;
+
+  thread_ = new base::Thread("Chrome_WebDataThread");
+  if (!thread_->Start()) {
+    delete thread_;
+    thread_ = NULL;
+    return false;
+  }
+
   ScheduleTask(NewRunnableMethod(this,
       &WebDataService::InitializeDatabaseIfNecessary));
   return true;
 }
 
+class ShutdownTask : public Task {
+ public:
+  explicit ShutdownTask(WebDataService* wds) : service_(wds) {
+  }
+  virtual void Run() {
+    service_->ShutdownDatabase();
+  }
+
+ private:
+
+  WebDataService* service_;
+};
+
 void WebDataService::Shutdown() {
-  UnloadDatabase();
+  if (thread_) {
+    // We cannot use NewRunnableMethod() because this can be called from our
+    // destructor. NewRunnableMethod() would AddRef() this instance.
+    ScheduleTask(new ShutdownTask(this));
+
+    // The thread destructor sends a message to terminate the thread and waits
+    // until the thread has exited.
+    delete thread_;
+    thread_ = NULL;
+  }
 }
 
 bool WebDataService::IsRunning() const {
-  return is_running_;
+  return thread_ != NULL;
 }
 
 void WebDataService::UnloadDatabase() {
@@ -77,8 +106,8 @@ void WebDataService::ScheduleCommit() {
 }
 
 void WebDataService::ScheduleTask(Task* t) {
-  if (is_running_)
-    ChromeThread::PostTask(ChromeThread::DB, FROM_HERE, t);
+  if (thread_)
+    thread_->message_loop()->PostTask(FROM_HERE, t);
   else
     NOTREACHED() << "Task scheduled after Shutdown()";
 }
diff --git a/chrome/browser/webdata/web_data_service.h b/chrome/browser/webdata/web_data_service.h
index 4421b71..3a6c83b 100644
--- a/chrome/browser/webdata/web_data_service.h
+++ b/chrome/browser/webdata/web_data_service.h
@@ -12,7 +12,6 @@
 #include "base/file_path.h"
 #include "base/lock.h"
 #include "base/ref_counted.h"
-#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "webkit/glue/form_field.h"
@@ -146,9 +145,7 @@ template <class T> class WDObjectResult : public WDTypedResult {
 
 class WebDataServiceConsumer;
 
-class WebDataService
-    : public base::RefCountedThreadSafe<WebDataService,
-                                        ChromeThread::DeleteOnUIThread> {
+class WebDataService : public base::RefCountedThreadSafe<WebDataService> {
  public:
 
   // All requests return an opaque handle of the following type.
@@ -432,8 +429,6 @@ class WebDataService
   //////////////////////////////////////////////////////////////////////////////
  private:
   friend class base::RefCountedThreadSafe<WebDataService>;
-  friend class ChromeThread;
-  friend class DeleteTask<WebDataService>;
   friend class ShutdownTask;
 
   typedef GenericRequest2<std::vector<const TemplateURL*>,
@@ -512,6 +507,8 @@ class WebDataService
 
   void GetWebAppImagesImpl(GenericRequest<GURL>* request);
 
+  base::Thread* thread() { return thread_; }
+
   // Schedule a task on our worker thread.
   void ScheduleTask(Task* t);
 
@@ -521,8 +518,8 @@ class WebDataService
   // Return the next request handle.
   int GetNextRequestHandle();
 
-  // True once initialization has started.
-  bool is_running_;
+  // Our worker thread. All requests are processed from that thread.
+  base::Thread* thread_;
 
   // The path with which to initialize the database.
   FilePath path_;
diff --git a/chrome/browser/webdata/web_data_service_unittest.cc b/chrome/browser/webdata/web_data_service_unittest.cc
index 7c92725..1e26539 100644
--- a/chrome/browser/webdata/web_data_service_unittest.cc
+++ b/chrome/browser/webdata/web_data_service_unittest.cc
@@ -71,12 +71,10 @@ class AutofillWebDataServiceConsumer: public WebDataServiceConsumer {
 class WebDataServiceTest : public testing::Test {
  public:
   WebDataServiceTest()
-      : ui_thread_(ChromeThread::UI, &message_loop_),
-        db_thread_(ChromeThread::DB) {}
+      : ui_thread_(ChromeThread::UI, &message_loop_) {}
 
  protected:
   virtual void SetUp() {
-    db_thread_.Start();
     name1_ = ASCIIToUTF16("name1");
     name2_ = ASCIIToUTF16("name2");
     value1_ = ASCIIToUTF16("value1");
@@ -96,7 +94,6 @@ class WebDataServiceTest : public testing::Test {
       wds_->Shutdown();
     file_util::Delete(profile_dir_, true);
 
-    db_thread_.Stop();
     MessageLoop::current()->PostTask(FROM_HERE, new MessageLoop::QuitTask);
     MessageLoop::current()->Run();
   }
@@ -113,7 +110,6 @@ class WebDataServiceTest : public testing::Test {
 
   MessageLoopForUI message_loop_;
   ChromeThread ui_thread_;
-  ChromeThread db_thread_;
   string16 name1_;
   string16 name2_;
   string16 value1_;