Replace ancient crufty AppCacheThread with much improved MessageLoopProxy usage.

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

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

10 files changed, 34 insertions, 191 deletions

diff --git a/webkit/appcache/appcache_service.cc b/webkit/appcache/appcache_service.cc
index 73552c9..b05ff01 100644
--- a/webkit/appcache/appcache_service.cc
+++ b/webkit/appcache/appcache_service.cc
@@ -434,10 +434,11 @@ AppCacheService::~AppCacheService() {
 }
 
 void AppCacheService::Initialize(const FilePath& cache_directory,
+                                 base::MessageLoopProxy* db_thread,
                                  base::MessageLoopProxy* cache_thread) {
   DCHECK(!storage_.get());
   AppCacheStorageImpl* storage = new AppCacheStorageImpl(this);
-  storage->Initialize(cache_directory, cache_thread);
+  storage->Initialize(cache_directory, db_thread, cache_thread);
   storage_.reset(storage);
 }
 
diff --git a/webkit/appcache/appcache_service.h b/webkit/appcache/appcache_service.h
index 3858295..2b8cdd4 100644
--- a/webkit/appcache/appcache_service.h
+++ b/webkit/appcache/appcache_service.h
@@ -57,6 +57,7 @@ class AppCacheService {
   virtual ~AppCacheService();
 
   void Initialize(const FilePath& cache_directory,
+                  base::MessageLoopProxy* db_thread,
                   base::MessageLoopProxy* cache_thread);
 
   // Purges any memory not needed.
diff --git a/webkit/appcache/appcache_storage_impl.cc b/webkit/appcache/appcache_storage_impl.cc
index 4f6f5fc6..0812dc1 100644
--- a/webkit/appcache/appcache_storage_impl.cc
+++ b/webkit/appcache/appcache_storage_impl.cc
@@ -23,7 +23,6 @@
 #include "webkit/appcache/appcache_quota_client.h"
 #include "webkit/appcache/appcache_response.h"
 #include "webkit/appcache/appcache_service.h"
-#include "webkit/appcache/appcache_thread.h"
 #include "webkit/quota/quota_client.h"
 #include "webkit/quota/quota_manager.h"
 #include "webkit/quota/special_storage_policy.h"
@@ -130,7 +129,10 @@ class AppCacheStorageImpl::DatabaseTask
     : public base::RefCountedThreadSafe<DatabaseTask> {
  public:
   explicit DatabaseTask(AppCacheStorageImpl* storage)
-      : storage_(storage), database_(storage->database_) {}
+      : storage_(storage), database_(storage->database_),
+        io_thread_(base::MessageLoopProxy::current()) {
+    DCHECK(io_thread_);
+  }
 
   virtual ~DatabaseTask() {}
 
@@ -167,12 +169,14 @@ class AppCacheStorageImpl::DatabaseTask
   void CallRun();
   void CallRunCompleted();
   void CallDisableStorage();
+
+  scoped_refptr<base::MessageLoopProxy> io_thread_;
 };
 
 void AppCacheStorageImpl::DatabaseTask::Schedule() {
   DCHECK(storage_);
-  DCHECK(AppCacheThread::CurrentlyOn(AppCacheThread::io()));
-  if (AppCacheThread::PostTask(AppCacheThread::db(), FROM_HERE,
+  DCHECK(io_thread_->BelongsToCurrentThread());
+  if (storage_->db_thread_->PostTask(FROM_HERE,
           NewRunnableMethod(this, &DatabaseTask::CallRun))) {
     storage_->scheduled_database_tasks_.push_back(this);
   } else {
@@ -181,27 +185,26 @@ void AppCacheStorageImpl::DatabaseTask::Schedule() {
 }
 
 void AppCacheStorageImpl::DatabaseTask::CancelCompletion() {
-  DCHECK(AppCacheThread::CurrentlyOn(AppCacheThread::io()));
+  DCHECK(io_thread_->BelongsToCurrentThread());
   delegates_.clear();
   storage_ = NULL;
 }
 
 void AppCacheStorageImpl::DatabaseTask::CallRun() {
-  DCHECK(AppCacheThread::CurrentlyOn(AppCacheThread::db()));
   if (!database_->is_disabled()) {
     Run();
     if (database_->is_disabled()) {
-      AppCacheThread::PostTask(AppCacheThread::io(), FROM_HERE,
+      io_thread_->PostTask(FROM_HERE,
           NewRunnableMethod(this, &DatabaseTask::CallDisableStorage));
     }
   }
-  AppCacheThread::PostTask(AppCacheThread::io(), FROM_HERE,
+  io_thread_->PostTask(FROM_HERE,
       NewRunnableMethod(this, &DatabaseTask::CallRunCompleted));
 }
 
 void AppCacheStorageImpl::DatabaseTask::CallRunCompleted() {
   if (storage_) {
-    DCHECK(AppCacheThread::CurrentlyOn(AppCacheThread::io()));
+    DCHECK(io_thread_->BelongsToCurrentThread());
     DCHECK(storage_->scheduled_database_tasks_.front() == this);
     storage_->scheduled_database_tasks_.pop_front();
     RunCompleted();
@@ -211,7 +214,7 @@ void AppCacheStorageImpl::DatabaseTask::CallRunCompleted() {
 
 void AppCacheStorageImpl::DatabaseTask::CallDisableStorage() {
   if (storage_) {
-    DCHECK(AppCacheThread::CurrentlyOn(AppCacheThread::io()));
+    DCHECK(io_thread_->BelongsToCurrentThread());
     storage_->Disable();
   }
 }
@@ -1163,8 +1166,7 @@ AppCacheStorageImpl::~AppCacheStorageImpl() {
                 std::mem_fun(&DatabaseTask::CancelCompletion));
 
   if (database_) {
-    AppCacheThread::PostTask(
-        AppCacheThread::db(),
+    db_thread_->PostTask(
         FROM_HERE,
         NewRunnableFunction(
             CleanUpOnDatabaseThread,
@@ -1175,9 +1177,11 @@ AppCacheStorageImpl::~AppCacheStorageImpl() {
 }
 
 void AppCacheStorageImpl::Initialize(const FilePath& cache_directory,
+                                     base::MessageLoopProxy* db_thread,
                                      base::MessageLoopProxy* cache_thread) {
+  DCHECK(db_thread);
+
   cache_directory_ = cache_directory;
-  cache_thread_ = cache_thread;
   is_incognito_ = cache_directory_.empty();
 
   FilePath db_file_path;
@@ -1185,6 +1189,9 @@ void AppCacheStorageImpl::Initialize(const FilePath& cache_directory,
     db_file_path = cache_directory_.Append(kAppCacheDatabaseName);
   database_ = new AppCacheDatabase(db_file_path);
 
+  db_thread_ = db_thread;
+  cache_thread_ = cache_thread;
+
   scoped_refptr<InitTask> task(new InitTask(this));
   task->Schedule();
 }
@@ -1640,7 +1647,7 @@ void AppCacheStorageImpl::OnDiskCacheInitialized(int rv) {
     Disable();
     if (!is_incognito_) {
       VLOG(1) << "Deleting existing appcache data and starting over.";
-      AppCacheThread::PostTask(AppCacheThread::db(), FROM_HERE,
+      db_thread_->PostTask(FROM_HERE,
           NewRunnableFunction(DeleteDirectory, cache_directory_));
     }
   }
diff --git a/webkit/appcache/appcache_storage_impl.h b/webkit/appcache/appcache_storage_impl.h
index 0901dcd..dee3bed 100644
--- a/webkit/appcache/appcache_storage_impl.h
+++ b/webkit/appcache/appcache_storage_impl.h
@@ -25,6 +25,7 @@ class AppCacheStorageImpl : public AppCacheStorage {
   virtual ~AppCacheStorageImpl();
 
   void Initialize(const FilePath& cache_directory,
+                  base::MessageLoopProxy* db_thread,
                   base::MessageLoopProxy* cache_thread);
   void Disable();
   bool is_disabled() const { return is_disabled_; }
@@ -122,9 +123,14 @@ class AppCacheStorageImpl : public AppCacheStorage {
 
   // The directory in which we place files in the file system.
   FilePath cache_directory_;
-  scoped_refptr<base::MessageLoopProxy> cache_thread_;
   bool is_incognito_;
 
+  // This class operates primarily on the io thread, but schedules
+  // its DatabaseTasks on the db thread. Seperately, the disk_cache uses
+  // the cache_thread.
+  scoped_refptr<base::MessageLoopProxy> db_thread_;
+  scoped_refptr<base::MessageLoopProxy> cache_thread_;
+
   // Structures to keep track of DatabaseTasks that are in-flight.
   DatabaseTaskQueue scheduled_database_tasks_;
   PendingCacheLoads pending_cache_loads_;
diff --git a/webkit/appcache/appcache_storage_impl_unittest.cc b/webkit/appcache/appcache_storage_impl_unittest.cc
index 1b2b5ff..cb5c7d1 100644
--- a/webkit/appcache/appcache_storage_impl_unittest.cc
+++ b/webkit/appcache/appcache_storage_impl_unittest.cc
@@ -16,7 +16,6 @@
 #include "webkit/appcache/appcache_service.h"
 #include "webkit/appcache/appcache_storage_impl.h"
 #include "webkit/quota/quota_manager.h"
-#include "webkit/tools/test_shell/simple_appcache_system.h"
 
 namespace appcache {
 
@@ -45,39 +44,9 @@ const int kDefaultEntryIdOffset = 12345;
 
 const int kMockQuota = 5000;
 
-// For the duration of this test case, we hijack the AppCacheThread API
-// calls and implement them in terms of the io and db threads created here.
-
 scoped_ptr<base::Thread> io_thread;
 scoped_ptr<base::Thread> db_thread;
 
-class TestThreadProvider : public SimpleAppCacheSystem::ThreadProvider {
- public:
-  virtual bool PostTask(
-      int id,
-      const tracked_objects::Location& from_here,
-      Task* task) {
-    GetMessageLoop(id)->PostTask(from_here, task);
-    return true;
-  }
-
-  virtual bool CurrentlyOn(int id) {
-    return MessageLoop::current() == GetMessageLoop(id);
-  }
-
-  MessageLoop* GetMessageLoop(int id) {
-    DCHECK(io_thread.get() && db_thread.get());
-    if (id == SimpleAppCacheSystem::IO_THREAD_ID)
-      return io_thread->message_loop();
-    if (id == SimpleAppCacheSystem::DB_THREAD_ID)
-      return db_thread->message_loop();
-    NOTREACHED() << "Invalid AppCacheThreadID value";
-    return NULL;
-  }
-};
-
-TestThreadProvider thread_provider;
-
 }  // namespace
 
 class AppCacheStorageImplTest : public testing::Test {
@@ -264,12 +233,9 @@ class AppCacheStorageImplTest : public testing::Test {
 
     db_thread.reset(new base::Thread("AppCacheTest::DBThread"));
     ASSERT_TRUE(db_thread->Start());
-
-    SimpleAppCacheSystem::set_thread_provider(&thread_provider);
   }
 
   static void TearDownTestCase() {
-    SimpleAppCacheSystem::set_thread_provider(NULL);
     io_thread.reset(NULL);
     db_thread.reset(NULL);
   }
@@ -290,7 +256,7 @@ class AppCacheStorageImplTest : public testing::Test {
   void SetUpTest() {
     DCHECK(MessageLoop::current() == io_thread->message_loop());
     service_.reset(new AppCacheService(NULL));
-    service_->Initialize(FilePath(), NULL);
+    service_->Initialize(FilePath(), db_thread->message_loop_proxy(), NULL);
     mock_quota_manager_proxy_ = new MockQuotaManagerProxy();
     service_->quota_manager_proxy_ = mock_quota_manager_proxy_;
     delegate_.reset(new MockStorageDelegate(this));
diff --git a/webkit/appcache/appcache_thread.cc b/webkit/appcache/appcache_thread.cc
deleted file mode 100644
index 579cb53..0000000
--- a/webkit/appcache/appcache_thread.cc
+++ /dev/null
@@ -1,13 +0,0 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "webkit/appcache/appcache_thread.h"
-
-namespace appcache {
-
-// static
-int AppCacheThread::db_;
-int AppCacheThread::io_;
-
-}  // namespace appcache
diff --git a/webkit/appcache/appcache_thread.h b/webkit/appcache/appcache_thread.h
deleted file mode 100644
index b4e46ca..0000000
--- a/webkit/appcache/appcache_thread.h
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef WEBKIT_APPCACHE_APPCACHE_THREAD_H_
-#define WEBKIT_APPCACHE_APPCACHE_THREAD_H_
-
-#include "base/task.h"
-
-namespace tracked_objects {
-class Location;
-}
-
-namespace appcache {
-
-// The appcache system uses two threads, an IO thread and a DB thread.
-// It does not create these threads, the embedder is responsible for
-// providing them to the appcache library by providing a concrete
-// implementation of the PostTask and CurrentlyOn methods declared here,
-// and by calling the Init method prior to using the appcache library.
-class AppCacheThread {
- public:
-  static void Init(int db, int io) {
-    db_ = db;
-    io_ = io;
-  }
-  static int db() { return db_; }
-  static int io() { return io_; }
-
-  static bool PostTask(int id,
-                       const tracked_objects::Location& from_here,
-                       Task* task);
-  static bool CurrentlyOn(int id);
-
-  template <class T>
-  static bool DeleteSoon(int id,
-                         const tracked_objects::Location& from_here,
-                         T* object) {
-    return PostTask(id, from_here, new DeleteTask<T>(object));
-  }
-
- private:
-  AppCacheThread();
-  ~AppCacheThread();
-
-  static int db_;
-  static int io_;
-};
-
-}  // namespace appcache
-
-#endif  // WEBKIT_APPCACHE_APPCACHE_THREAD_H_
diff --git a/webkit/appcache/webkit_appcache.gypi b/webkit/appcache/webkit_appcache.gypi
index 93383a7..cfce447 100644
--- a/webkit/appcache/webkit_appcache.gypi
+++ b/webkit/appcache/webkit_appcache.gypi
@@ -48,8 +48,6 @@
         'appcache_storage.h',
         'appcache_storage_impl.cc',
         'appcache_storage_impl.h',
-        'appcache_thread.cc',
-        'appcache_thread.h',
         'appcache_working_set.cc',
         'appcache_working_set.h',
         'appcache_update_job.cc',
diff --git a/webkit/tools/test_shell/simple_appcache_system.cc b/webkit/tools/test_shell/simple_appcache_system.cc
index 93d70cf..6a8d454 100644
--- a/webkit/tools/test_shell/simple_appcache_system.cc
+++ b/webkit/tools/test_shell/simple_appcache_system.cc
@@ -19,34 +19,6 @@ using WebKit::WebApplicationCacheHostClient;
 using appcache::WebApplicationCacheHostImpl;
 using appcache::AppCacheBackendImpl;
 using appcache::AppCacheInterceptor;
-using appcache::AppCacheThread;
-
-namespace appcache {
-
-// An impl of AppCacheThread we need to provide to the appcache lib.
-
-bool AppCacheThread::PostTask(
-    int id,
-    const tracked_objects::Location& from_here,
-    Task* task) {
-  if (SimpleAppCacheSystem::thread_provider()) {
-    return SimpleAppCacheSystem::thread_provider()->PostTask(
-        id, from_here, task);
-  }
-  scoped_ptr<Task> task_ptr(task);
-  MessageLoop* loop = SimpleAppCacheSystem::GetMessageLoop(id);
-  if (loop)
-    loop->PostTask(from_here, task_ptr.release());
-  return loop ? true : false;
-}
-
-bool AppCacheThread::CurrentlyOn(int id) {
-  if (SimpleAppCacheSystem::thread_provider())
-    return SimpleAppCacheSystem::thread_provider()->CurrentlyOn(id);
-  return MessageLoop::current() == SimpleAppCacheSystem::GetMessageLoop(id);
-}
-
-}  // namespace appcache
 
 // SimpleFrontendProxy --------------------------------------------------------
 // Proxies method calls from the backend IO thread to the frontend UI thread.
@@ -366,8 +338,7 @@ SimpleAppCacheSystem::SimpleAppCacheSystem()
           backend_proxy_(new SimpleBackendProxy(this))),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           frontend_proxy_(new SimpleFrontendProxy(this))),
-      backend_impl_(NULL), service_(NULL), db_thread_("AppCacheDBThread"),
-      thread_provider_(NULL) {
+      backend_impl_(NULL), service_(NULL), db_thread_("AppCacheDBThread") {
   DCHECK(!instance_);
   instance_ = this;
 }
@@ -393,7 +364,6 @@ SimpleAppCacheSystem::~SimpleAppCacheSystem() {
 
 void SimpleAppCacheSystem::InitOnUIThread(const FilePath& cache_directory) {
   DCHECK(!ui_message_loop_);
-  AppCacheThread::Init(DB_THREAD_ID, IO_THREAD_ID);
   ui_message_loop_ = MessageLoop::current();
   cache_directory_ = cache_directory;
 }
@@ -413,6 +383,7 @@ void SimpleAppCacheSystem::InitOnIOThread(
   service_ = new appcache::AppCacheService(NULL);
   backend_impl_ = new appcache::AppCacheBackendImpl();
   service_->Initialize(cache_directory_,
+                       db_thread_.message_loop_proxy(),
                        SimpleResourceLoaderBridge::GetCacheThread());
   service_->set_request_context(request_context);
   backend_impl_->Initialize(service_, frontend_proxy_.get(), kSingleProcessId);
diff --git a/webkit/tools/test_shell/simple_appcache_system.h b/webkit/tools/test_shell/simple_appcache_system.h
index d850051..2975268a 100644
--- a/webkit/tools/test_shell/simple_appcache_system.h
+++ b/webkit/tools/test_shell/simple_appcache_system.h
@@ -11,7 +11,6 @@
 #include "webkit/appcache/appcache_backend_impl.h"
 #include "webkit/appcache/appcache_frontend_impl.h"
 #include "webkit/appcache/appcache_service.h"
-#include "webkit/appcache/appcache_thread.h"
 #include "webkit/glue/resource_type.h"
 
 namespace WebKit {
@@ -81,37 +80,9 @@ class SimpleAppCacheSystem {
       instance_->GetExtraResponseBits(request, cache_id, manifest_url);
   }
 
-  // Some unittests create their own IO and DB threads.
-
-  enum AppCacheThreadID {
-    DB_THREAD_ID,
-    IO_THREAD_ID,
-  };
-
-  class ThreadProvider {
-   public:
-    virtual ~ThreadProvider() {}
-    virtual bool PostTask(
-        int id,
-        const tracked_objects::Location& from_here,
-        Task* task) = 0;
-    virtual bool CurrentlyOn(int id) = 0;
-  };
-
-  static void set_thread_provider(ThreadProvider* provider) {
-    DCHECK(instance_);
-    DCHECK(!provider || !instance_->thread_provider_);
-    instance_->thread_provider_ = provider;
-  }
-
-  static ThreadProvider* thread_provider() {
-    return instance_ ? instance_->thread_provider_ : NULL;
-  }
-
  private:
   friend class SimpleBackendProxy;
   friend class SimpleFrontendProxy;
-  friend class appcache::AppCacheThread;
 
   // Instance methods called by our static public methods
   void InitOnUIThread(const FilePath& cache_directory);
@@ -137,16 +108,6 @@ class SimpleAppCacheSystem {
   bool is_initailized_on_ui_thread() {
     return ui_message_loop_ ? true : false;
   }
-  static MessageLoop* GetMessageLoop(int id) {
-    if (instance_) {
-      if (id == IO_THREAD_ID)
-        return instance_->io_message_loop_;
-      if (id == DB_THREAD_ID)
-        return instance_->db_thread_.message_loop();
-      NOTREACHED() << "Invalid AppCacheThreadID value";
-    }
-    return NULL;
-  }
 
   FilePath cache_directory_;
   MessageLoop* io_message_loop_;
@@ -164,9 +125,6 @@ class SimpleAppCacheSystem {
   // We start a thread for use as the DB thread.
   base::Thread db_thread_;
 
-  // Some unittests create there own IO and DB threads.
-  ThreadProvider* thread_provider_;
-
   // A low-tech singleton.
   static SimpleAppCacheSystem* instance_;
 };