Retry r27137. Create renderers for ExtensionHosts one at a time to avoid blocking the UI.

I added a process.Close() to the fast shutdown path for renderers. The problem
was that we were trying to use an old terminated process handle.

BUG=14040
TEST=Install a bunch of extensions with toolstrips, then restart Chrome. The
UI should be responsive while the toolstrips are loading.

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

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

8 files changed, 103 insertions, 14 deletions

diff --git a/chrome/browser/cocoa/extension_view_mac.mm b/chrome/browser/cocoa/extension_view_mac.mm
index a64eee8..b8e11f7 100644
--- a/chrome/browser/cocoa/extension_view_mac.mm
+++ b/chrome/browser/cocoa/extension_view_mac.mm
@@ -64,5 +64,5 @@ void ExtensionViewMac::CreateWidgetHostView() {
   // disappear.
   [render_widget_host_view_->native_view() retain];
 
-  extension_host_->CreateRenderView(render_widget_host_view_);
+  extension_host_->CreateRenderViewSoon(render_widget_host_view_);
 }
diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index 8ff822f..84852e2 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -4,7 +4,11 @@
 
 #include "chrome/browser/extensions/extension_host.h"
 
+#include <list>
+
 #include "app/resource_bundle.h"
+#include "base/message_loop.h"
+#include "base/singleton.h"
 #include "base/string_util.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_list.h"
@@ -40,6 +44,66 @@ bool ExtensionHost::enable_dom_automation_ = false;
 
 static const char* kToolstripTextColorSubstitution = "$TEXT_COLOR$";
 
+// Helper class that rate-limits the creation of renderer processes for
+// ExtensionHosts, to avoid blocking the UI.
+class ExtensionHost::ProcessCreationQueue {
+ public:
+  static ProcessCreationQueue* get() {
+    return Singleton<ProcessCreationQueue>::get();
+  }
+
+  // Add a host to the queue for RenderView creation.
+  void CreateSoon(ExtensionHost* host) {
+    queue_.push_back(host);
+    PostTask();
+  }
+
+  // Remove a host from the queue (in case it's being deleted).
+  void Remove(ExtensionHost* host) {
+    Queue::iterator it = std::find(queue_.begin(), queue_.end(), host);
+    if (it != queue_.end())
+      queue_.erase(it);
+  }
+
+ private:
+  friend class Singleton<ProcessCreationQueue>;
+  friend struct DefaultSingletonTraits<ProcessCreationQueue>;
+  ProcessCreationQueue()
+      : pending_create_(false),
+        ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) { }
+
+  // Queue up a delayed task to process the next ExtensionHost in the queue.
+  void PostTask() {
+    if (!pending_create_) {
+      MessageLoop::current()->PostTask(FROM_HERE,
+          method_factory_.NewRunnableMethod(
+             &ProcessCreationQueue::ProcessOneHost));
+      pending_create_ = true;
+    }
+  }
+
+  // Create the RenderView for the next host in the queue.
+  void ProcessOneHost() {
+    pending_create_ = false;
+    if (queue_.empty())
+      return;  // can happen on shutdown
+
+    queue_.front()->CreateRenderViewNow();
+    queue_.pop_front();
+
+    if (!queue_.empty())
+      PostTask();
+  }
+
+  typedef std::list<ExtensionHost*> Queue;
+  Queue queue_;
+  bool pending_create_;
+  ScopedRunnableMethodFactory<ProcessCreationQueue> method_factory_;
+};
+
+////////////////
+// ExtensionHost
+
 ExtensionHost::ExtensionHost(Extension* extension, SiteInstance* site_instance,
                              const GURL& url, ViewType::Type host_type)
     : extension_(extension),
@@ -59,6 +123,7 @@ ExtensionHost::~ExtensionHost() {
       NotificationType::EXTENSION_HOST_DESTROYED,
       Source<Profile>(profile_),
       Details<ExtensionHost>(this));
+  ProcessCreationQueue::get()->Remove(this);
   render_view_host_->Shutdown();  // deletes render_view_host
 }
 
