Revert Fast tab closure and dependent CL

Revert r205149 (Fast tab closure) and dependent CL r207181 (Move histograms and supporting code that don't belong in content out) since it breaks a bunch of systems.

Mechanical revert of problematic CLs, setting NOTRY so we can land this in the face of flaky bots.

BUG=142458,156896,249289,246999,246634,248998,250863
TBR=sky@chromium.org,joi@chromium.org,jam@chromium.org
NOTRY=True

Review URL: https://chromiumcodereview.appspot.com/17487002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@207712 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 128 insertions, 418 deletions

diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index d6d87e3..112525a 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -584,14 +584,6 @@ bool Browser::ShouldCloseWindow() {
   return unload_controller_->ShouldCloseWindow();
 }
 
-bool Browser::TabsNeedBeforeUnloadFired() {
-  return unload_controller_->TabsNeedBeforeUnloadFired();
-}
-
-bool Browser::HasCompletedUnloadProcessing() const {
-  return unload_controller_->HasCompletedUnloadProcessing();
-}
-
 bool Browser::IsAttemptingToCloseBrowser() const {
   return unload_controller_->is_attempting_to_close_browser();
 }
@@ -635,6 +627,8 @@ void Browser::OnWindowClosing() {
       chrome::NOTIFICATION_BROWSER_CLOSING,
       content::Source<Browser>(this),
       content::NotificationService::NoDetails());
+
+  tab_strip_model_->CloseAllTabs();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -1171,6 +1165,10 @@ void Browser::HandleKeyboardEvent(content::WebContents* source,
   window()->HandleKeyboardEvent(event);
 }
 
+bool Browser::TabsNeedBeforeUnloadFired() {
+  return unload_controller_->TabsNeedBeforeUnloadFired();
+}
+
 bool Browser::IsMouseLocked() const {
   return fullscreen_controller_->IsMouseLocked();
 }
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
index 60ac1ea..2766b78 100644
--- a/chrome/browser/ui/browser.h
+++ b/chrome/browser/ui/browser.h
@@ -293,13 +293,6 @@ class Browser : public TabStripModelObserver,
   // Gives beforeunload handlers the chance to cancel the close.
   bool ShouldCloseWindow();
 
-  // Figure out if there are tabs that have beforeunload handlers.
-  // It starts beforeunload/unload processing as a side-effect.
-  bool TabsNeedBeforeUnloadFired();
-
-  // Returns true if all tabs' beforeunload/unload events have fired.
-  bool HasCompletedUnloadProcessing() const;
-
   bool IsAttemptingToCloseBrowser() const;
 
   // Invoked when the window containing us is closing. Performs the necessary
@@ -437,6 +430,9 @@ class Browser : public TabStripModelObserver,
       content::WebContents* source,
       const content::NativeWebKeyboardEvent& event) OVERRIDE;
 
+  // Figure out if there are tabs that have beforeunload handlers.
+  bool TabsNeedBeforeUnloadFired();
+
   bool is_type_tabbed() const { return type_ == TYPE_TABBED; }
   bool is_type_popup() const { return type_ == TYPE_POPUP; }
 
diff --git a/chrome/browser/ui/browser_tabstrip.cc b/chrome/browser/ui/browser_tabstrip.cc
index 9ffa615..aa308e0 100644
--- a/chrome/browser/ui/browser_tabstrip.cc
+++ b/chrome/browser/ui/browser_tabstrip.cc
@@ -9,7 +9,6 @@
 #include "chrome/browser/ui/blocked_content/blocked_content_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_navigator.h"
-#include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
@@ -29,9 +28,7 @@ void AddBlankTabAt(Browser* browser, int index, bool foreground) {
   params.disposition = foreground ? NEW_FOREGROUND_TAB : NEW_BACKGROUND_TAB;
   params.tabstrip_index = index;
   chrome::Navigate(&params);
-  CoreTabHelper* core_tab_helper =
-      CoreTabHelper::FromWebContents(params.target_contents);
-  core_tab_helper->set_new_tab_start_time(new_tab_start_time);
+  params.target_contents->SetNewTabStartTime(new_tab_start_time);
 }
 
 content::WebContents* AddSelectedTabWithURL(
diff --git a/chrome/browser/ui/cocoa/applescript/window_applescript.mm b/chrome/browser/ui/cocoa/applescript/window_applescript.mm
index fe6677f..fe9e055 100644
--- a/chrome/browser/ui/cocoa/applescript/window_applescript.mm
+++ b/chrome/browser/ui/cocoa/applescript/window_applescript.mm
@@ -21,7 +21,6 @@
 #include "chrome/browser/ui/cocoa/applescript/error_applescript.h"
 #import "chrome/browser/ui/cocoa/applescript/tab_applescript.h"
 #include "chrome/browser/ui/host_desktop.h"
-#include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/web_contents.h"
@@ -180,8 +179,7 @@
       browser_,
       GURL(chrome::kChromeUINewTabURL),
       content::PAGE_TRANSITION_TYPED);
