Fix leak from cl 524003 (Change WDS to use the DB thread...)

This is a second try of cl 524003 -- the previous version caused a leak.  The only change in this patch is the addition of the ui_thread_ member in TemplateURLModelTest and the corresponding initializer.  This allows the DeleteOnUIThread trait of the WebDataService to do its job.

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

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

3 files changed, 21 insertions, 43 deletions

diff --git a/chrome/browser/webdata/web_data_service.cc b/chrome/browser/webdata/web_data_service.cc
index 4407cdd..d25492b 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()
-  : thread_(NULL),
+  : is_running_(false),
     db_(NULL),
     failed_init_(false),
     should_commit_(false),
@@ -38,8 +38,8 @@ WebDataService::WebDataService()
 }
 
 WebDataService::~WebDataService() {
-  if (thread_) {
-    Shutdown();
+  if (is_running_ && db_) {
+    DLOG_ASSERT("WebDataService dtor called without Shutdown");
   }
 }
 
@@ -51,47 +51,18 @@ bool WebDataService::Init(const FilePath& profile_path) {
 
 bool WebDataService::InitWithPath(const FilePath& path) {
   path_ = path;
-
-  thread_ = new base::Thread("Chrome_WebDataThread");
-  if (!thread_->Start()) {
-    delete thread_;
-    thread_ = NULL;
-    return false;
-  }
-
+  is_running_ = true;
   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() {
-  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;
-  }
+  UnloadDatabase();
 }
 
 bool WebDataService::IsRunning() const {
-  return thread_ != NULL;
+  return is_running_;
 }
 
 void WebDataService::UnloadDatabase() {
@@ -106,8 +77,8 @@ void WebDataService::ScheduleCommit() {
 }
 
 void WebDataService::ScheduleTask(Task* t) {
-  if (thread_)
-    thread_->message_loop()->PostTask(FROM_HERE, t);
+  if (is_running_)
+    ChromeThread::PostTask(ChromeThread::DB, 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 3a6c83b..4421b71 100644
--- a/chrome/browser/webdata/web_data_service.h
+++ b/chrome/browser/webdata/web_data_service.h
@@ -12,6 +12,7 @@
 #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"
@@ -145,7 +146,9 @@ template <class T> class WDObjectResult : public WDTypedResult {
 
 class WebDataServiceConsumer;
 
-class WebDataService : public base::RefCountedThreadSafe<WebDataService> {
+class WebDataService
+    : public base::RefCountedThreadSafe<WebDataService,
+                                        ChromeThread::DeleteOnUIThread> {
  public:
 
   // All requests return an opaque handle of the following type.
@@ -429,6 +432,8 @@ class WebDataService : public base::RefCountedThreadSafe<WebDataService> {
   //////////////////////////////////////////////////////////////////////////////
  private:
   friend class base::RefCountedThreadSafe<WebDataService>;
+  friend class ChromeThread;
+  friend class DeleteTask<WebDataService>;
   friend class ShutdownTask;
 
   typedef GenericRequest2<std::vector<const TemplateURL*>,
@@ -507,8 +512,6 @@ class WebDataService : public base::RefCountedThreadSafe<WebDataService> {
 
   void GetWebAppImagesImpl(GenericRequest<GURL>* request);
 
-  base::Thread* thread() { return thread_; }
-
   // Schedule a task on our worker thread.
   void ScheduleTask(Task* t);
 
@@ -518,8 +521,8 @@ class WebDataService : public base::RefCountedThreadSafe<WebDataService> {
   // Return the next request handle.
   int GetNextRequestHandle();
 
-  // Our worker thread. All requests are processed from that thread.
-  base::Thread* thread_;
+  // True once initialization has started.
+  bool is_running_;
 
   // 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 1e26539..7c92725 100644
--- a/chrome/browser/webdata/web_data_service_unittest.cc
+++ b/chrome/browser/webdata/web_data_service_unittest.cc
@@ -71,10 +71,12 @@ class AutofillWebDataServiceConsumer: public WebDataServiceConsumer {
 class WebDataServiceTest : public testing::Test {
  public:
   WebDataServiceTest()
-      : ui_thread_(ChromeThread::UI, &message_loop_) {}
+      : ui_thread_(ChromeThread::UI, &message_loop_),
+        db_thread_(ChromeThread::DB) {}
 
  protected:
   virtual void SetUp() {
+    db_thread_.Start();
     name1_ = ASCIIToUTF16("name1");
     name2_ = ASCIIToUTF16("name2");
     value1_ = ASCIIToUTF16("value1");
@@ -94,6 +96,7 @@ 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();
   }
@@ -110,6 +113,7 @@ class WebDataServiceTest : public testing::Test {
 
   MessageLoopForUI message_loop_;
   ChromeThread ui_thread_;
+  ChromeThread db_thread_;
   string16 name1_;
   string16 name2_;
   string16 value1_;