Quickly close tabs/window with long-running unload handlers.

chrome/browser/ui/unload_controller.cc

Index: chrome/browser/ui/unload_controller.cc
diff --git a/chrome/browser/ui/unload_controller.cc b/chrome/browser/ui/unload_controller.cc
index 3768f734c747178e8442ccc0834141cbd809edae..9e827de4958d814ffa8f6d98fb7121e4bd795bbd 100644
--- a/chrome/browser/ui/unload_controller.cc
+++ b/chrome/browser/ui/unload_controller.cc
@@ -4,6 +4,9 @@
 
 #include "chrome/browser/ui/unload_controller.h"
 
+#include <set>
+
+#include "base/callback.h"
 #include "base/message_loop.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
@@ -15,15 +18,73 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_delegate.h"
 
 namespace chrome {
 
+typedef base::Callback<void(void)> DetachedTabsClosedCallback;
+
+////////////////////////////////////////////////////////////////////////////////
+// UnloadDetachedHandler is used to close tabs quickly, http://crbug.com/142458.
+//   - Allows unload handlers to run in the background.
+//   - Comes into play after the beforeunload handlers (if any) have run.
+//   - Does not close the tabs; it holds tabs while they are closed.
+class UnloadDetachedHandler : public content::WebContentsDelegate {
+ public:
+  explicit UnloadDetachedHandler(const DetachedTabsClosedCallback& callback)
+      : tabs_closed_callback_(callback) { }
+  virtual ~UnloadDetachedHandler() { }
+
+  // Returns true if it succeeds.
+  bool DetachWebContents(TabStripModel* tab_strip_model,
+                         content::WebContents* web_contents) {
+    int index = tab_strip_model->GetIndexOfWebContents(web_contents);
+    if (index != TabStripModel::kNoTab &&
+        web_contents->NeedToFireBeforeUnload()) {
+      tab_strip_model->DetachTabContentsAndCreateHistoryAt(index);
+      detached_web_contents_.insert(web_contents);
+      web_contents->SetDelegate(this);
+      web_contents->OnUnloadDetachedStarted();
+      return true;
+    }
+    return false;
+  }
+
+  bool HasTabs() const {
+    return !detached_web_contents_.empty();
+  }
+
+ private:
+  // WebContentsDelegate implementation.
+  virtual bool ShouldSuppressDialogs() OVERRIDE {
+    return true;  // Return true so dialogs are suppressed.
+  }
+  virtual void CloseContents(content::WebContents* source) OVERRIDE {
+    detached_web_contents_.erase(source);
+    TabContents* tab_contents = TabContents::FromWebContents(source);
+    DCHECK(tab_contents);
+    delete tab_contents;
+    if (detached_web_contents_.empty())
+      tabs_closed_callback_.Run();
+  }
+
+  const DetachedTabsClosedCallback tabs_closed_callback_;
+  std::set<content::WebContents*> detached_web_contents_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(UnloadDetachedHandler);
+};
+
+
 ////////////////////////////////////////////////////////////////////////////////
 // UnloadController, public:
 
 UnloadController::UnloadController(Browser* browser)
     : browser_(browser),
       is_attempting_to_close_browser_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(
+          unload_detached_handler_(new UnloadDetachedHandler(
+              base::Bind(&UnloadController::ProcessPendingTabs,
+                         base::Unretained(this))))),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   browser_->tab_strip_model()->AddObserver(this);
 }
