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 da61646b28229538ae9910d971e5dda34fa32efd..05845fbad277e26bcd59ad37ec7885b0cc648ac3 100644
--- a/chrome/browser/ui/unload_controller.cc
+++ b/chrome/browser/ui/unload_controller.cc
@@ -4,25 +4,54 @@
 
 #include "chrome/browser/ui/unload_controller.h"
 
+#include "base/logging.h"
 #include "base/message_loop.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_delegate.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 {
 
+
+////////////////////////////////////////////////////////////////////////////////
+// DetachedWebContentsDelegate will delete web contents when they close.
+class UnloadController::DetachedWebContentsDelegate
+    : public content::WebContentsDelegate {
+ public:
+  DetachedWebContentsDelegate() { }
+  virtual ~DetachedWebContentsDelegate() { }
+
+ private:
+  // WebContentsDelegate implementation.
+  virtual bool ShouldSuppressDialogs() OVERRIDE {
+    return true;  // Return true so dialogs are suppressed.
+  }
+
+  virtual void CloseContents(content::WebContents* source) OVERRIDE {
+    // Finished detached close.
+    // UnloadController will observe |NOTIFICATION_WEB_CONTENTS_DISCONNECTED|.
+    delete source;
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(DetachedWebContentsDelegate);
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // UnloadController, public:
 
 UnloadController::UnloadController(Browser* browser)
     : browser_(browser),
+      tab_needing_before_unload_ack_(NULL),
       is_attempting_to_close_browser_(false),
+      detached_delegate_(new DetachedWebContentsDelegate()),
       weak_factory_(this) {
   browser_->tab_strip_model()->AddObserver(this);
 }
@@ -34,16 +63,20 @@ UnloadController::~UnloadController() {
 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) {
       contents->SetClosedByUserGesture(false);
+    } else {
+      // No more dialogs are possible, so remove the tab and finish
+      // running unload listeners asynchrounously.
+      browser_->tab_strip_model()->delegate()->CreateHistoricalTab(contents);
+      DetachWebContents(contents);
+    }
     return proceed;
   }
 
@@ -53,10 +86,10 @@ bool UnloadController::BeforeUnloadFired(content::WebContents* contents,
     return false;
   }
 
-  if (RemoveFromSet(&tabs_needing_before_unload_fired_, contents)) {
-    // Now that beforeunload has fired, put the tab on the queue to fire
-    // unload.
-    tabs_needing_unload_fired_.insert(contents);
+  if (tab_needing_before_unload_ack_ == contents) {
+    // Now that beforeunload has fired, queue the tab to fire unload.
+    tab_needing_before_unload_ack_ = NULL;
+    tabs_needing_unload_.insert(contents);
     ProcessPendingTabs();
     // We want to handle firing the unload event ourselves since we want to
     // fire all the beforeunload events before attempting to fire the unload
@@ -81,15 +114,20 @@ bool UnloadController::ShouldCloseWindow() {
 }
 
 bool UnloadController::TabsNeedBeforeUnloadFired() {
-  if (tabs_needing_before_unload_fired_.empty()) {
-    for (int i = 0; i < browser_->tab_strip_model()->count(); ++i) {
-      content::WebContents* contents =
-          browser_->tab_strip_model()->GetWebContentsAt(i);
-      if (contents->NeedToFireBeforeUnload())
-        tabs_needing_before_unload_fired_.insert(contents);
-    }
+  if (!tabs_needing_before_unload_.empty() ||
+      tab_needing_before_unload_ack_ != NULL)
+    return true;
+
+  if (!tabs_needing_unload_.empty())
+    return false;
+
+  for (int i = 0; i < browser_->tab_strip_model()->count(); ++i) {
+    content::WebContents* contents =
+        browser_->tab_strip_model()->GetWebContentsAt(i);
+    if (contents->NeedToFireBeforeUnload())
+      tabs_needing_before_unload_.insert(contents);
   }
-  return !tabs_needing_before_unload_fired_.empty();
+  return !tabs_needing_before_unload_.empty();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -99,12 +137,15 @@ void UnloadController::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
   switch (type) {
-    case content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED:
-      if (is_attempting_to_close_browser_) {
-        ClearUnloadState(content::Source<content::WebContents>(source).ptr(),
-                         false);  // See comment for ClearUnloadState().
-      }
+    case content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED: {
+      content::WebContents* contents =
+          content::Source<content::WebContents>(source).ptr();
+      registrar_.Remove(this,
+                        content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
+                        source);
+      ClearUnloadState(contents);
       break;
+    }
     default:
       NOTREACHED() << "Got a notification we didn't register for.";
   }
@@ -151,11 +192,32 @@ void UnloadController::TabAttachedImpl(content::WebContents* contents) {
 }
 
 void UnloadController::TabDetachedImpl(content::WebContents* contents) {
-  if (is_attempting_to_close_browser_)
-    ClearUnloadState(contents, false);
+  if (tabs_needing_unload_ack_.find(contents) !=
+      tabs_needing_unload_ack_.end()) {
+    // Tab needs unload to complete.
+    // It will send |NOTIFICATION_WEB_CONTENTS_DISCONNECTED| when done.
+    return;
+  }
+
   registrar_.Remove(this,
                     content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
                     content::Source<content::WebContents>(contents));
+
+  if (is_attempting_to_close_browser_)
+    ClearUnloadState(contents);
+}
+
+bool UnloadController::DetachWebContents(content::WebContents* contents) {
+  int index = browser_->tab_strip_model()->GetIndexOfWebContents(contents);
+  if (index != TabStripModel::kNoTab &&
+      contents->NeedToFireBeforeUnload()) {
+    tabs_needing_unload_ack_.insert(contents);
+    browser_->tab_strip_model()->DetachWebContentsAt(index);
+    contents->SetDelegate(detached_delegate_.get());
+    contents->OnUnloadDetachedStarted();
+    return true;
+  }
+  return false;
 }
 
 void UnloadController::ProcessPendingTabs() {
@@ -166,58 +228,95 @@ void UnloadController::ProcessPendingTabs() {
     return;
   }
 
-  if (HasCompletedUnloadProcessing()) {
-    // We've finished all the unload events and can proceed to close the
-    // browser.
-    browser_->OnWindowClosing();
+  if (tab_needing_before_unload_ack_ != NULL) {
+    // Wait for |BeforeUnloadFired| before proceeding.
     return;
   }
 
-  // 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());
+  // Process a beforeunload handler.
+  if (!tabs_needing_before_unload_.empty()) {
+    WebContentsSet::iterator it = tabs_needing_before_unload_.begin();
+    content::WebContents* contents = *it;
+    tabs_needing_before_unload_.erase(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->GetRenderViewHost()->FirePageBeforeUnload(false);
+    if (contents->GetRenderViewHost()) {
+      tab_needing_before_unload_ack_ = contents;
+      contents->OnCloseStarted();
+      contents->GetRenderViewHost()->FirePageBeforeUnload(false);
     } else {
-      ClearUnloadState(web_contents, true);
+      ProcessPendingTabs();
     }
-  } 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();
+    return;
+  }
+
+  // Process all the unload handlers. (The beforeunload handlers have finished.)
+  if (!tabs_needing_unload_.empty()) {
+    browser_->OnWindowClosing();
+
+    // Run unload handlers detached since no more interaction is possible.
+    WebContentsSet::iterator it = tabs_needing_unload_.begin();
+    while (it != tabs_needing_unload_.end()) {
+      WebContentsSet::iterator current = it++;
+      content::WebContents* contents = *current;
+      tabs_needing_unload_.erase(current);
+      // 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 (contents->GetRenderViewHost()) {
+        contents->OnUnloadStarted();
+        DetachWebContents(contents);
+        contents->GetRenderViewHost()->ClosePage();
+      }
+    }
+
+    // Get the browser hidden.
+    if (browser_->tab_strip_model()->empty()) {
+      browser_->TabStripEmpty();
+    } else {
+      browser_->tab_strip_model()->CloseAllTabs();  // tabs not needing unload
+    }
+    return;
+  }
+
+  if (HasCompletedUnloadProcessing()) {
+    browser_->OnWindowClosing();
+
+    // Get the browser closed.
+    if (browser_->tab_strip_model()->empty()) {
+      browser_->TabStripEmpty();
     } else {
-      ClearUnloadState(web_contents, true);
+      // There may be tabs if the last tab needing beforeunload crashed.
+      browser_->tab_strip_model()->CloseAllTabs();
     }
-  } else {
-    NOTREACHED();
+    return;
   }
 }
 
 bool UnloadController::HasCompletedUnloadProcessing() const {
   return is_attempting_to_close_browser_ &&
-      tabs_needing_before_unload_fired_.empty() &&
-      tabs_needing_unload_fired_.empty();
+      tabs_needing_before_unload_.empty() &&
+      tab_needing_before_unload_ack_ == NULL &&
+      tabs_needing_unload_.empty() &&
+      tabs_needing_unload_ack_.empty();
 }
 
 void UnloadController::CancelWindowClose() {
   // Closing of window can be canceled from a beforeunload handler.
   DCHECK(is_attempting_to_close_browser_);
-  tabs_needing_before_unload_fired_.clear();
-  tabs_needing_unload_fired_.clear();
+  tabs_needing_before_unload_.clear();
+  if (tab_needing_before_unload_ack_ != NULL) {
+    tab_needing_before_unload_ack_->OnCloseCanceled();
+    tab_needing_before_unload_ack_ = NULL;
+  }
+  for (WebContentsSet::iterator it = tabs_needing_unload_.begin();
+       it != tabs_needing_unload_.end(); it++) {
+    content::WebContents* contents = *it;
+    contents->OnCloseCanceled();
+  }
+  tabs_needing_unload_.clear();
+
+  // No need to clear tabs_needing_unload_ack_. Those tabs are already detached.
+
   is_attempting_to_close_browser_ = false;
 
   content::NotificationService::current()->Notify(
@@ -226,33 +325,34 @@ void UnloadController::CancelWindowClose() {
       content::NotificationService::NoDetails());
 }
 
-bool UnloadController::RemoveFromSet(UnloadListenerSet* set,
-                                     content::WebContents* web_contents) {
-  DCHECK(is_attempting_to_close_browser_);
+void UnloadController::ClearUnloadState(content::WebContents* contents) {
+  if (tabs_needing_unload_ack_.erase(contents) > 0) {
+    if (HasCompletedUnloadProcessing())
+      PostTaskForProcessPendingTabs();
+    return;
+  }
 
-  UnloadListenerSet::iterator iter =
-      std::find(set->begin(), set->end(), web_contents);
-  if (iter != set->end()) {
-    set->erase(iter);
-    return true;
+  if (!is_attempting_to_close_browser_)
+    return;
+
+  if (tab_needing_before_unload_ack_ == contents) {
+    tab_needing_before_unload_ack_ = NULL;
+    PostTaskForProcessPendingTabs();
+    return;
   }
-  return false;
-}
 
-void UnloadController::ClearUnloadState(content::WebContents* web_contents,
-                                        bool process_now) {
-  if (is_attempting_to_close_browser_) {
-    RemoveFromSet(&tabs_needing_before_unload_fired_, web_contents);
-    RemoveFromSet(&tabs_needing_unload_fired_, web_contents);
-    if (process_now) {
-      ProcessPendingTabs();
-    } else {
-      base::MessageLoop::current()->PostTask(
-          FROM_HERE,
-          base::Bind(&UnloadController::ProcessPendingTabs,
-                     weak_factory_.GetWeakPtr()));
-    }
+  if (tabs_needing_before_unload_.erase(contents) > 0 ||
+      tabs_needing_unload_.erase(contents) > 0) {
+    if (tab_needing_before_unload_ack_ == NULL)
+      PostTaskForProcessPendingTabs();
   }
 }
 
+void UnloadController::PostTaskForProcessPendingTabs() {
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&UnloadController::ProcessPendingTabs,
+                 weak_factory_.GetWeakPtr()));
+}
+
 }  // namespace chrome