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 bc8c1805360beb6682c6f656a5baf12f41a4aa42..4c4a37aad4278f9e750ffb188b92b44edc9bc587 100644
--- a/chrome/browser/ui/unload_controller.cc
+++ b/chrome/browser/ui/unload_controller.cc
@@ -4,6 +4,10 @@
 
 #include "chrome/browser/ui/unload_controller.h"
 
+#include <map>
+
+#include "base/callback.h"
+#include "base/memory/scoped_vector.h"
 #include "base/message_loop.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
@@ -15,15 +19,84 @@
 #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;
+typedef std::map<content::WebContents*, TabContents*> DetachedTabsMap;

[Ben Goodger (Google), 2012/10/31 18:28:32]

this you can put at the start of the private area of the class, right? I don't
think we need to define at file scope.

+
+////////////////////////////////////////////////////////////////////////////////
+// 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.
+//
+// TODO(slamm): Support closes that destroy the browser: last tab close, window.
+//     http://crbug.com/156896
+// TODO(slamm): Hide unload time from the user for cross-process navigations.
+//     http://crbug.com/156958
+class UnloadDetachedHandler : public content::WebContentsDelegate {
+ public:
+  explicit UnloadDetachedHandler(const DetachedTabsClosedCallback& callback)
+      : tabs_closed_callback_(callback) { }
+  ~UnloadDetachedHandler() { }

[Ben Goodger (Google), 2012/10/31 18:28:32]

virtual dtor

+
+  // Returns true if it succeeds.
+  bool DetachWebContents(TabStripModel* tab_strip_model,
+                         content::WebContents* web_contents) {
+    if (tab_strip_model->count() > 1) {
+      // Only allow tab to be detached if it is not the last one.
+      // Otherwise, the tab_strip_model would notify the browser that
+      // the strip is empty, and cause it to close without waiting for
+      // the unload handlers.
+      int index = tab_strip_model->GetIndexOfWebContents(web_contents);
+      if (index != TabStripModel::kNoTab &&
+        web_contents->NeedToFireBeforeUnload()) {
+        TabContents* tab_contents = tab_strip_model->DetachTabContentsAt(index);
+        detached_tabs_[web_contents] = tab_contents;
+        web_contents->SetDelegate(this);
+        web_contents->OnUnloadDetachedStarted();
+        return true;
+      }
+    }
+    return false;
+  }
+
+  bool HasTabs() const {
+    return !detached_tabs_.empty();
+  }
+
+ private:
+  // WebContentsDelegate implementation.
+  virtual bool ShouldSuppressDialogs() OVERRIDE {
+    return true;  // Return true so dialogs are suppressed.
+  }
+  virtual void CloseContents(content::WebContents* source) OVERRIDE {
+    DetachedTabsMap::iterator it = detached_tabs_.find(source);
+    TabContents* tab_contents = (*it).second;

[Ben Goodger (Google), 2012/10/31 18:28:32]

So, Avi is in the process of removing TC from Chrome (replacing just with
WebContents). He's in the process of sorting out TabStripModel right now, and we
can make his life a little easier changing the data structure here.

Instead of a map, you can just keep a set of WebContentses.

Where you need the TC to delete, you can just call

delete TabContents::FromWebContents(web_contents);

(or:

TabContents* tc = TabContents::FromWebContents(web_contents);
DCHECK(tc);
delete tc;

... if you're feeling extra pedantic.)

This means Avi won't have to do this once he comes through these parts.

+    delete tab_contents;
+    detached_tabs_.erase(it);
+    tabs_closed_callback_.Run();
+  }
+
+  const DetachedTabsClosedCallback tabs_closed_callback_;
+  DetachedTabsMap detached_tabs_;
+
+  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 +116,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;
   }
 
@@ -75,7 +154,7 @@ bool UnloadController::ShouldCloseWindow() {
   is_attempting_to_close_browser_ = true;
 
   if (!TabsNeedBeforeUnloadFired())
-    return true;
+    return !unload_detached_handler_->HasTabs();
 
   ProcessPendingTabs();
   return false;
@@ -182,6 +261,7 @@ void UnloadController::ProcessPendingTabs() {
     // 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);
@@ -199,7 +279,9 @@ void UnloadController::ProcessPendingTabs() {
     // 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();
+      if (!unload_detached_handler_->HasTabs()) {

[Ben Goodger (Google), 2012/10/31 18:28:32]

nit, no braces.

+        web_contents->GetRenderViewHost()->ClosePage();
+      }
     } else {
       ClearUnloadState(web_contents, true);
     }