@@ -43,8 +104,14 @@ bool UnloadController::CanCloseContents(content::WebContents* contents) {
 bool UnloadController::BeforeUnloadFired(content::WebContents* contents,
                                          bool proceed) {
   if (!is_attempting_to_close_browser_) {
-    if (!proceed)
+    if (proceed) {
+      // No more dialogs are possible, so remove the tab and finish
+      // running unload listeners asynchrounously.
+      unload_detached_handler_->DetachWebContents(browser_->tab_strip_model(),
+                                                  contents);
+    } else {
       contents->SetClosedByUserGesture(false);
+    }
     return proceed;
   }
 
@@ -55,8 +122,7 @@ bool UnloadController::BeforeUnloadFired(content::WebContents* contents,
   }
 
   if (RemoveFromSet(&tabs_needing_before_unload_fired_, contents)) {
-    // Now that beforeunload has fired, put the tab on the queue to fire
-    // unload.
+    // Now that beforeunload has fired, queue the tab to fire unload.
     tabs_needing_unload_fired_.insert(contents);
     ProcessPendingTabs();
     // We want to handle firing the unload event ourselves since we want to
@@ -69,16 +135,15 @@ bool UnloadController::BeforeUnloadFired(content::WebContents* contents,
 }
 
 bool UnloadController::ShouldCloseWindow() {
-  if (HasCompletedUnloadProcessing())
-    return true;
-
-  is_attempting_to_close_browser_ = true;
-
-  if (!TabsNeedBeforeUnloadFired())
-    return true;
-
-  ProcessPendingTabs();
-  return false;
+  if (!is_attempting_to_close_browser_) {
+    is_attempting_to_close_browser_ = true;
+    if (TabsNeedBeforeUnloadFired()) {
+      ProcessPendingTabs();
+    } else {
+      browser_->OnWindowClosing();
+    }
+  }
+  return HasCompletedUnloadProcessing();
 }
 
 bool UnloadController::TabsNeedBeforeUnloadFired() {
@@ -160,63 +225,69 @@ void UnloadController::TabDetachedImpl(content::WebContents* contents) {
 }
 
 void UnloadController::ProcessPendingTabs() {
-  if (!is_attempting_to_close_browser_) {
-    // Because we might invoke this after a delay it's possible for the value of
-    // is_attempting_to_close_browser_ to have changed since we scheduled the
-    // task.
-    return;
-  }
-
   if (HasCompletedUnloadProcessing()) {
-    // We've finished all the unload events and can proceed to close the
-    // browser.
-    browser_->OnWindowClosing();
-    return;
-  }
-
-  // Process beforeunload tabs first. When that queue is empty, process
-  // unload tabs.
-  if (!tabs_needing_before_unload_fired_.empty()) {
+    if (browser_->tab_strip_model()->empty()) {
+      browser_->OnUnloadProcessingCompleted();
+    } else {
+      // Having tabs means OnWindowClosing was never called. That happens if
+      // the last tab needing beforeunload crashes (i.e. we observe that the
+      // web_contents is disconnected).
+      browser_->OnWindowClosing();
+    }
+  } else if (!tabs_needing_before_unload_fired_.empty()) {
+    // Process all the beforeunload handlers before the unload handlers.
     content::WebContents* web_contents =
         *(tabs_needing_before_unload_fired_.begin());
     // Null check render_view_host here as this gets called on a PostTask and
     // the tab's render_view_host may have been nulled out.
     if (web_contents->GetRenderViewHost()) {
+      web_contents->OnCloseStarted();
       web_contents->GetRenderViewHost()->FirePageBeforeUnload(false);
     } else {
       ClearUnloadState(web_contents, true);
     }
   } else if (!tabs_needing_unload_fired_.empty()) {
-    // We've finished firing all beforeunload events and can proceed with unload
-    // events.
-    // TODO(ojan): We should add a call to browser_shutdown::OnShutdownStarting
-    // somewhere around here so that we have accurate measurements of shutdown
-    // time.
-    // TODO(ojan): We can probably fire all the unload events in parallel and
-    // get a perf benefit from that in the cases where the tab hangs in it's
-    // unload handler or takes a long time to page in.
-    content::WebContents* web_contents = *(tabs_needing_unload_fired_.begin());
-    // Null check render_view_host here as this gets called on a PostTask and
-    // the tab's render_view_host may have been nulled out.
-    if (web_contents->GetRenderViewHost()) {
-      web_contents->GetRenderViewHost()->ClosePage();
-    } else {
-      ClearUnloadState(web_contents, true);
+    // All beforeunload handlers have fired. Proceed with unload handlers.
+
+    // Copy unload tabs to avoid iterator issues when detaching tabs.
+    UnloadListenerSet unload_tabs = tabs_needing_unload_fired_;
+
+    // Run unload handlers detached since no more interaction is possible.
+    for (UnloadListenerSet::iterator it = unload_tabs.begin();
+         it != unload_tabs.end(); it++) {
+      content::WebContents* web_contents = *it;
+      // Null check render_view_host here as this gets called on a PostTask
+      // and the tab's render_view_host may have been nulled out.
+      if (web_contents->GetRenderViewHost()) {
+        web_contents->OnUnloadStarted();
+        unload_detached_handler_->DetachWebContents(
+            browser_->tab_strip_model(), web_contents);
+        web_contents->GetRenderViewHost()->ClosePage();
+      }
     }
-  } else {
-    NOTREACHED();
+    tabs_needing_unload_fired_.clear();
+
+    // Close the remaining tabs to make the browser hide the window
+    // while the unload handlers finish.
+    browser_->OnWindowClosing();
   }
 }
 
 bool UnloadController::HasCompletedUnloadProcessing() const {
   return is_attempting_to_close_browser_ &&
       tabs_needing_before_unload_fired_.empty() &&
-      tabs_needing_unload_fired_.empty();
+      tabs_needing_unload_fired_.empty() &&
+      !unload_detached_handler_->HasTabs();
 }
 
 void UnloadController::CancelWindowClose() {
   // Closing of window can be canceled from a beforeunload handler.
   DCHECK(is_attempting_to_close_browser_);
+  for (UnloadListenerSet::iterator it = tabs_needing_unload_fired_.begin();
+       it != tabs_needing_unload_fired_.end(); it++) {
+    content::WebContents* web_contents = *it;
+    web_contents->OnCloseCanceled();
+  }
   tabs_needing_before_unload_fired_.clear();
   tabs_needing_unload_fired_.clear();
   is_attempting_to_close_browser_ = false;