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

There is a flaky failure in Linux startup tests that
started appearing after r33063 landed.  Try backing
out r33063 to see if this CL is the cause.

BUG=24144
TEST=linux startup tests pass
TBR=jorlow

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

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

6 files changed, 71 insertions, 45 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 d295ce1..75b9bfc 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)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnNamespaceId, storage_type,
-        reply_msg));
+    PostTaskToWebKitThread(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)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnCloneNamespaceId, namespace_id,
-        reply_msg));
+    PostTaskToWebKitThread(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)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnStorageAreaId, namespace_id, origin,
-        reply_msg));
+    PostTaskToWebKitThread(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)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnLength, storage_area_id, reply_msg));
+    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
+        &DOMStorageDispatcherHost::OnLength, storage_area_id, reply_msg));
     return;
   }
 
@@ -240,9 +240,8 @@ 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)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnKey, storage_area_id, index,
-        reply_msg));
+    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
+        &DOMStorageDispatcherHost::OnKey, storage_area_id, index, reply_msg));
     return;
   }
 
@@ -263,9 +262,9 @@ void DOMStorageDispatcherHost::OnGetItem(int64 storage_area_id,
                                          const string16& key,
                                          IPC::Message* reply_msg) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnGetItem, storage_area_id, key,
-        reply_msg));
+    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
+        &DOMStorageDispatcherHost::OnGetItem,
+        storage_area_id, key, reply_msg));
     return;
   }
 
@@ -286,8 +285,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)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnSetItem, storage_area_id, key, value,
+    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
+        &DOMStorageDispatcherHost::OnSetItem, storage_area_id, key, value,
         url, reply_msg));
     return;
   }
@@ -310,9 +309,8 @@ void DOMStorageDispatcherHost::OnSetItem(
 void DOMStorageDispatcherHost::OnRemoveItem(
     int64 storage_area_id, const string16& key, const GURL& url) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnRemoveItem, storage_area_id, key,
-        url));
+    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
+        &DOMStorageDispatcherHost::OnRemoveItem, storage_area_id, key, url));
     return;
   }
 
@@ -330,8 +328,8 @@ void DOMStorageDispatcherHost::OnRemoveItem(
 
 void DOMStorageDispatcherHost::OnClear(int64 storage_area_id, const GURL& url) {
   if (ChromeThread::CurrentlyOn(ChromeThread::IO)) {
-    ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, NewRunnableMethod(
-        this, &DOMStorageDispatcherHost::OnClear, storage_area_id, url));
+    PostTaskToWebKitThread(FROM_HERE, NewRunnableMethod(this,
+        &DOMStorageDispatcherHost::OnClear, storage_area_id, url));
     return;
   }
 
@@ -358,3 +356,9 @@ 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 6ca2d62..b99ac94 100644
--- a/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
+++ b/chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h
@@ -76,6 +76,10 @@ 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 c3ad81f..6311140 100644
--- a/chrome/browser/in_process_webkit/webkit_thread.cc
+++ b/chrome/browser/in_process_webkit/webkit_thread.cc
@@ -9,7 +9,9 @@
 #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.
@@ -21,19 +23,11 @@ WebKitThread::~WebKitThread() {
   DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::WEBKIT));
 }
 
-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.
+void WebKitThread::EnsureInitialized() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+  if (webkit_thread_.get())
     return;
-  }
-
-  webkit_thread_.reset(new InternalWebKitThread);
-  bool started = webkit_thread_->Start();
-  DCHECK(started);
+  InitializeThread();
 }
 
 WebKitThread::InternalWebKitThread::InternalWebKitThread()
@@ -49,10 +43,21 @@ 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 UI thread any longer than we have to.
+  // them now) so we don't block the IO 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 9838730..97b1856 100644
--- a/chrome/browser/in_process_webkit/webkit_thread.h
+++ b/chrome/browser/in_process_webkit/webkit_thread.h
@@ -14,14 +14,20 @@
 
 class BrowserWebKitClientImpl;
 
-// This creates a WebKit main thread on instantiation (if not in
-// --single-process mode) on construction and kills it on deletion.
+// 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.
 class WebKitThread {
  public:
   // Called from the UI thread.
   WebKitThread();
   ~WebKitThread();
-  void Initialize();
+
+  // 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();
 
  private:
   // Must be private so that we can carefully control its lifetime.
@@ -39,7 +45,12 @@ class WebKitThread {
     scoped_ptr<BrowserWebKitClientImpl> webkit_client_;
   };
 
-  // Pointer to the actual WebKitThread.
+  // 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.
   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 7203cd9..3b0f99e 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host.cc
+++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc
@@ -286,7 +286,6 @@ 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 4feed71..301a2a1 100644
--- a/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc
+++ b/chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc
@@ -320,7 +320,8 @@ void CheckSuccessfulRequest(const std::vector<IPC::Message>& messages,
 }
 
 // Tests whether many messages get dispatched properly.
-TEST_F(ResourceDispatcherHostTest, TestMany) {
+// TODO(jorlow): Re-enable once bug 28364 is fixed.
+TEST_F(ResourceDispatcherHostTest, DISABLED_TestMany) {
   EXPECT_EQ(0, host_.GetOutstandingRequestsMemoryCost(0));
 
   MakeTestRequest(0, 1, URLRequestTestJob::test_url_1());
@@ -346,7 +347,8 @@ TEST_F(ResourceDispatcherHostTest, TestMany) {
 
 // Tests whether messages get canceled properly. We issue three requests,
 // cancel one of them, and make sure that each sent the proper notifications.
-TEST_F(ResourceDispatcherHostTest, Cancel) {
+// TODO(jorlow): Re-enable once bug 28364 is fixed.
+TEST_F(ResourceDispatcherHostTest, DISABLED_Cancel) {
   EXPECT_EQ(0, host_.GetOutstandingRequestsMemoryCost(0));
 
   MakeTestRequest(0, 1, URLRequestTestJob::test_url_1());
@@ -606,7 +608,8 @@ 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.
-TEST_F(ResourceDispatcherHostTest, TestBlockedRequestsDontLeak) {
+// TODO(jorlow): Re-enable once bug 28364 is fixed.
+TEST_F(ResourceDispatcherHostTest, DISABLED_TestBlockedRequestsDontLeak) {
   // This second receiver is used to emulate a second process.
   ForwardingReceiver second_receiver(this);