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

chrome/browser/ui/unload_controller.h

Index: chrome/browser/ui/unload_controller.h
diff --git a/chrome/browser/ui/unload_controller.h b/chrome/browser/ui/unload_controller.h
index fb51668b0554bd67d563d351b8c5db5ee7081f75..c2784cc74060165bde5a09d286013becd9453a2f 100644
--- a/chrome/browser/ui/unload_controller.h
+++ b/chrome/browser/ui/unload_controller.h
@@ -23,7 +23,34 @@ class WebContents;
 }
 
 namespace chrome {
-
+class UnloadDetachedHandler;
+
+// UnloadController manages closing tabs and windows -- especially in
+// regards to beforeunload handlers (proceed/cancel dialogs) and
+// unload handlers (no user interaction).
+//
+// Typical flow of closing a tab:
+//  1. Browser calls |CanCloseContents()|.
+//     If true, browser calls |contents::CloseWebContents()|.

[Ben Goodger (Google), 2012/10/29 22:12:43]

chrome::CloseWebContents()

[slamm, 2012/10/31 18:03:59]

Done.

+//  2. Browser calls |BeforeUnloadFired()| for tabs with beforeunload or unload

[Ben Goodger (Google), 2012/10/29 22:12:43]

Technically, the WebContents calls BeforeUnloadFired via its delegate interface.
I'd write this as:

   // 2. WebContents notifies us via its delegate and BeforeUnloadFired()
   //    that the beforeunload handler was run. If the user allowed the
   //    close to continue, we hand off running the unload handler to
   //    UnloadDetachedHandler which decides if the tab can be unloaded
   //    in the background. If it can, the tab is removed from the tab
   //    strip at this point.

[slamm, 2012/10/31 18:03:59]

Done. Your description is much better.

+//     handlers.  If |proceed| is true, |UnloadController| hands-off the tab to
+//     |UnloadDetachedHandler| to run the unload handler in the background.
+//     |UnloadDetachedHandler| decides if the tab can unload in the background,
+//     and removes it from the tab strip if it can.
+//
+// Typical flow of closing a window:
+//  1. Browser calls |ShouldCloseWindow()|.
+//     If beforeunload/unload handlers need to run, |UnloadController| returns
+//     false and calls |ProcessPendingTabs()| (private method).
+//  2. For each tab with a beforeunload/unload handler, |ProcessPendingTabs()|
+//        calls |web_contents->OnCloseStarted()|
+//        and   |web_contents->GetRenderViewHost()->FirePageBeforeUnload()|.
+//  3. |UnloadController| waits for any tabs from the "closing a tab" flow above
+//     to finish detached unloads.
+//  4. For each tab with a beforeunload/unload handler, |ProcessPendingTabs()|
+//        calls |web_contents->GetRenderViewHost()->ClosePage()|.
+//  5. |ProcessPendingTabs()| calls |browser_->OnWindowClosing()|.
+//
 class UnloadController : public content::NotificationObserver,
                          public TabStripModelObserver {
  public:
@@ -114,11 +141,11 @@ class UnloadController : public content::NotificationObserver,
 
   content::NotificationRegistrar registrar_;
 
-  // Tracks tabs that need there beforeunload event fired before we can
+  // Tracks tabs that need their beforeunload event fired before we can
   // close the browser. Only gets populated when we try to close the browser.
   UnloadListenerSet tabs_needing_before_unload_fired_;
 
-  // Tracks tabs that need there unload event fired before we can
+  // Tracks tabs that need their unload event fired before we can
   // close the browser. Only gets populated when we try to close the browser.
   UnloadListenerSet tabs_needing_unload_fired_;
 
@@ -128,6 +155,9 @@ class UnloadController : public content::NotificationObserver,
   // Browser window isn't just immediately closed.
   bool is_attempting_to_close_browser_;
 
+  // Allow unload handlers to run without holding up the UI.
+  UnloadDetachedHandler* const unload_detached_handler_;

[Ben Goodger (Google), 2012/10/29 22:12:43]

We don't tend to use this type of const in chrome

[slamm, 2012/10/31 18:03:59]

Changed to scoped_ptr.

+
   base::WeakPtrFactory<UnloadController> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(UnloadController);