Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1838)

Unified Diff: chrome/browser/ui/unload_controller.cc

Issue 11016023: Quickly close tabs/window with long-running unload handlers. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Patch for landing Created 7 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/ui/unload_controller.h ('k') | chrome/browser/ui/views/frame/browser_view.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/unload_controller.cc
diff --git a/chrome/browser/ui/unload_controller.cc b/chrome/browser/ui/unload_controller.cc
index da61646b28229538ae9910d971e5dda34fa32efd..b52348c30ae25d9568f6c3ea1fb7967e96ca1f00 100644
--- a/chrome/browser/ui/unload_controller.cc
+++ b/chrome/browser/ui/unload_controller.cc
@@ -4,25 +4,54 @@
#include "chrome/browser/ui/unload_controller.h"
+#include "base/logging.h"
#include "base/message_loop.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_tabstrip.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"
#include "chrome/common/chrome_notification_types.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_source.h"
#include "content/public/browser/notification_types.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_delegate.h"
namespace chrome {
+
+////////////////////////////////////////////////////////////////////////////////
+// DetachedWebContentsDelegate will delete web contents when they close.
+class UnloadController::DetachedWebContentsDelegate
+ : public content::WebContentsDelegate {
+ public:
+ DetachedWebContentsDelegate() { }
+ virtual ~DetachedWebContentsDelegate() { }
+
+ private:
+ // WebContentsDelegate implementation.
+ virtual bool ShouldSuppressDialogs() OVERRIDE {
+ return true; // Return true so dialogs are suppressed.
+ }
+
+ virtual void CloseContents(content::WebContents* source) OVERRIDE {
+ // Finished detached close.
+ // UnloadController will observe |NOTIFICATION_WEB_CONTENTS_DISCONNECTED|.
+ delete source;
+ }
+
+ DISALLOW_COPY_AND_ASSIGN(DetachedWebContentsDelegate);
+};
+
////////////////////////////////////////////////////////////////////////////////
// UnloadController, public:
UnloadController::UnloadController(Browser* browser)
: browser_(browser),
+ tab_needing_before_unload_ack_(NULL),
is_attempting_to_close_browser_(false),
+ detached_delegate_(new DetachedWebContentsDelegate()),
weak_factory_(this) {
browser_->tab_strip_model()->AddObserver(this);
}
@@ -34,16 +63,20 @@ UnloadController::~UnloadController() {
bool UnloadController::CanCloseContents(content::WebContents* contents) {
// Don't try to close the tab when the whole browser is being closed, since
// that avoids the fast shutdown path where we just kill all the renderers.
- if (is_attempting_to_close_browser_)
- ClearUnloadState(contents, true);
return !is_attempting_to_close_browser_;
}
bool UnloadController::BeforeUnloadFired(content::WebContents* contents,
bool proceed) {
if (!is_attempting_to_close_browser_) {
- if (!proceed)
+ if (!proceed) {
contents->SetClosedByUserGesture(false);
+ } else {
+ // No more dialogs are possible, so remove the tab and finish
+ // running unload listeners asynchrounously.
+ browser_->tab_strip_model()->delegate()->CreateHistoricalTab(contents);
+ DetachWebContents(contents);
+ }
return proceed;
}
@@ -53,10 +86,10 @@ bool UnloadController::BeforeUnloadFired(content::WebContents* contents,
return false;
}
- if (RemoveFromSet(&tabs_needing_before_unload_fired_, contents)) {
- // Now that beforeunload has fired, put the tab on the queue to fire
- // unload.
- tabs_needing_unload_fired_.insert(contents);
+ if (tab_needing_before_unload_ack_ == contents) {
+ // Now that beforeunload has fired, queue the tab to fire unload.
+ tab_needing_before_unload_ack_ = NULL;
+ tabs_needing_unload_.insert(contents);
ProcessPendingTabs();
// We want to handle firing the unload event ourselves since we want to
// fire all the beforeunload events before attempting to fire the unload
@@ -81,15 +114,20 @@ bool UnloadController::ShouldCloseWindow() {
}
bool UnloadController::TabsNeedBeforeUnloadFired() {
- if (tabs_needing_before_unload_fired_.empty()) {
- for (int i = 0; i < browser_->tab_strip_model()->count(); ++i) {
- content::WebContents* contents =
- browser_->tab_strip_model()->GetWebContentsAt(i);
- if (contents->NeedToFireBeforeUnload())
- tabs_needing_before_unload_fired_.insert(contents);
- }
+ if (!tabs_needing_before_unload_.empty() ||
+ tab_needing_before_unload_ack_ != NULL)
+ return true;
+
+ if (!tabs_needing_unload_.empty())
+ return false;
+
+ for (int i = 0; i < browser_->tab_strip_model()->count(); ++i) {
+ content::WebContents* contents =
+ browser_->tab_strip_model()->GetWebContentsAt(i);
+ if (contents->NeedToFireBeforeUnload())
+ tabs_needing_before_unload_.insert(contents);
}
- return !tabs_needing_before_unload_fired_.empty();
+ return !tabs_needing_before_unload_.empty();
}
////////////////////////////////////////////////////////////////////////////////
@@ -99,12 +137,15 @@ void UnloadController::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
switch (type) {
- case content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED:
- if (is_attempting_to_close_browser_) {
- ClearUnloadState(content::Source<content::WebContents>(source).ptr(),
- false); // See comment for ClearUnloadState().
- }
+ case content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED: {
+ registrar_.Remove(this,
+ content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
+ source);
+ content::WebContents* contents =
+ content::Source<content::WebContents>(source).ptr();
+ ClearUnloadState(contents);
break;
+ }
default:
NOTREACHED() << "Got a notification we didn't register for.";
}
@@ -151,11 +192,40 @@ 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, false);
- registrar_.Remove(this,
- content::NOTIFICATION_WEB_CONTENTS_DISCONNECTED,
- content::Source<content::WebContents>(contents));
+ ClearUnloadState(contents);
+}
+
+bool UnloadController::DetachWebContents(content::WebContents* contents) {
+ int index = browser_->tab_strip_model()->GetIndexOfWebContents(contents);
+ if (index != TabStripModel::kNoTab &&
+ contents->NeedToFireBeforeUnload()) {
+ tabs_needing_unload_ack_.insert(contents);
+ browser_->tab_strip_model()->DetachWebContentsAt(index);
+ contents->SetDelegate(detached_delegate_.get());
+ contents->OnUnloadDetachedStarted();
+ return true;
+ }
+ return false;
}
void UnloadController::ProcessPendingTabs() {
@@ -166,58 +236,95 @@ void UnloadController::ProcessPendingTabs() {
return;
}
- if (HasCompletedUnloadProcessing()) {
- // We've finished all the unload events and can proceed to close the
- // browser.
- browser_->OnWindowClosing();
+ if (tab_needing_before_unload_ack_ != NULL) {
+ // Wait for |BeforeUnloadFired| before proceeding.
return;
}
- // Process beforeunload tabs first. When that queue is empty, process
- // unload tabs.
- if (!tabs_needing_before_unload_fired_.empty()) {
- content::WebContents* web_contents =
- *(tabs_needing_before_unload_fired_.begin());
+ // Process a beforeunload handler.
+ if (!tabs_needing_before_unload_.empty()) {
+ WebContentsSet::iterator it = tabs_needing_before_unload_.begin();
+ content::WebContents* contents = *it;
+ tabs_needing_before_unload_.erase(it);
// Null check render_view_host here as this gets called on a PostTask and
// the tab's render_view_host may have been nulled out.
- if (web_contents->GetRenderViewHost()) {
- web_contents->GetRenderViewHost()->FirePageBeforeUnload(false);
+ if (contents->GetRenderViewHost()) {
+ tab_needing_before_unload_ack_ = contents;
+ contents->OnCloseStarted();
+ contents->GetRenderViewHost()->FirePageBeforeUnload(false);
} else {
- ClearUnloadState(web_contents, true);
+ ProcessPendingTabs();
}
- } else if (!tabs_needing_unload_fired_.empty()) {
- // We've finished firing all beforeunload events and can proceed with unload
- // events.
- // TODO(ojan): We should add a call to browser_shutdown::OnShutdownStarting
- // somewhere around here so that we have accurate measurements of shutdown
- // time.
- // TODO(ojan): We can probably fire all the unload events in parallel and
- // get a perf benefit from that in the cases where the tab hangs in it's
- // unload handler or takes a long time to page in.
- content::WebContents* web_contents = *(tabs_needing_unload_fired_.begin());
- // Null check render_view_host here as this gets called on a PostTask and
- // the tab's render_view_host may have been nulled out.
- if (web_contents->GetRenderViewHost()) {
- web_contents->GetRenderViewHost()->ClosePage();
+ return;
+ }
+
+ // Process all the unload handlers. (The beforeunload handlers have finished.)
+ if (!tabs_needing_unload_.empty()) {
+ browser_->OnWindowClosing();
+
+ // Run unload handlers detached since no more interaction is possible.
+ WebContentsSet::iterator it = tabs_needing_unload_.begin();
+ while (it != tabs_needing_unload_.end()) {
+ WebContentsSet::iterator current = it++;
+ content::WebContents* contents = *current;
+ tabs_needing_unload_.erase(current);
+ // Null check render_view_host here as this gets called on a PostTask
+ // and the tab's render_view_host may have been nulled out.
+ if (contents->GetRenderViewHost()) {
+ contents->OnUnloadStarted();
+ DetachWebContents(contents);
+ contents->GetRenderViewHost()->ClosePage();
+ }
+ }
+
+ // Get the browser hidden.
+ if (browser_->tab_strip_model()->empty()) {
+ browser_->TabStripEmpty();
} else {
- ClearUnloadState(web_contents, true);
+ browser_->tab_strip_model()->CloseAllTabs(); // tabs not needing unload
}
- } else {
- NOTREACHED();
+ return;
+ }
+
+ if (HasCompletedUnloadProcessing()) {
+ browser_->OnWindowClosing();
+
+ // Get the browser closed.
+ if (browser_->tab_strip_model()->empty()) {
+ browser_->TabStripEmpty();
+ } else {
+ // There may be tabs if the last tab needing beforeunload crashed.
+ browser_->tab_strip_model()->CloseAllTabs();
+ }
+ return;
}
}
bool UnloadController::HasCompletedUnloadProcessing() const {
return is_attempting_to_close_browser_ &&
- tabs_needing_before_unload_fired_.empty() &&
- tabs_needing_unload_fired_.empty();
+ tabs_needing_before_unload_.empty() &&
+ tab_needing_before_unload_ack_ == NULL &&
+ tabs_needing_unload_.empty() &&
+ tabs_needing_unload_ack_.empty();
}
void UnloadController::CancelWindowClose() {
// Closing of window can be canceled from a beforeunload handler.
DCHECK(is_attempting_to_close_browser_);
- tabs_needing_before_unload_fired_.clear();
- tabs_needing_unload_fired_.clear();
+ tabs_needing_before_unload_.clear();
+ if (tab_needing_before_unload_ack_ != NULL) {
+ tab_needing_before_unload_ack_->OnCloseCanceled();
+ tab_needing_before_unload_ack_ = NULL;
+ }
+ for (WebContentsSet::iterator it = tabs_needing_unload_.begin();
+ it != tabs_needing_unload_.end(); it++) {
+ content::WebContents* contents = *it;
+ contents->OnCloseCanceled();
+ }
+ tabs_needing_unload_.clear();
+
+ // No need to clear tabs_needing_unload_ack_. Those tabs are already detached.
+
is_attempting_to_close_browser_ = false;
content::NotificationService::current()->Notify(
@@ -226,33 +333,34 @@ void UnloadController::CancelWindowClose() {
content::NotificationService::NoDetails());
}
-bool UnloadController::RemoveFromSet(UnloadListenerSet* set,
- content::WebContents* web_contents) {
- DCHECK(is_attempting_to_close_browser_);
+void UnloadController::ClearUnloadState(content::WebContents* contents) {
+ if (tabs_needing_unload_ack_.erase(contents) > 0) {
+ if (HasCompletedUnloadProcessing())
+ PostTaskForProcessPendingTabs();
+ return;
+ }
- UnloadListenerSet::iterator iter =
- std::find(set->begin(), set->end(), web_contents);
- if (iter != set->end()) {
- set->erase(iter);
- return true;
+ if (!is_attempting_to_close_browser_)
+ return;
+
+ if (tab_needing_before_unload_ack_ == contents) {
+ tab_needing_before_unload_ack_ = NULL;
+ PostTaskForProcessPendingTabs();
+ return;
}
- return false;
-}
-void UnloadController::ClearUnloadState(content::WebContents* web_contents,
- bool process_now) {
- if (is_attempting_to_close_browser_) {
- RemoveFromSet(&tabs_needing_before_unload_fired_, web_contents);
- RemoveFromSet(&tabs_needing_unload_fired_, web_contents);
- if (process_now) {
- ProcessPendingTabs();
- } else {
- base::MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&UnloadController::ProcessPendingTabs,
- weak_factory_.GetWeakPtr()));
- }
+ if (tabs_needing_before_unload_.erase(contents) > 0 ||
+ tabs_needing_unload_.erase(contents) > 0) {
+ if (tab_needing_before_unload_ack_ == NULL)
+ PostTaskForProcessPendingTabs();
}
}
+void UnloadController::PostTaskForProcessPendingTabs() {
+ base::MessageLoop::current()->PostTask(
+ FROM_HERE,
+ base::Bind(&UnloadController::ProcessPendingTabs,
+ weak_factory_.GetWeakPtr()));
+}
+
} // namespace chrome
« no previous file with comments | « chrome/browser/ui/unload_controller.h ('k') | chrome/browser/ui/views/frame/browser_view.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698