summaryrefslogtreecommitdiffstats
path: root/chrome/browser
diff options
context:
space:
mode:
authorpinkerton@chromium.org <pinkerton@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-02-18 21:51:39 +0000
committerpinkerton@chromium.org <pinkerton@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-02-18 21:51:39 +0000
commit41741a96ce6b7b3cfca3d59ef3bb59064dc939e0 (patch)
tree04691674fee10a438145d1bc42325c9ea00d2f7a /chrome/browser
parent4daf03583a367047c06cbc05aace8f9edc355237 (diff)
downloadchromium_src-41741a96ce6b7b3cfca3d59ef3bb59064dc939e0.zip
chromium_src-41741a96ce6b7b3cfca3d59ef3bb59064dc939e0.tar.gz
chromium_src-41741a96ce6b7b3cfca3d59ef3bb59064dc939e0.tar.bz2
Fix window close and application quit on Mac to call the proper Browser machinery in the proper order. Add comments to BrowserWindow::Close to stress its assumptions. DCHECK that nobody adds a NULL browser to the browser list.
Review URL: http://codereview.chromium.org/20460 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@9973 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r--chrome/browser/app_controller_mac.mm7
-rw-r--r--chrome/browser/browser_list.cc1
-rw-r--r--chrome/browser/browser_window.h6
-rw-r--r--chrome/browser/browser_window_cocoa.mm5
-rw-r--r--chrome/browser/browser_window_controller.mm48
5 files changed, 54 insertions, 13 deletions
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index 5a9609a..d7b0a72 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -8,6 +8,7 @@
#import "chrome/app/chrome_dll_resource.h"
#import "chrome/browser/browser.h"
#import "chrome/browser/browser_list.h"
+#include "chrome/browser/browser_shutdown.h"
#import "chrome/browser/command_updater.h"
#import "chrome/browser/profile_manager.h"
#import "chrome/common/temp_scaffolding_stubs.h"
@@ -48,6 +49,12 @@
// handle it. If it says to continue, post the quit message, otherwise
// go back to normal.
+ // TODO(pinkerton): Not sure where this should live, including it here
+ // causes all sorts of asserts from the open renderers. On Windows, it
+ // lives in Browser::OnWindowClosing, but that's not appropriate on Mac
+ // since we don't shut down when we reach zero windows.
+ // browser_shutdown::OnShutdownStarting(browser_shutdown::WINDOW_CLOSE);
+
// Close all the windows.
BrowserList::CloseAllBrowsers(true);
diff --git a/chrome/browser/browser_list.cc b/chrome/browser/browser_list.cc
index 5769c1b..de85eba0 100644
--- a/chrome/browser/browser_list.cc
+++ b/chrome/browser/browser_list.cc
@@ -27,6 +27,7 @@ std::vector<BrowserList::Observer*> BrowserList::observers_;
// static
void BrowserList::AddBrowser(Browser* browser) {
+ DCHECK(browser);
browsers_.push_back(browser);
g_browser_process->AddRefModule();
diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h
index 902cbf5..6a1215f 100644
--- a/chrome/browser/browser_window.h
+++ b/chrome/browser/browser_window.h
@@ -37,7 +37,11 @@ class BrowserWindow {
// Closes the frame as soon as possible. If the frame is not in a drag
// session, it will close immediately; otherwise, it will move offscreen (so
- // events are still fired) until the drag ends, then close.
+ // events are still fired) until the drag ends, then close. This assumes
+ // that the Browser is not immediately destroyed, but will be eventually
+ // destroyed by other means (eg, the tab strip going to zero elements).
+ // Bad things happen if the Browser dtor is called directly as a result of
+ // invoking this method.
virtual void Close() = 0;
// Activates (brings to front) the window. Restores the window from minimized
diff --git a/chrome/browser/browser_window_cocoa.mm b/chrome/browser/browser_window_cocoa.mm
index 0780734..bbf6735 100644
--- a/chrome/browser/browser_window_cocoa.mm
+++ b/chrome/browser/browser_window_cocoa.mm
@@ -31,6 +31,9 @@ void BrowserWindowCocoa::SetBounds(const gfx::Rect& bounds) {
[screen frame].size.height - bounds.height() - bounds.y();
}
+// Callers assume that this doesn't immediately delete the Browser object.
+// The controller implementing the window delegate methods called from
+// |-performClose:| must take precautiions to ensure that.
void BrowserWindowCocoa::Close() {
[window_ orderOut:controller_];
[window_ performClose:controller_];
@@ -97,6 +100,8 @@ bool BrowserWindowCocoa::IsFullscreen() const {
}
gfx::Rect BrowserWindowCocoa::GetRootWindowResizerRect() const {
+ // TODO(pinkerton): fill this in so scrollbars go in the correct places
+ NOTIMPLEMENTED();
return gfx::Rect();
}
diff --git a/chrome/browser/browser_window_controller.mm b/chrome/browser/browser_window_controller.mm
index 101fc9c..282193b 100644
--- a/chrome/browser/browser_window_controller.mm
+++ b/chrome/browser/browser_window_controller.mm
@@ -24,9 +24,9 @@
- (void)dealloc {
browser_->CloseAllTabs();
+ [tabStripController_ release];
delete browser_;
delete windowShim_;
- [tabStripController_ release];
[super dealloc];
}
@@ -56,24 +56,48 @@
}
- (void)destroyBrowser {
- // we need the window to go away now, other areas of code will be checking
- // the number of browser objects remaining after we finish so we can't defer
- // deletion via autorelease.
+ // We need the window to go away now.
[self autorelease];
}
-// Called when the window is closing from Cocoa. Destroy this controller,
-// which will tear down the rest of the infrastructure as the Browser is
-// itself destroyed.
+// Called when the window meets the criteria to be closed (ie,
+// |-windowShoudlClose:| returns YES). We must be careful to preserve the
+// semantics of BrowserWindow::Close() and not call the Browser's dtor directly
+// from this method.
- (void)windowWillClose:(NSNotification *)notification {
- [self autorelease];
+ DCHECK(!browser_->tabstrip_model()->count());
+
+ // We can't acutally use |-autorelease| here because there's an embedded
+ // run loop in the |-performClose:| which contains its own autorelease pool.
+ // Instead we use call it after a zero-length delay, which gets us back
+ // to the main event loop.
+ [self performSelector:@selector(autorelease)
+ withObject:nil
+ afterDelay:0];
}
-// Called when the user wants to close a window. Usually it's ok, but we may
-// want to prompt the user when they have multiple tabs open, for example.
+// Called when the user wants to close a window or from the shutdown process.
+// The Browser object is in control of whether or not we're allowed to close. It
+// may defer closing due to several states, such as onUnload handlers needing to
+// be fired. If closing is deferred, the Browser will handle the processing
+// required to get us to the closing state and (by watching for all the tabs
+// going away) will again call to close the window when it's finally ready.
- (BOOL)windowShouldClose:(id)sender {
- // TODO(pinkerton): check tab model to see if it's ok to close the
- // window. Use NSGetAlertPanel() and runModalForWindow:.
+ // Give beforeunload handlers the chance to cancel the close before we hide
+ // the window below.
+ if (!browser_->ShouldCloseWindow())
+ return NO;
+
+ if (!browser_->tabstrip_model()->empty()) {
+ // 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();
+ return NO;
+ }
+
+ // the tab strip is empty, it's ok to close the window
return YES;
}