-  CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(contents);
-  core_tab_helper->set_new_tab_start_time(newTabStartTime);
+  contents->SetNewTabStartTime(newTabStartTime);
   [aTab setWebContents:contents];
 }
 
@@ -198,9 +196,7 @@
   params.disposition = NEW_FOREGROUND_TAB;
   params.tabstrip_index = index;
   chrome::Navigate(&params);
-  CoreTabHelper* core_tab_helper =
-      CoreTabHelper::FromWebContents(params.target_contents);
-  core_tab_helper->set_new_tab_start_time(newTabStartTime);
+  params.target_contents->SetNewTabStartTime(newTabStartTime);
 
   [aTab setWebContents:params.target_contents];
 }
diff --git a/chrome/browser/ui/cocoa/browser_window_controller.mm b/chrome/browser/ui/cocoa/browser_window_controller.mm
index d200899..d4cc3a3 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller.mm
+++ b/chrome/browser/ui/cocoa/browser_window_controller.mm
@@ -599,18 +599,11 @@ enum {
   [self saveWindowPositionIfNeeded];
 
   if (!browser_->tab_strip_model()->empty()) {
-    // Tab strip isn't empty.  Hide the window (so it appears to have closed
+    // Tab strip isn't empty.  Hide the frame (so it appears to have closed
     // immediately) and close all the tabs, allowing the renderers to shut
     // down. When the tab strip is empty we'll be called back again.
     [[self window] orderOut:self];
     browser_->OnWindowClosing();
-    browser_->tab_strip_model()->CloseAllTabs();
-    return NO;
-  } else if (!browser_->HasCompletedUnloadProcessing()) {
-    // The browser needs to finish running unload handlers.
-    // Hide the window (so it appears to have closed immediately), and
-    // the browser will call us back again when it is ready to close.
-    [[self window] orderOut:self];
     return NO;
   }
 
diff --git a/chrome/browser/ui/gtk/browser_window_gtk.cc b/chrome/browser/ui/gtk/browser_window_gtk.cc
index f343c44..16dfc66 100644
--- a/chrome/browser/ui/gtk/browser_window_gtk.cc
+++ b/chrome/browser/ui/gtk/browser_window_gtk.cc
@@ -1449,13 +1449,6 @@ bool BrowserWindowGtk::CanClose() const {
     // down. When the tab strip is empty we'll be called back again.
     gtk_widget_hide(GTK_WIDGET(window_));
     browser_->OnWindowClosing();
-    browser_->tab_strip_model()->CloseAllTabs();
-    return false;
-  } else if (!browser_->HasCompletedUnloadProcessing()) {
-    // The browser needs to finish running unload handlers.
-    // Hide the window (so it appears to have closed immediately), and
-    // the browser will call us back again when it is ready to close.
-    gtk_widget_hide(GTK_WIDGET(window_));
     return false;
   }
 
diff --git a/chrome/browser/ui/tab_contents/core_tab_helper.cc b/chrome/browser/ui/tab_contents/core_tab_helper.cc
index f8dc42e..502a3f8 100644
--- a/chrome/browser/ui/tab_contents/core_tab_helper.cc
+++ b/chrome/browser/ui/tab_contents/core_tab_helper.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 
-#include "base/metrics/histogram.h"
 #include "chrome/browser/renderer_host/web_cache_manager.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ -85,26 +84,6 @@ string16 CoreTabHelper::GetStatusText() const {
   return string16();
 }
 
