Revert r203922, "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 fcafe591de73136ba5c66330a7f4e4ce21137acc..da61646b28229538ae9910d971e5dda34fa32efd 100644
--- a/chrome/browser/ui/unload_controller.cc
+++ b/chrome/browser/ui/unload_controller.cc
@@ -4,54 +4,25 @@
 
 #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);
 }
@@ -63,20 +34,16 @@ 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;
   }
 
@@ -86,10 +53,10 @@ bool UnloadController::BeforeUnloadFired(content::WebContents* contents,
     return false;
   }
 
-  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);
+  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);
     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
@@ -114,20 +81,15 @@ bool UnloadController::ShouldCloseWindow() {
 }
 
 bool UnloadController::TabsNeedBeforeUnloadFired() {
-  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);
+  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);
+    }
   }
-  return !tabs_needing_before_unload_.empty();
+  return !tabs_needing_before_unload_fired_.empty();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -137,12 +99,12 @@ void UnloadController::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
   switch (type) {
-    case content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED: {
-      content::WebContents* contents =
-          content::Source<content::WebContents>(source).ptr();
-      ClearUnloadState(contents);
+    case content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED:
+      if (is_attempting_to_close_browser_) {
+        ClearUnloadState(content::Source<content::WebContents>(source).ptr(),
+                         false);  // See comment for ClearUnloadState().
+      }
       break;
-    }
     default:
       NOTREACHED() << "Got a notification we didn't register for.";
   }
@@ -189,32 +151,11 @@ void UnloadController::TabAttachedImpl(content::WebContents* contents) {
 }
 
 void UnloadController::TabDetachedImpl(content::WebContents* contents) {
-  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;
-  }
-
+  if (is_attempting_to_close_browser_)
+    ClearUnloadState(contents, false);
   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() {
@@ -225,95 +166,58 @@ void UnloadController::ProcessPendingTabs() {
     return;
   }
 
-  if (tab_needing_before_unload_ack_ != NULL) {
-    // Wait for |BeforeUnloadFired| before proceeding.
+  if (HasCompletedUnloadProcessing()) {
+    // We've finished all the unload events and can proceed to close the
+    // browser.
+    browser_->OnWindowClosing();
     return;
   }
 
-  // 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);
+  // 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 (contents->GetRenderViewHost()) {
-      tab_needing_before_unload_ack_ = contents;
-      contents->OnCloseStarted();
-      contents->GetRenderViewHost()->FirePageBeforeUnload(false);
-    } else {
-      ProcessPendingTabs();
-    }
-    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();
+    if (web_contents->GetRenderViewHost()) {
+      web_contents->GetRenderViewHost()->FirePageBeforeUnload(false);
     } else {
-      browser_->tab_strip_model()->CloseAllTabs();  // tabs not needing unload
+      ClearUnloadState(web_contents, true);
     }
-    return;
-  }
-
-  if (HasCompletedUnloadProcessing()) {
-    browser_->OnWindowClosing();
-
-    // Get the browser closed.
-    if (browser_->tab_strip_model()->empty()) {
-      browser_->TabStripEmpty();
+  } 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 {
-      // There may be tabs if the last tab needing beforeunload crashed.
-      browser_->tab_strip_model()->CloseAllTabs();
+      ClearUnloadState(web_contents, true);
     }
-    return;
+  } else {
+    NOTREACHED();
   }
 }
 
 bool UnloadController::HasCompletedUnloadProcessing() const {
   return is_attempting_to_close_browser_ &&
-      tabs_needing_before_unload_.empty() &&
-      tab_needing_before_unload_ack_ == NULL &&
-      tabs_needing_unload_.empty() &&
-      tabs_needing_unload_ack_.empty();
+      tabs_needing_before_unload_fired_.empty() &&
+      tabs_needing_unload_fired_.empty();
 }
 
 void UnloadController::CancelWindowClose() {
   // Closing of window can be canceled from a beforeunload handler.
   DCHECK(is_attempting_to_close_browser_);
-  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.
-
+  tabs_needing_before_unload_fired_.clear();
+  tabs_needing_unload_fired_.clear();
   is_attempting_to_close_browser_ = false;
 
   content::NotificationService::current()->Notify(
@@ -322,34 +226,33 @@ void UnloadController::CancelWindowClose() {
       content::NotificationService::NoDetails());
 }
 
-void UnloadController::ClearUnloadState(content::WebContents* contents) {
-  if (tabs_needing_unload_ack_.erase(contents) > 0) {
-    if (HasCompletedUnloadProcessing())
-      PostTaskForProcessPendingTabs();
-    return;
-  }
-
-  if (!is_attempting_to_close_browser_)
-    return;
-
-  if (tab_needing_before_unload_ack_ == contents) {
-    tab_needing_before_unload_ack_ = NULL;
-    PostTaskForProcessPendingTabs();
-    return;
-  }
+bool UnloadController::RemoveFromSet(UnloadListenerSet* set,
+                                     content::WebContents* web_contents) {
+  DCHECK(is_attempting_to_close_browser_);
 
-  if (tabs_needing_before_unload_.erase(contents) > 0 ||
-      tabs_needing_unload_.erase(contents) > 0) {
-    if (tab_needing_before_unload_ack_ == NULL)
-      PostTaskForProcessPendingTabs();
+  UnloadListenerSet::iterator iter =
+      std::find(set->begin(), set->end(), web_contents);
+  if (iter != set->end()) {
+    set->erase(iter);
+    return true;
   }
+  return false;
 }
 
-void UnloadController::PostTaskForProcessPendingTabs() {
-  base::MessageLoop::current()->PostTask(
-      FROM_HERE,
-      base::Bind(&UnloadController::ProcessPendingTabs,
-                 weak_factory_.GetWeakPtr()));
+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()));
+    }
+  }
 }
 
 }  // namespace chrome