From 18346ce5509557b3730a44d405619e4e6724754e Mon Sep 17 00:00:00 2001 From: "viettrungluu@chromium.org" Date: Sat, 1 May 2010 00:46:26 +0000 Subject: Mac: Try to make sure BrowserWindowCocoa::Close() only actually closes on the main event loop. Otherwise, we might *add* browsers synchronously in Close(), which modifies the browser list, which is not allowed. BUG=42102 TEST=enable popups, go to popuptest.com, click on multi-popup-test or whatever it's called, quit, make sure it quit cleanly, repeat a few times Review URL: http://codereview.chromium.org/1759017 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46168 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/cocoa/browser_window_cocoa.mm | 10 +++++++++- chrome/browser/cocoa/tab_window_controller.mm | 7 +++++-- 2 files changed, 14 insertions(+), 3 deletions(-) (limited to 'chrome/browser') diff --git a/chrome/browser/cocoa/browser_window_cocoa.mm b/chrome/browser/cocoa/browser_window_cocoa.mm index 7946aae..385ec9b 100644 --- a/chrome/browser/cocoa/browser_window_cocoa.mm +++ b/chrome/browser/cocoa/browser_window_cocoa.mm @@ -87,13 +87,21 @@ void BrowserWindowCocoa::Close() { // Make sure we hide the window immediately. Even though performClose: // calls orderOut: eventually, it leaves the window on-screen long enough // that we start to see tabs shutting down. http://crbug.com/23959 + // Do the close on the main event loop to meet the expectations of + // |BrowserList| -- otherwise, windows may be added or removed while the + // list is being iterated through. In particular, any immediate teardown can + // trigger Javascript to open a new window that gets added to the + // |BrowserList| while the list is still being iterated through. (See also + // the GTK implementation in browser_list_gtk.cc.) http://crbug.com/42102 // TODO(viettrungluu): This is kind of bad, since |-performClose:| calls // |-windowShouldClose:| (on its delegate, which is probably the // controller) which may return |NO| causing the window to not be closed, // thereby leaving a hidden window. In fact, our window-closing procedure // involves a (indirect) recursion on |-performClose:|, which is also bad. [window() orderOut:controller_]; - [window() performClose:controller_]; + [window() performSelector:@selector(performClose:) + withObject:controller_ + afterDelay:0]; } } diff --git a/chrome/browser/cocoa/tab_window_controller.mm b/chrome/browser/cocoa/tab_window_controller.mm index 2338483..795b1cc 100644 --- a/chrome/browser/cocoa/tab_window_controller.mm +++ b/chrome/browser/cocoa/tab_window_controller.mm @@ -70,9 +70,12 @@ - (void)removeOverlay { [self setUseOverlay:NO]; if (closeDeferred_) { - // See comment in BrowserWindowCocoa::Close() about orderOut:. + // See comments in BrowserWindowCocoa::Close() about |-orderOut:| and + // |-performSelector:...|. [[self window] orderOut:self]; - [[self window] performClose:self]; // Autoreleases the controller. + [[self window] performSelector:@selector(performClose:) + withObject:self + afterDelay:0]; } } -- cgit v1.1