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

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

Issue 6203006: Attempt at fixing crash in ProcessPendingTabs. With the current code, (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Better comments Created 9 years, 11 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
Index: chrome/browser/ui/browser.cc
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 49e694b494fc7e3e4ee987b1d85c43b22e4f6f82..d14e8e623cf66f6f194f8cc1c62a9a4cf5534bb6 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -2838,7 +2838,7 @@ void Browser::CloseContents(TabContents* source) {
// waiting for unload to fire. Don't actually try to close the tab as it
// will go down the slow shutdown path instead of the fast path of killing
// all the renderer processes.
- ClearUnloadState(source);
+ ClearUnloadState(source, true);
return;
}
@@ -3199,12 +3199,10 @@ void Browser::Observe(NotificationType type,
switch (type.value) {
case NotificationType::TAB_CONTENTS_DISCONNECTED:
if (is_attempting_to_close_browser_) {
- // Need to do this asynchronously as it will close the tab, which is
- // currently on the call stack above us.
- MessageLoop::current()->PostTask(
- FROM_HERE,
- method_factory_.NewRunnableMethod(&Browser::ClearUnloadState,
- Source<TabContents>(source).ptr()));
+ // Pass in false so that we delay processing. We need to delay the
+ // processing as it may close the tab, which is currently on the call
+ // stack above us.
+ ClearUnloadState(Source<TabContents>(source).ptr(), false);
}
break;
@@ -3861,7 +3859,7 @@ void Browser::ProcessPendingTabs() {
if (tab->render_view_host()) {
tab->render_view_host()->FirePageBeforeUnload(false);
} else {
- ClearUnloadState(tab);
+ ClearUnloadState(tab, true);
}
} else if (!tabs_needing_unload_fired_.empty()) {
// We've finished firing all beforeunload events and can proceed with unload
@@ -3878,7 +3876,7 @@ void Browser::ProcessPendingTabs() {
if (tab->render_view_host()) {
tab->render_view_host()->ClosePage(false, -1, -1);
} else {
- ClearUnloadState(tab);
+ ClearUnloadState(tab, true);
}
} else {
NOTREACHED();
@@ -3918,18 +3916,23 @@ bool Browser::RemoveFromSet(UnloadListenerSet* set, TabContents* tab) {
return false;
}
-void Browser::ClearUnloadState(TabContents* tab) {
+void Browser::ClearUnloadState(TabContents* tab, bool process_now) {
// Closing of browser could be canceled (via IsClosingPermitted) between the
// time when request was initiated and when this method is called, so check
// for is_attempting_to_close_browser_ flag before proceeding.
if (is_attempting_to_close_browser_) {
RemoveFromSet(&tabs_needing_before_unload_fired_, tab);
RemoveFromSet(&tabs_needing_unload_fired_, tab);
- ProcessPendingTabs();
+ if (process_now) {
+ ProcessPendingTabs();
+ } else {
+ MessageLoop::current()->PostTask(
+ FROM_HERE,
+ method_factory_.NewRunnableMethod(&Browser::ProcessPendingTabs));
+ }
}
}
-
///////////////////////////////////////////////////////////////////////////////
// Browser, In-progress download termination handling (private):
@@ -4088,6 +4091,14 @@ void Browser::TabDetachedAtImpl(TabContentsWrapper* contents, int index,
find_bar_controller_->ChangeTabContents(NULL);
}
+ if (is_attempting_to_close_browser_) {
+ // If this is the last tab with unload handlers, then ProcessPendingTabs
+ // would call back into the TabStripModel (which is invoking this method on
+ // us). Avoid that by passing in false so that the call to
+ // ProcessPendingTabs is delayed.
+ ClearUnloadState(contents->tab_contents(), false);
+ }
+
registrar_.Remove(this, NotificationType::TAB_CONTENTS_DISCONNECTED,
Source<TabContentsWrapper>(contents));
}

Powered by Google App Engine
This is Rietveld 408576698