RESUBMIT of http://codereview.chromium.org/404025/show

Start the WebKit thread when we initialize the resource
dispatcher host.

NOTE: It's very possible this will have a startup
performance impact. I'm going to watch the bots after
committing.

TODO: Clean up shutdown logic that exists simply
because we don't know whether or not we'll ever start
the WebKit thread.

Originally landed in r33063, reverted in r33101 due to
coincidence with Modules Linux startup test failures.

TBR=jorlow, darin
TEST=The WebKit thread gets spun up early in the
initialization process.
BUG=24144,28364

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

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

6 files changed, 45 insertions, 71 deletions

diff --git a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
index 75b9bfc..d295ce1 100644
--- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
+++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc
@@ -150,9 +150,9 @@ void DOMStorageDispatcherHost::Send(IPC::Message* message) {
 void DOMStorageDispatcherHost::OnNamespaceId(DOMStorageType storage_type,
                                              IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnNamespaceId,
-        storage_type, reply_msg));
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnNamespaceId, storage_type,
+        reply_msg));
     return;
   }
 
@@ -170,9 +170,9 @@ void DOMStorageDispatcherHost::OnNamespaceId(DOMStorageType storage_type,
 void DOMStorageDispatcherHost::OnCloneNamespaceId(int64 namespace_id,
                                                   IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnCloneNamespaceId,
-        namespace_id, reply_msg));
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnCloneNamespaceId, namespace_id,
+        reply_msg));
     return;
   }
 
@@ -195,9 +195,9 @@ void DOMStorageDispatcherHost::OnStorageAreaId(int64 namespace_id,
                                                const string16& origin,
                                                IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnStorageAreaId,
-        namespace_id, origin, reply_msg));
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnStorageAreaId, namespace_id, origin,
+        reply_msg));
     return;
   }
 
@@ -219,8 +219,8 @@ void DOMStorageDispatcherHost::OnStorageAreaId(int64 namespace_id,
 void DOMStorageDispatcherHost::OnLength(int64 storage_area_id,
                                         IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnLength, storage_area_id, reply_msg));
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnLength, storage_area_id, reply_msg));
     return;
   }
 
@@ -240,8 +240,9 @@ void DOMStorageDispatcherHost::OnLength(int64 storage_area_id,
 void DOMStorageDispatcherHost::OnKey(int64 storage_area_id, unsigned index,
                                      IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnKey, storage_area_id, index, reply_msg));
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnKey, storage_area_id, index,
+        reply_msg));
     return;
   }
 
@@ -262,9 +263,9 @@ void DOMStorageDispatcherHost::OnGetItem(int64 storage_area_id,
                                          const string16& key,
                                          IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnGetItem,
-        storage_area_id, key, reply_msg));
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnGetItem, storage_area_id, key,
+        reply_msg));
     return;
   }
 
@@ -285,8 +286,8 @@ void DOMStorageDispatcherHost::OnSetItem(
     int64 storage_area_id, const string16& key, const string16& value,
     const GURL& url, IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnSetItem, storage_area_id, key, value,
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnSetItem, storage_area_id, key, value,
         url, reply_msg));
     return;
   }
@@ -309,8 +310,9 @@ void DOMStorageDispatcherHost::OnSetItem(
 void DOMStorageDispatcherHost::OnRemoveItem(
     int64 storage_area_id, const string16& key, const GURL& url) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnRemoveItem, storage_area_id, key, url));
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnRemoveItem, storage_area_id, key,
+        url));
     return;
   }
 
@@ -328,8 +330,8 @@ void DOMStorageDispatcherHost::OnRemoveItem(
 
 void DOMStorageDispatcherHost::OnClear(int64 storage_area_id, const GURL& url) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
-        &DOMStorageDispatcherHost::OnClear, storage_area_id, url));
+    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
+        this, &DOMStorageDispatcherHost::OnClear, storage_area_id, url));
     return;
   }
 
@@ -356,9 +358,3 @@ void DOMStorageDispatcherHost::OnStorageEvent(
     ++cur;
   }
 }
-
-void DOMStorageDispatcherHost::PostTaskToWebKitThread(
-    const tracked_objects::Location& from_here, Task* task) {
-  webkit_thread_->EnsureInitialized();
-  ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, task);
-}
diff --git a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
index b99ac94..6ca2d62 100644
--- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
+++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
@@ -76,10 +76,6 @@ class DOMStorageDispatcherHost
     return webkit_context_->dom_storage_context();
   }
 
-  // Posts a task to the WebKit thread, initializing it if necessary.
-  void PostTaskToWebKitThread(
-      const tracked_objects::Location& from_here, Task* task);
-
   // Use whenever there's a chance OnStorageEvent will be called.
   class ScopedStorageEventContext {
    public:
diff --git a/chrome/browser/in_process_webkit/webkit_thread.cc b/chrome/browser/in_process_webkit/webkit_thread.cc
index 6311140..c3ad81f 100644
--- a/chrome/browser/in_process_webkit/webkit_thread.cc
+++ b/chrome/browser/in_process_webkit/webkit_thread.cc
@@ -9,9 +9,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebKit.h"
 
-// This happens on the UI thread before the IO thread has been shut down.
 WebKitThread::WebKitThread() {
-  // The thread is started lazily by InitializeThread() on the IO thread.
 }
 
 // This happens on the UI thread after the IO thread has been shut down.
@@ -23,11 +21,19 @@ WebKitThread::~WebKitThread() {
   DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::WEBKIT));
 }
 