-void CoreTabHelper::OnCloseStarted() {
-  if (close_start_time_.is_null())
-    close_start_time_ = base::TimeTicks::Now();
-}
-
-void CoreTabHelper::OnCloseCanceled() {
-  close_start_time_ = base::TimeTicks();
-  before_unload_end_time_ = base::TimeTicks();
-  unload_detached_start_time_ = base::TimeTicks();
-}
-
-void CoreTabHelper::OnUnloadStarted() {
-  before_unload_end_time_ = base::TimeTicks::Now();
-}
-
-void CoreTabHelper::OnUnloadDetachedStarted() {
-  if (unload_detached_start_time_.is_null())
-    unload_detached_start_time_ = base::TimeTicks::Now();
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // WebContentsObserver overrides
 
@@ -112,30 +91,3 @@ void CoreTabHelper::WasShown() {
   WebCacheManager::GetInstance()->ObserveActivity(
       web_contents()->GetRenderProcessHost()->GetID());
 }
-
-void CoreTabHelper::WebContentsDestroyed(WebContents* web_contents) {
-  // OnCloseStarted isn't called in unit tests.
-  if (!close_start_time_.is_null()) {
-    base::TimeTicks now = base::TimeTicks::Now();
-    base::TimeDelta close_time = now - close_start_time_;
-    UMA_HISTOGRAM_TIMES("Tab.Close", close_time);
-
-    base::TimeTicks unload_start_time = close_start_time_;
-    base::TimeTicks unload_end_time = now;
-    if (!before_unload_end_time_.is_null())
-      unload_start_time = before_unload_end_time_;
-    if (!unload_detached_start_time_.is_null())
-      unload_end_time = unload_detached_start_time_;
-    base::TimeDelta unload_time = unload_end_time - unload_start_time;
-    UMA_HISTOGRAM_TIMES("Tab.Close.UnloadTime", unload_time);
-
-  }
-}
-
-void CoreTabHelper::BeforeUnloadFired(const base::TimeTicks& proceed_time) {
-  before_unload_end_time_ = proceed_time;
-}
-
-void CoreTabHelper::BeforeUnloadDialogCancelled() {
-  OnCloseCanceled();
-}
diff --git a/chrome/browser/ui/tab_contents/core_tab_helper.h b/chrome/browser/ui/tab_contents/core_tab_helper.h
index e5a37dd..834613e 100644
--- a/chrome/browser/ui/tab_contents/core_tab_helper.h
+++ b/chrome/browser/ui/tab_contents/core_tab_helper.h
@@ -5,7 +5,6 @@
 #ifndef CHROME_BROWSER_UI_TAB_CONTENTS_CORE_TAB_HELPER_H_
 #define CHROME_BROWSER_UI_TAB_CONTENTS_CORE_TAB_HELPER_H_
 
-#include "base/time.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
@@ -17,64 +16,25 @@ class CoreTabHelper : public content::WebContentsObserver,
  public:
   virtual ~CoreTabHelper();
 
+  CoreTabHelperDelegate* delegate() const { return delegate_; }
+  void set_delegate(CoreTabHelperDelegate* d) { delegate_ = d; }
+
   // Initial title assigned to NavigationEntries from Navigate.
   static string16 GetDefaultTitle();
 
   // Returns a human-readable description the tab's loading state.
   string16 GetStatusText() const;
 
-  // Notification that tab closing has started.  This can be called multiple
-  // times, subsequent calls are ignored.
-  void OnCloseStarted();
-
-  // Notification that tab closing was cancelled. This can happen when a user
-  // cancels a window close via another tab's beforeunload dialog.
-  void OnCloseCanceled();
-
-  // Set the time during close when unload is started. Normally, this is set
-  // after the beforeunload dialog. However, for a window close, it is set
-  // after all the beforeunload dialogs have finished.
-  void OnUnloadStarted();
-
-  // Set the time during close when the tab is no longer visible.
-  void OnUnloadDetachedStarted();
-
-  CoreTabHelperDelegate* delegate() const { return delegate_; }
-  void set_delegate(CoreTabHelperDelegate* d) { delegate_ = d; }
-
-  void set_new_tab_start_time(const base::TimeTicks& time) {
-    new_tab_start_time_ = time;
-  }
-
-  base::TimeTicks new_tab_start_time() const { return new_tab_start_time_; }
-
  private:
   explicit CoreTabHelper(content::WebContents* web_contents);
   friend class content::WebContentsUserData<CoreTabHelper>;
 
   // content::WebContentsObserver overrides:
   virtual void WasShown() OVERRIDE;
-  virtual void WebContentsDestroyed(
-      content::WebContents* web_contents) OVERRIDE;
-  virtual void BeforeUnloadFired(const base::TimeTicks& proceed_time) OVERRIDE;
-  virtual void BeforeUnloadDialogCancelled() OVERRIDE;
 
   // Delegate for notifying our owner about stuff. Not owned by us.
   CoreTabHelperDelegate* delegate_;
 
-  // The time when we started to create the new tab page.  This time is from
-  // before we created this WebContents.
-  base::TimeTicks new_tab_start_time_;
-
-  // The time that we started to close this WebContents.
-  base::TimeTicks close_start_time_;
-
-  // The time when onbeforeunload ended.
-  base::TimeTicks before_unload_end_time_;
-
-  // The time when the tab was removed from view during close.
-  base::TimeTicks unload_detached_start_time_;
-
   DISALLOW_COPY_AND_ASSIGN(CoreTabHelper);
 };
 
