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

chrome/browser/ui/unload_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #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);
 }
 
 UnloadController::~UnloadController() {
   browser_->tab_strip_model()->RemoveObserver(this);
 }
 
 bool UnloadController::CanCloseContents(content::WebContents* contents) {
   // Don't try to close the tab when the whole browser is being closed, since
   // that avoids the fast shutdown path where we just kill all the renderers.
   if (is_attempting_to_close_browser_)
     ClearUnloadState(contents, true);
   return !is_attempting_to_close_browser_;
 }
 
 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;
   }
 
   if (!proceed) {
     CancelWindowClose();
     contents->SetClosedByUserGesture(false);
     return false;
   }
 
   if (RemoveFromSet(&tabs_needing_before_unload_fired_, contents)) {
@@ skipping 10 matching lines @@
   return true;
 }
 
 bool UnloadController::ShouldCloseWindow() {
   if (HasCompletedUnloadProcessing())
     return true;
 
   is_attempting_to_close_browser_ = true;
 
   if (!TabsNeedBeforeUnloadFired())
-    return true;
+    return !unload_detached_handler_->HasTabs();
 
   ProcessPendingTabs();
   return false;
 }
 
 bool UnloadController::TabsNeedBeforeUnloadFired() {
   if (tabs_needing_before_unload_fired_.empty()) {
     for (int i = 0; i < browser_->tab_count(); ++i) {
       content::WebContents* contents =
           chrome::GetTabContentsAt(browser_, i)->web_contents();
@@ skipping 86 matching lines @@
   }
 
   // Process beforeunload tabs first. When that queue is empty, process
   // unload tabs.
   if (!tabs_needing_before_unload_fired_.empty()) {
     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();
+      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);
     }
   } else {
     NOTREACHED();
   }
 }
 
 bool UnloadController::HasCompletedUnloadProcessing() const {
   return is_attempting_to_close_browser_ &&
@@ skipping 37 matching lines @@
     } else {
       MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(&UnloadController::ProcessPendingTabs,
                      weak_factory_.GetWeakPtr()));
     }
   }
 }
 
 }  // namespace chrome