-void WebKitThread::EnsureInitialized() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  if (webkit_thread_.get())
+void WebKitThread::Initialize() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  DCHECK(!webkit_thread_.get());
+
+  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess)) {
+    // TODO(jorlow): This thread should be used (and started) in single process
+    //               mode rather than following different code paths.
     return;
-  InitializeThread();
+  }
+
+  webkit_thread_.reset(new InternalWebKitThread);
+  bool started = webkit_thread_->Start();
+  DCHECK(started);
 }
 
 WebKitThread::InternalWebKitThread::InternalWebKitThread()
@@ -43,21 +49,10 @@ void WebKitThread::InternalWebKitThread::Init() {
   webkit_client_.reset(new BrowserWebKitClientImpl);
   WebKit::initialize(webkit_client_.get());
   // If possible, post initialization tasks to this thread (rather than doing
-  // them now) so we don't block the IO thread any longer than we have to.
+  // them now) so we don't block the UI thread any longer than we have to.
 }
 
 void WebKitThread::InternalWebKitThread::CleanUp() {
   DCHECK(webkit_client_.get());
   WebKit::shutdown();
 }
-
-MessageLoop* WebKitThread::InitializeThread() {
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess))
-    return NULL;
-
-  DCHECK(!webkit_thread_.get());
-  webkit_thread_.reset(new InternalWebKitThread);
-  bool started = webkit_thread_->Start();
-  DCHECK(started);
-  return webkit_thread_->message_loop();
-}
diff --git a/chrome/browser/in_process_webkit/webkit_thread.h b/chrome/browser/in_process_webkit/webkit_thread.h
index 97b1856..9838730 100644
--- a/chrome/browser/in_process_webkit/webkit_thread.h
+++ b/chrome/browser/in_process_webkit/webkit_thread.h
@@ -14,20 +14,14 @@
 
 class BrowserWebKitClientImpl;
 
-// This is an object that represents WebKit's "main" thread within the browser
-// process.  It should be instantiated and destroyed on the UI thread
-// before/after the IO thread is created/destroyed.  All other usage should be
-// on the IO thread.  If the browser is being run in --single-process mode, a
-// thread will never be spun up.
+// This creates a WebKit main thread on instantiation (if not in
+// --single-process mode) on construction and kills it on deletion.
 class WebKitThread {
  public:
   // Called from the UI thread.
   WebKitThread();
   ~WebKitThread();
-
-  // Creates the WebKit thread if it hasn't been already created.  Only call
-  // from the IO thread.  Only do fast-path work here.
-  void EnsureInitialized();
+  void Initialize();
 
  private:
   // Must be private so that we can carefully control its lifetime.
@@ -45,12 +39,7 @@ class WebKitThread {
     scoped_ptr<BrowserWebKitClientImpl> webkit_client_;
   };
 
-  // Returns the WebKit thread's message loop or NULL if we're in
-  // --single-process mode.  Do slow-path initialization work here.
-  MessageLoop* InitializeThread();
-
-  // Pointer to the actual WebKitThread.  NULL if not yet started.  Only modify
-  // from the IO thread while the WebKit thread is not running.
+  // Pointer to the actual WebKitThread.
   scoped_ptr<InternalWebKitThread> webkit_thread_;
 
   DISALLOW_COPY_AND_ASSIGN(WebKitThread);
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc
index 3b0f99e..7203cd9 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host.cc
+++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc
@@ -286,6 +286,7 @@ ResourceDispatcherHost::~ResourceDispatcherHost() {
 
 void ResourceDispatcherHost::Initialize() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  webkit_thread_->Initialize();
   safe_browsing_->Initialize();
   ChromeThread::PostTask(
       ChromeThread::IO, FROM_HERE,
diff --git a/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc b/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc
index 301a2a1..4feed71 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc
+++ b/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc
@@ -320,8 +320,7 @@ void CheckSuccessfulRequest(const std::vector<IPC::Message>& messages,
 }
 
 // Tests whether many messages get dispatched properly.
-// TODO(jorlow): Re-enable once bug 28364 is fixed.
-TEST_F(ResourceDispatcherHostTest, DISABLED_TestMany) {
+TEST_F(ResourceDispatcherHostTest, TestMany) {
   EXPECT_EQ(0, host_.GetOutstandingRequestsMemoryCost(0));
 
   MakeTestRequest(0, 1, URLRequestTestJob::test_url_1());
@@ -347,8 +346,7 @@ TEST_F(ResourceDispatcherHostTest, DISABLED_TestMany) {
 
 // Tests whether messages get canceled properly. We issue three requests,
 // cancel one of them, and make sure that each sent the proper notifications.
-// TODO(jorlow): Re-enable once bug 28364 is fixed.
-TEST_F(ResourceDispatcherHostTest, DISABLED_Cancel) {
+TEST_F(ResourceDispatcherHostTest, Cancel) {
   EXPECT_EQ(0, host_.GetOutstandingRequestsMemoryCost(0));
 
   MakeTestRequest(0, 1, URLRequestTestJob::test_url_1());
@@ -608,8 +606,7 @@ TEST_F(ResourceDispatcherHostTest, TestBlockedRequestsProcessDies) {
 // away.  Note that we rely on Purify for finding the leaks if any.
 // If this test turns the Purify bot red, check the ResourceDispatcherHost
 // destructor to make sure the blocked requests are deleted.
-// TODO(jorlow): Re-enable once bug 28364 is fixed.
-TEST_F(ResourceDispatcherHostTest, DISABLED_TestBlockedRequestsDontLeak) {
+TEST_F(ResourceDispatcherHostTest, TestBlockedRequestsDontLeak) {
   // This second receiver is used to emulate a second process.
   ForwardingReceiver second_receiver(this);