diff --git a/chrome/browser/ui/tabs/tab_strip_model.cc b/chrome/browser/ui/tabs/tab_strip_model.cc
index a550ca0..78c8f0e 100644
--- a/chrome/browser/ui/tabs/tab_strip_model.cc
+++ b/chrome/browser/ui/tabs/tab_strip_model.cc
@@ -1115,9 +1115,7 @@ bool TabStripModel::InternalCloseTabs(const std::vector<int>& indices,
     if (index == kNoTab)
       continue;
 
-    CoreTabHelper* core_tab_helper =
-        CoreTabHelper::FromWebContents(closing_contents);
-    core_tab_helper->OnCloseStarted();
+    closing_contents->OnCloseStarted();
 
     // Update the explicitly closed state. If the unload handlers cancel the
     // close the state is reset in Browser. We don't update the explicitly
diff --git a/chrome/browser/ui/unload_controller.cc b/chrome/browser/ui/unload_controller.cc
index 19d6154..da61646 100644
--- a/chrome/browser/ui/unload_controller.cc
+++ b/chrome/browser/ui/unload_controller.cc
@@ -4,55 +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/tab_contents/core_tab_helper.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);
 }
@@ -64,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;
   }
 
@@ -87,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
@@ -115,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();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -138,15 +99,12 @@ void UnloadController::Observe(int type,
                                const content::NotificationSource& source,
                                const content::NotificationDetails& details) {
   switch (type) {
-    case content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED: {
-      registrar_.Remove(this,
-                        content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
-                        source);
-      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.";
   }
@@ -193,41 +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 WEB_CONTENTS_DISCONNECTED was received then the notification may have
-  // already been unregistered.
-  const content::NotificationSource& source =
-      content::Source<content::WebContents>(contents);
-  if (registrar_.IsRegistered(this,
-                              content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
-                              source)) {
-    registrar_.Remove(this,
-                      content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
-                      source);
-  }
-
   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());
-    CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(contents);
-    core_tab_helper->OnUnloadDetachedStarted();
-    return true;
-  }
-  return false;
+    ClearUnloadState(contents, false);
+  registrar_.Remove(this,
+                    content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
+                    content::Source<content::WebContents>(contents));
 }
 
 void UnloadController::ProcessPendingTabs() {
@@ -238,105 +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;
-
-      CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(contents);
-      core_tab_helper->OnCloseStarted();
-
-      contents->GetRenderViewHost()->FirePageBeforeUnload(false);
+    if (web_contents->GetRenderViewHost()) {
+      web_contents->GetRenderViewHost()->FirePageBeforeUnload(false);
     } else {
-      ProcessPendingTabs();
+      ClearUnloadState(web_contents, true);
     }
-    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()) {
-        CoreTabHelper* core_tab_helper =
-            CoreTabHelper::FromWebContents(contents);
-        core_tab_helper->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 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) {
-
-    CoreTabHelper* core_tab_helper =
-        CoreTabHelper::FromWebContents(tab_needing_before_unload_ack_);
-    core_tab_helper->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;
-
-    CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(contents);
-    core_tab_helper->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(
@@ -345,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
diff --git a/chrome/browser/ui/unload_controller.h b/chrome/browser/ui/unload_controller.h
index 47e50b0..4c13c67 100644
--- a/chrome/browser/ui/unload_controller.h
+++ b/chrome/browser/ui/unload_controller.h
@@ -7,9 +7,7 @@
 
 #include <set>
 
-#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/strings/string_piece.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -24,33 +22,7 @@ class WebContents;
 }
 
 namespace chrome {
-// UnloadController manages closing tabs and windows -- especially in
-// regards to beforeunload handlers (have proceed/cancel dialogs) and
-// unload handlers (have no user interaction).
-//
-// Typical flow of closing a tab:
-//  1. Browser calls CanCloseContents().
-//     If true, browser calls contents::CloseWebContents().
-//  2. WebContents notifies us via its delegate and BeforeUnloadFired()
-//     that the beforeunload handler was run. If the user allowed the
-//     close to continue, we detached the tab and hold onto it while the
-//     close finishes.
-//
-// Typical flow of closing a window:
-//  1. BrowserView::CanClose() calls TabsNeedBeforeUnloadFired().
-//     If beforeunload/unload handlers need to run, UnloadController returns
-//     true and calls ProcessPendingTabs() (private method).
-//  2. For each tab with a beforeunload/unload handler, ProcessPendingTabs()
-//        calls |CoreTabHelper::OnCloseStarted()|
-//        and   |web_contents->GetRenderViewHost()->FirePageBeforeUnload()|.
-//  3. If the user allowed the close to continue, we detach all the tabs with
-//     unload handlers, remove them from the tab strip, and finish closing
-//     the tabs in the background.
-//  4. The browser gets notified that the tab strip is empty and calls
-//     CloseFrame where the empty tab strip causes the window to hide.
-//     Once the detached tabs finish, the browser calls CloseFrame again and
-//     the window is finally closed.
-//
+
 class UnloadController : public content::NotificationObserver,
                          public TabStripModelObserver {
  public:
@@ -77,7 +49,7 @@ class UnloadController : public content::NotificationObserver,
   }
 
   // Called in response to a request to close |browser_|'s window. Returns true
-  // when there are no remaining beforeunload handlers to be run.
+  // when there are no remaining unload handlers to be run.
   bool ShouldCloseWindow();
 
   // Returns true if |browser_| has any tabs that have BeforeUnload handlers
@@ -89,10 +61,9 @@ class UnloadController : public content::NotificationObserver,
   //             could be pursued.
   bool TabsNeedBeforeUnloadFired();
 
-  // Returns true if all tabs' beforeunload/unload events have fired.
-  bool HasCompletedUnloadProcessing() const;
-
  private:
+  typedef std::set<content::WebContents*> UnloadListenerSet;
+
   // Overridden from content::NotificationObserver:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
@@ -113,52 +84,43 @@ class UnloadController : public content::NotificationObserver,
   void TabAttachedImpl(content::WebContents* contents);
   void TabDetachedImpl(content::WebContents* contents);
 
-  // Detach |contents| and wait for it to finish closing.
-  // The close must be inititiated outside of this method.
-  // Returns true if it succeeds.
-  bool DetachWebContents(content::WebContents* contents);
-
   // Processes the next tab that needs it's beforeunload/unload event fired.
   void ProcessPendingTabs();
 
+  // Whether we've completed firing all the tabs' beforeunload/unload events.
+  bool HasCompletedUnloadProcessing() const;
+
   // Clears all the state associated with processing tabs' beforeunload/unload
   // events since the user cancelled closing the window.
   void CancelWindowClose();
 
-  // Cleans up state appropriately when we are trying to close the
-  // browser or close a tab in the background. We also use this in the
-  // cases where a tab crashes or hangs even if the
-  // beforeunload/unload haven't successfully fired.
-  void ClearUnloadState(content::WebContents* contents);
-
-  // Helper for |ClearUnloadState| to unwind stack before proceeding.
-  void PostTaskForProcessPendingTabs();
-
-  // Log a step of the unload processing.
-  void LogUnloadStep(const base::StringPiece& step_name,
-                     content::WebContents* contents) const;
+  // Removes |web_contents| from the passed |set|.
+  // Returns whether the tab was in the set in the first place.
+  bool RemoveFromSet(UnloadListenerSet* set,
+                     content::WebContents* web_contents);
+
+  // Cleans up state appropriately when we are trying to close the browser and
+  // the tab has finished firing its unload handler. We also use this in the
+  // cases where a tab crashes or hangs even if the beforeunload/unload haven't
+  // successfully fired. If |process_now| is true |ProcessPendingTabs| is
+  // invoked immediately, otherwise it is invoked after a delay (PostTask).
+  //
+  // Typically you'll want to pass in true for |process_now|. Passing in true
+  // may result in deleting |tab|. If you know that shouldn't happen (because of
+  // the state of the stack), pass in false.
+  void ClearUnloadState(content::WebContents* web_contents, bool process_now);
 
   Browser* browser_;
 
   content::NotificationRegistrar registrar_;
 
-  typedef std::set<content::WebContents*> WebContentsSet;
-
-  // Tracks tabs that need their beforeunload event started.
-  // Only gets populated when we try to close the browser.
-  WebContentsSet tabs_needing_before_unload_;
+  // Tracks tabs that need there 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 the tab that needs its beforeunload event result.
-  // Only gets populated when we try to close the browser.
-  content::WebContents* tab_needing_before_unload_ack_;
-
-  // Tracks tabs that need their unload event started.
-  // Only gets populated when we try to close the browser.
-  WebContentsSet tabs_needing_unload_;
-
-  // Tracks tabs that need to finish running their unload event.
-  // Populated both when closing individual tabs and when closing the browser.
-  WebContentsSet tabs_needing_unload_ack_;
+  // Tracks tabs that need there unload event fired before we can
+  // close the browser. Only gets populated when we try to close the browser.
+  UnloadListenerSet tabs_needing_unload_fired_;
 
   // Whether we are processing the beforeunload and unload events of each tab
   // in preparation for closing the browser. UnloadController owns this state
@@ -166,10 +128,6 @@ class UnloadController : public content::NotificationObserver,
   // Browser window isn't just immediately closed.
   bool is_attempting_to_close_browser_;
 
-  // Manage tabs with beforeunload/unload handlers that close detached.
-  class DetachedWebContentsDelegate;
-  scoped_ptr<DetachedWebContentsDelegate> detached_delegate_;
-
   base::WeakPtrFactory<UnloadController> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(UnloadController);
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index df661dc..e867372 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -1830,13 +1830,6 @@ bool BrowserView::CanClose() {
     // down. When the tab strip is empty we'll be called back again.
     frame_->Hide();
     browser_->OnWindowClosing();
-    browser_->tab_strip_model()->CloseAllTabs();
-    return false;
-  } else if (!browser_->HasCompletedUnloadProcessing()) {
-    // The browser needs to finish running unload handlers.
-    // Hide the frame (so it appears to have closed immediately), and
-    // the browser will call us back again when it is ready to close.
-    frame_->Hide();
     return false;
   }
 
diff --git a/chrome/browser/ui/webui/metrics_handler.cc b/chrome/browser/ui/webui/metrics_handler.cc
index 9394e07..b57ce0f 100644
--- a/chrome/browser/ui/webui/metrics_handler.cc
+++ b/chrome/browser/ui/webui/metrics_handler.cc
@@ -11,7 +11,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/metrics/metric_event_duration_details.h"
-#include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/user_metrics.h"
@@ -83,12 +82,10 @@ void MetricsHandler::HandleLogEventTime(const ListValue* args) {
 
   // Not all new tab pages get timed. In those cases, we don't have a
   // new_tab_start_time_.
-  CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(tab);
-  if (core_tab_helper->new_tab_start_time().is_null())
+  if (tab->GetNewTabStartTime().is_null())
     return;
 
-  base::TimeDelta duration =
-      base::TimeTicks::Now() - core_tab_helper->new_tab_start_time();
+  base::TimeDelta duration = base::TimeTicks::Now() - tab->GetNewTabStartTime();
   MetricEventDurationDetails details(event_name,
       static_cast<int>(duration.InMilliseconds()));
 
@@ -99,8 +96,7 @@ void MetricsHandler::HandleLogEventTime(const ListValue* args) {
   } else if (event_name == "Tab.NewTabOnload") {
     UMA_HISTOGRAM_TIMES("Tab.NewTabOnload", duration);
     // The new tab page has finished loading; reset it.
-    CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(tab);
-    core_tab_helper->set_new_tab_start_time(base::TimeTicks());
+    tab->SetNewTabStartTime(base::TimeTicks());
   } else {
     NOTREACHED();
   }