summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorjeremy@chromium.org <jeremy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-06-21 05:16:44 +0000
committerjeremy@chromium.org <jeremy@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-06-21 05:16:44 +0000
commited1bb720731ea7c7644c208da970f0609a7e3b62 (patch)
tree5d66f86a406225f13f3c87485db4e2dbdde688b0
parent8dc0d4052bbc6e023577e372679d471da9717236 (diff)
downloadchromium_src-ed1bb720731ea7c7644c208da970f0609a7e3b62.zip
chromium_src-ed1bb720731ea7c7644c208da970f0609a7e3b62.tar.gz
chromium_src-ed1bb720731ea7c7644c208da970f0609a7e3b62.tar.bz2
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
-rw-r--r--chrome/browser/ui/browser.cc14
-rw-r--r--chrome/browser/ui/browser.h10
-rw-r--r--chrome/browser/ui/browser_tabstrip.cc5
-rw-r--r--chrome/browser/ui/cocoa/applescript/window_applescript.mm8
-rw-r--r--chrome/browser/ui/cocoa/browser_window_controller.mm9
-rw-r--r--chrome/browser/ui/gtk/browser_window_gtk.cc7
-rw-r--r--chrome/browser/ui/tab_contents/core_tab_helper.cc48
-rw-r--r--chrome/browser/ui/tab_contents/core_tab_helper.h46
-rw-r--r--chrome/browser/ui/tabs/tab_strip_model.cc4
-rw-r--r--chrome/browser/ui/unload_controller.cc280
-rw-r--r--chrome/browser/ui/unload_controller.h98
-rw-r--r--chrome/browser/ui/views/frame/browser_view.cc7
-rw-r--r--chrome/browser/ui/webui/metrics_handler.cc10
-rw-r--r--chrome/browser/unload_browsertest.cc205
-rw-r--r--chrome/test/data/fast_tab_close/no_listeners.html11
-rw-r--r--chrome/test/data/fast_tab_close/unload_sets_cookie.html18
-rw-r--r--chrome/test/data/fast_tab_close/unload_sleep_before_cookie.html28
-rw-r--r--content/browser/web_contents/web_contents_impl.cc32
-rw-r--r--content/browser/web_contents/web_contents_impl.h12
-rw-r--r--content/public/browser/web_contents.h9
-rw-r--r--content/public/browser/web_contents_observer.h6
21 files changed, 175 insertions, 692 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();
}
diff --git a/chrome/browser/unload_browsertest.cc b/chrome/browser/unload_browsertest.cc
index 331b23c..9552d0a 100644
--- a/chrome/browser/unload_browsertest.cc
+++ b/chrome/browser/unload_browsertest.cc
@@ -411,210 +411,5 @@ IN_PROC_BROWSER_TEST_F(UnloadTest, BrowserCloseTabWhenOtherTabHasListener) {
CheckTitle("only_one_unload");
}
-
-class FastUnloadTest : public UnloadTest {
- public:
- virtual void SetUpInProcessBrowserTestFixture() OVERRIDE {
- ASSERT_TRUE(test_server()->Start());
- }
-
- virtual void TearDownInProcessBrowserTestFixture() OVERRIDE {
- test_server()->Stop();
- }
-
- GURL GetUrl(const std::string& name) {
- return GURL(test_server()->GetURL(
- "files/fast_tab_close/" + name + ".html"));
- }
-
- void NavigateToPage(const char* name) {
- ui_test_utils::NavigateToURL(browser(), GetUrl(name));
- CheckTitle(name);
- }
-
- void NavigateToPageInNewTab(const char* name) {
- ui_test_utils::NavigateToURLWithDisposition(
- browser(), GetUrl(name), NEW_FOREGROUND_TAB,
- ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
- CheckTitle(name);
- }
-
- std::string GetCookies(const char* name) {
- content::WebContents* contents =
- browser()->tab_strip_model()->GetActiveWebContents();
- return content::GetCookies(contents->GetBrowserContext(), GetUrl(name));
- }
-};
-
-class FastTabCloseTabStripModelObserver : public TabStripModelObserver {
- public:
- FastTabCloseTabStripModelObserver(TabStripModel* model,
- base::RunLoop* run_loop)
- : model_(model),
- run_loop_(run_loop) {
- model_->AddObserver(this);
- }
-
- virtual ~FastTabCloseTabStripModelObserver() {
- model_->RemoveObserver(this);
- }
-
- // TabStripModelObserver:
- virtual void TabDetachedAt(content::WebContents* contents,
- int index) OVERRIDE {
- run_loop_->Quit();
- }
-
- private:
- TabStripModel* const model_;
- base::RunLoop* const run_loop_;
-};
-
-
-// Test that fast-tab-close works when closing a tab with an unload handler
-// (http://crbug.com/142458).
-IN_PROC_BROWSER_TEST_F(FastUnloadTest, UnloadHidden) {
- NavigateToPage("no_listeners");
- NavigateToPageInNewTab("unload_sets_cookie");
- EXPECT_EQ("", GetCookies("no_listeners"));
-
- {
- base::RunLoop run_loop;
- FastTabCloseTabStripModelObserver observer(
- browser()->tab_strip_model(), &run_loop);
- chrome::CloseTab(browser());
- run_loop.Run();
- }
-
- // Check that the browser only has the original tab.
- CheckTitle("no_listeners");
- EXPECT_EQ(1, browser()->tab_strip_model()->count());
-
- // Show that the web contents to go away after the was removed.
- // Without unload-detached, this times-out because it happens earlier.
- content::WindowedNotificationObserver contents_destroyed_observer(
- content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
- content::NotificationService::AllSources());
- contents_destroyed_observer.Wait();
-
- // Browser still has the same tab.
- CheckTitle("no_listeners");
- EXPECT_EQ(1, browser()->tab_strip_model()->count());
- EXPECT_EQ("unloaded=ohyeah", GetCookies("no_listeners"));
-}
-
-// Test that fast-tab-close does not break a solo tab.
-IN_PROC_BROWSER_TEST_F(FastUnloadTest, PRE_ClosingLastTabFinishesUnload) {
- // The unload handler sleeps before setting the cookie to catch cases when
- // unload handlers are not allowed to run to completion. (For example,
- // using the detached handler for the tab and then closing the browser.)
- NavigateToPage("unload_sleep_before_cookie");
- EXPECT_EQ(1, browser()->tab_strip_model()->count());
- EXPECT_EQ("", GetCookies("unload_sleep_before_cookie"));
-
- content::WindowedNotificationObserver window_observer(
- chrome::NOTIFICATION_BROWSER_CLOSED,
- content::NotificationService::AllSources());
- chrome::CloseTab(browser());
- window_observer.Wait();
-}
-IN_PROC_BROWSER_TEST_F(FastUnloadTest, ClosingLastTabFinishesUnload) {
- // Check for cookie set in unload handler of PRE_ test.
- NavigateToPage("no_listeners");
- EXPECT_EQ("unloaded=ohyeah", GetCookies("no_listeners"));
-}
-
-// Test that fast-tab-close does not break window close.
-IN_PROC_BROWSER_TEST_F(FastUnloadTest, PRE_WindowCloseFinishesUnload) {
- NavigateToPage("no_listeners");
-
- // The unload handler sleeps before setting the cookie to catch cases when
- // unload handlers are not allowed to run to completion. Without the sleep,
- // the cookie can get set even if the browser does not wait for
- // the unload handler to finish.
- NavigateToPageInNewTab("unload_sleep_before_cookie");
- EXPECT_EQ(2, browser()->tab_strip_model()->count());
- EXPECT_EQ("", GetCookies("no_listeners"));
-
- content::WindowedNotificationObserver window_observer(
- chrome::NOTIFICATION_BROWSER_CLOSED,
- content::NotificationService::AllSources());
- chrome::CloseWindow(browser());
- window_observer.Wait();
-}
-IN_PROC_BROWSER_TEST_F(FastUnloadTest, WindowCloseFinishesUnload) {
- // Check for cookie set in unload during PRE_ test.
- NavigateToPage("no_listeners");
- EXPECT_EQ("unloaded=ohyeah", GetCookies("no_listeners"));
-}
-
-// Test that a tab crash during unload does not break window close.
-//
-// Hits assertion on Linux and Mac:
-// [FATAL:profile_destroyer.cc(46)] Check failed:
-// hosts.empty() ||
-// profile->IsOffTheRecord() ||
-// content::RenderProcessHost::run_renderer_in_process().
-// More details: The renderer process host matches the closed, crashed tab.
-// The |UnloadController| receives |NOTIFICATION_WEB_CONTENTS_DISCONNECTED|
-// and proceeds with the close.
-IN_PROC_BROWSER_TEST_F(FastUnloadTest, DISABLED_WindowCloseAfterUnloadCrash) {
- NavigateToPage("no_listeners");
- NavigateToPageInNewTab("unload_sets_cookie");
- content::WebContents* unload_contents =
- browser()->tab_strip_model()->GetActiveWebContents();
- EXPECT_EQ("", GetCookies("no_listeners"));
-
- {
- base::RunLoop run_loop;
- FastTabCloseTabStripModelObserver observer(
- browser()->tab_strip_model(), &run_loop);
- chrome::CloseTab(browser());
- run_loop.Run();
- }
-
- // Check that the browser only has the original tab.
- CheckTitle("no_listeners");
- EXPECT_EQ(1, browser()->tab_strip_model()->count());
-
- CrashTab(unload_contents);
-
- // Check that the browser only has the original tab.
- CheckTitle("no_listeners");
- EXPECT_EQ(1, browser()->tab_strip_model()->count());
-
- content::WindowedNotificationObserver window_observer(
- chrome::NOTIFICATION_BROWSER_CLOSED,
- content::NotificationService::AllSources());
- chrome::CloseWindow(browser());
- window_observer.Wait();
-}
-
-// Times out on Windows and Linux.
-#if defined(OS_WIN) || defined(OS_LINUX)
-#define MAYBE_WindowCloseAfterBeforeUnloadCrash \
- DISABLED_WindowCloseAfterBeforeUnloadCrash
-#else
-#define MAYBE_WindowCloseAfterBeforeUnloadCrash \
- WindowCloseAfterBeforeUnloadCrash
-#endif
-IN_PROC_BROWSER_TEST_F(FastUnloadTest,
- MAYBE_WindowCloseAfterBeforeUnloadCrash) {
- // Tests makes no sense in single-process mode since the renderer is hung.
- if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess))
- return;
-
- NavigateToDataURL(BEFORE_UNLOAD_HTML, "beforeunload");
- content::WebContents* beforeunload_contents =
- browser()->tab_strip_model()->GetActiveWebContents();
-
- content::WindowedNotificationObserver window_observer(
- chrome::NOTIFICATION_BROWSER_CLOSED,
- content::NotificationService::AllSources());
- chrome::CloseWindow(browser());
- CrashTab(beforeunload_contents);
- window_observer.Wait();
-}
-
// TODO(ojan): Add tests for unload/beforeunload that have multiple tabs
// and multiple windows.
diff --git a/chrome/test/data/fast_tab_close/no_listeners.html b/chrome/test/data/fast_tab_close/no_listeners.html
deleted file mode 100644
index 8a61cb4..0000000
--- a/chrome/test/data/fast_tab_close/no_listeners.html
+++ /dev/null
@@ -1,11 +0,0 @@
-<!--
-// Copyright 2013 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.
--->
-<html>
-<head>
- <title>no_listeners</title>
-</head>
-<body></body>
-</html>
diff --git a/chrome/test/data/fast_tab_close/unload_sets_cookie.html b/chrome/test/data/fast_tab_close/unload_sets_cookie.html
deleted file mode 100644
index 45b8a50..0000000
--- a/chrome/test/data/fast_tab_close/unload_sets_cookie.html
+++ /dev/null
@@ -1,18 +0,0 @@
-<!--
-// Copyright 2013 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.
--->
-<html>
-<head>
- <title>unload_sets_cookie</title>
-<script>
-addEventListener("unload", function() {
- var expires = new Date();
- expires.setMinutes(expires.getMinutes() + 1);
- document.cookie = "unloaded=ohyeah;path=/;expires=" + expires.toUTCString();
-}, false);
-</script>
-</head>
-<body></body>
-</html>
diff --git a/chrome/test/data/fast_tab_close/unload_sleep_before_cookie.html b/chrome/test/data/fast_tab_close/unload_sleep_before_cookie.html
deleted file mode 100644
index 93b700b..0000000
--- a/chrome/test/data/fast_tab_close/unload_sleep_before_cookie.html
+++ /dev/null
@@ -1,28 +0,0 @@
-<!--
-// Copyright 2013 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.
--->
-<html>
-<head>
- <title>unload_sleep_before_cookie</title>
-<script>
-function sleep_ms(ms) {
- var start_ms = Number(new Date());
- var remaining_ms = ms;
- while (remaining_ms > 0) {
- remaining_ms = ms - (Number(new Date()) - start_ms);
- }
-}
-addEventListener("unload", function() {
- sleep_ms(200);
-
- var expires = new Date();
- expires.setMinutes(expires.getMinutes() + 1);
- document.cookie = "unloaded=ohyeah;path=/;expires=" + expires.toUTCString();
- console.log("set the cookie");
-}, false);
-</script>
-</head>
-<body></body>
-</html>
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 7a4e607..ee3eaa4 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -9,7 +9,6 @@
#include "base/command_line.h"
#include "base/debug/trace_event.h"
#include "base/lazy_instance.h"
-#include "base/logging.h"
#include "base/metrics/histogram.h"
#include "base/metrics/stats_counters.h"
#include "base/strings/string16.h"
@@ -399,6 +398,16 @@ WebContentsImpl::~WebContentsImpl() {
}
#endif
+ // OnCloseStarted isn't called in unit tests.
+ if (!close_start_time_.is_null()) {
+ base::TimeTicks now = base::TimeTicks::Now();
+ base::TimeTicks unload_start_time = close_start_time_;
+ if (!before_unload_end_time_.is_null())
+ unload_start_time = before_unload_end_time_;
+ UMA_HISTOGRAM_TIMES("Tab.Close", now - close_start_time_);
+ UMA_HISTOGRAM_TIMES("Tab.Close.UnloadTime", now - unload_start_time);
+ }
+
FOR_EACH_OBSERVER(WebContentsObserver,
observers_,
WebContentsImplDestroyed());
@@ -1939,10 +1948,23 @@ RendererPreferences* WebContentsImpl::GetMutableRendererPrefs() {
return &renderer_preferences_;
}
+void WebContentsImpl::SetNewTabStartTime(const base::TimeTicks& time) {
+ new_tab_start_time_ = time;
+}
+
+base::TimeTicks WebContentsImpl::GetNewTabStartTime() const {
+ return new_tab_start_time_;
+}
+
void WebContentsImpl::Close() {
Close(GetRenderViewHost());
}
+void WebContentsImpl::OnCloseStarted() {
+ if (close_start_time_.is_null())
+ close_start_time_ = base::TimeTicks::Now();
+}
+
void WebContentsImpl::DragSourceEndedAt(int client_x, int client_y,
int screen_x, int screen_y, WebKit::WebDragOperation operation) {
if (browser_plugin_embedder_.get())
@@ -3476,11 +3498,9 @@ void WebContentsImpl::WorkerCrashed() {
void WebContentsImpl::BeforeUnloadFiredFromRenderManager(
bool proceed, const base::TimeTicks& proceed_time,
bool* proceed_to_fire_unload) {
- FOR_EACH_OBSERVER(WebContentsObserver, observers_,
- BeforeUnloadFired(proceed_time));
+ before_unload_end_time_ = proceed_time;
if (delegate_)
delegate_->BeforeUnloadFired(this, proceed, proceed_to_fire_unload);
- // Note: |this| can be deleted at this point.
}
void WebContentsImpl::RenderViewGoneFromRenderManager(
@@ -3621,8 +3641,8 @@ void WebContentsImpl::OnDialogClosed(RenderViewHost* rvh,
DidStopLoading(rvh);
controller_.DiscardNonCommittedEntries();
- FOR_EACH_OBSERVER(WebContentsObserver, observers_,
- BeforeUnloadDialogCancelled());
+ close_start_time_ = base::TimeTicks();
+ before_unload_end_time_ = base::TimeTicks();
}
is_showing_before_unload_dialog_ = false;
static_cast<RenderViewHostImpl*>(
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index df7d535..a91a4c6 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -264,7 +264,10 @@ class CONTENT_EXPORT WebContentsImpl
virtual void SetOverrideEncoding(const std::string& encoding) OVERRIDE;
virtual void ResetOverrideEncoding() OVERRIDE;
virtual RendererPreferences* GetMutableRendererPrefs() OVERRIDE;
+ virtual void SetNewTabStartTime(const base::TimeTicks& time) OVERRIDE;
+ virtual base::TimeTicks GetNewTabStartTime() const OVERRIDE;
virtual void Close() OVERRIDE;
+ virtual void OnCloseStarted() OVERRIDE;
virtual void SystemDragEnded() OVERRIDE;
virtual void UserGestureDone() OVERRIDE;
virtual void SetClosedByUserGesture(bool value) OVERRIDE;
@@ -867,6 +870,15 @@ class CONTENT_EXPORT WebContentsImpl
// Settings that get passed to the renderer process.
RendererPreferences renderer_preferences_;
+ // The time that we started to create the new tab page.
+ 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 that this tab was last selected.
base::TimeTicks last_selected_time_;
diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h
index 037fef84..309e264 100644
--- a/content/public/browser/web_contents.h
+++ b/content/public/browser/web_contents.h
@@ -361,10 +361,19 @@ class WebContents : public PageNavigator,
// Returns the settings which get passed to the renderer.
virtual content::RendererPreferences* GetMutableRendererPrefs() = 0;
+ // Set the time when we started to create the new tab page. This time is
+ // from before we created this WebContents.
+ virtual void SetNewTabStartTime(const base::TimeTicks& time) = 0;
+ virtual base::TimeTicks GetNewTabStartTime() const = 0;
+
// Tells the tab to close now. The tab will take care not to close until it's
// out of nested message loops.
virtual void Close() = 0;
+ // Notification that tab closing has started. This can be called multiple
+ // times, subsequent calls are ignored.
+ virtual void OnCloseStarted() = 0;
+
// A render view-originated drag has ended. Informs the render view host and
// WebContentsDelegate.
virtual void SystemDragEnded() = 0;
diff --git a/content/public/browser/web_contents_observer.h b/content/public/browser/web_contents_observer.h
index b9ce1e6..0a01171 100644
--- a/content/public/browser/web_contents_observer.h
+++ b/content/public/browser/web_contents_observer.h
@@ -259,12 +259,6 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener,
// Invoked before a form repost warning is shown.
virtual void BeforeFormRepostWarningShow() {}
- // Invoked when the before unload fires. The time is from the renderer.
- virtual void BeforeUnloadFired(const base::TimeTicks& proceed_time) {}
-
- // Invoked when a user cancels a before unload dialog.
- virtual void BeforeUnloadDialogCancelled() {}
-
// IPC::Listener implementation.
virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;