@@ -92,9 +157,13 @@ bool ExtensionHost::IsRenderViewLive() const {
   return render_view_host_->IsRenderViewLive();
 }
 
-void ExtensionHost::CreateRenderView(RenderWidgetHostView* host_view) {
+void ExtensionHost::CreateRenderViewSoon(RenderWidgetHostView* host_view) {
   LOG(INFO) << "Creating RenderView for " + extension_->name();
   render_view_host_->set_view(host_view);
+  ProcessCreationQueue::get()->CreateSoon(this);
+}
+
+void ExtensionHost::CreateRenderViewNow() {
   render_view_host_->CreateRenderView();
   NavigateToURL(url_);
   DCHECK(IsRenderViewLive());
diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h
index 3a129a7..7f3ad2cf 100644
--- a/chrome/browser/extensions/extension_host.h
+++ b/chrome/browser/extensions/extension_host.h
@@ -38,6 +38,8 @@ class ExtensionHost : public RenderViewHostDelegate,
                       public ExtensionFunctionDispatcher::Delegate,
                       public NotificationObserver {
  public:
+  class ProcessCreationQueue;
+
   // Enable DOM automation in created render view hosts.
   static void EnableDOMAutomation() { enable_dom_automation_ = true; }
 
@@ -75,10 +77,10 @@ class ExtensionHost : public RenderViewHostDelegate,
   // Returns true if the render view is initialized and didn't crash.
   bool IsRenderViewLive() const;
 
-  // Initializes our RenderViewHost by creating its RenderView and navigating
-  // to this host's url.  Uses host_view for the RenderViewHost's view (can be
-  // NULL).
-  void CreateRenderView(RenderWidgetHostView* host_view);
+  // Prepares to initializes our RenderViewHost by creating its RenderView and
+  // navigating to this host's url. Uses host_view for the RenderViewHost's view
+  // (can be NULL). This happens delayed to avoid locking the UI.
+  void CreateRenderViewSoon(RenderWidgetHostView* host_view);
 
   // Sets |url_| and navigates |render_view_host_|.
   void NavigateToURL(const GURL& url);
@@ -137,10 +139,15 @@ class ExtensionHost : public RenderViewHostDelegate,
                        const NotificationDetails& details);
 
  private:
+  friend class ProcessCreationQueue;
+
   // Whether to allow DOM automation for created RenderViewHosts. This is used
   // for testing.
   static bool enable_dom_automation_;
 
+  // Actually create the RenderView for this host. See CreateRenderViewSoon.
+  void CreateRenderViewNow();
+
   // ExtensionFunctionDispatcher::Delegate
   // If this ExtensionHost has a view, this returns the Browser that view is a
   // part of.  If this is a global background page, we use the active Browser
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index 96e7a25..cec5a45 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -86,7 +86,7 @@ ExtensionHost* ExtensionProcessManager::CreateBackgroundHost(
   ExtensionHost* host =
       new ExtensionHost(extension, GetSiteInstanceForURL(url), url,
                         ViewType::EXTENSION_BACKGROUND_PAGE);
-  host->CreateRenderView(NULL);  // create a RenderViewHost with no view
+  host->CreateRenderViewSoon(NULL);  // create a RenderViewHost with no view
   OnExtensionHostCreated(host, true);
   return host;
 }
diff --git a/chrome/browser/gtk/extension_view_gtk.cc b/chrome/browser/gtk/extension_view_gtk.cc
index 4c06cfb..b6bb3a4 100644
--- a/chrome/browser/gtk/extension_view_gtk.cc
+++ b/chrome/browser/gtk/extension_view_gtk.cc
@@ -41,5 +41,5 @@ void ExtensionViewGtk::CreateWidgetHostView() {
   render_widget_host_view_ = new RenderWidgetHostViewGtk(render_view_host());
   render_widget_host_view_->InitAsChild();
 
-  extension_host_->CreateRenderView(render_widget_host_view_);
+  extension_host_->CreateRenderViewSoon(render_widget_host_view_);
 }
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 4276efa..676f8ce 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -195,7 +195,8 @@ BrowserRenderProcessHost::BrowserRenderProcessHost(Profile* profile)
       ALLOW_THIS_IN_INITIALIZER_LIST(cached_dibs_cleaner_(
             base::TimeDelta::FromSeconds(5),
             this, &BrowserRenderProcessHost::ClearTransportDIBCache)),
-      zygote_child_(false) {
+      zygote_child_(false),
+      fast_shutdown_(false) {
   widget_helper_ = new RenderWidgetHelper();
 
   registrar_.Add(this, NotificationType::USER_SCRIPTS_UPDATED,
@@ -442,6 +443,7 @@ bool BrowserRenderProcessHost::Init() {
       return false;
     }
     process_.set_handle(process);
+    fast_shutdown_ = false;
 
     // Log the launch time, separating out the first one (which will likely be
     // slower due to the rest of the browser initializing at the same time).
@@ -637,6 +639,8 @@ bool BrowserRenderProcessHost::FastShutdownIfPossible() {
   // Otherwise, we're allowed to just terminate the process. Using exit code 0
   // means that UMA won't treat this as a renderer crash.
   process_.Terminate(ResultCodes::NORMAL_EXIT);
+  process_.Close();
+  fast_shutdown_ = true;
   return true;
 }
 
@@ -763,7 +767,11 @@ void BrowserRenderProcessHost::OnMessageReceived(const IPC::Message& msg) {
 void BrowserRenderProcessHost::OnChannelConnected(int32 peer_pid) {
   // process_ is not NULL if we created the renderer process
   if (!process_.handle()) {
-    if (base::GetCurrentProcId() == peer_pid) {
+    if (fast_shutdown_) {
+      // We terminated the process, but the ChannelConnected task was still
+      // in the queue. We can safely ignore it.
+      return;
+    } else if (base::GetCurrentProcId() == peer_pid) {
       // We are in single-process mode. In theory we should have access to
       // ourself but it may happen that we don't.
       process_.set_handle(base::GetCurrentProcessHandle());
@@ -818,13 +826,15 @@ void BrowserRenderProcessHost::BadMessageTerminateProcess(
 void BrowserRenderProcessHost::OnChannelError() {
   // Our child process has died.  If we didn't expect it, it's a crash.
   // In any case, we need to let everyone know it's gone.
-
-  DCHECK(process_.handle());
   DCHECK(channel_.get());
 
   bool child_exited;
   bool did_crash;
-  if (zygote_child_) {
+  if (!process_.handle()) {
+    // The process has been terminated (likely FastShutdownIfPossible).
+    did_crash = false;
+    child_exited = true;
+  } else if (zygote_child_) {
 #if defined(OS_LINUX)
     did_crash = Singleton<ZygoteHost>()->DidProcessCrash(
         process_.handle(), &child_exited);
diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h
index b8a5c54..556ba05 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.h
+++ b/chrome/browser/renderer_host/browser_render_process_host.h
@@ -166,6 +166,9 @@ class BrowserRenderProcessHost : public RenderProcessHost,
   // True iff the renderer is a child of a zygote process.
   bool zygote_child_;
 
+  // True if FastShutdownIfPossible was called and was successful.
+  bool fast_shutdown_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowserRenderProcessHost);
 };
 
diff --git a/chrome/browser/views/extensions/extension_view.cc b/chrome/browser/views/extensions/extension_view.cc
index 4302c93..24ce5f1 100644
--- a/chrome/browser/views/extensions/extension_view.cc
+++ b/chrome/browser/views/extensions/extension_view.cc
@@ -86,7 +86,7 @@ void ExtensionView::CreateWidgetHostView() {
   NOTIMPLEMENTED();
 #endif
 
-  host_->CreateRenderView(view);
+  host_->CreateRenderViewSoon(view);
   SetVisible(false);
 
   if (!pending_background_.empty()) {