diff options
25 files changed, 215 insertions, 110 deletions
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm index 6214edd..1ebff27 100644 --- a/chrome/browser/app_controller_mac.mm +++ b/chrome/browser/app_controller_mac.mm @@ -232,21 +232,18 @@ void RecordLastRunAppBundlePath() { ![self shouldQuitWithInProgressDownloads]) return NO; - // Set the state to "trying to quit", so that closing all browser windows will - // lead to termination. - browser_shutdown::SetTryingToQuit(true); - // TODO(viettrungluu): Remove Apple Event handlers here? (It's safe to leave // them in, but I'm not sure about UX; we'd also want to disable other things // though.) http://crbug.com/40861 - if (!BrowserList::size()) - return YES; + size_t num_browsers = BrowserList::size(); - // Try to close all the windows. - BrowserList::CloseAllBrowsers(true); + // Initiate a shutdown (via BrowserList::CloseAllBrowsers()) if we aren't + // already shutting down. + if (!browser_shutdown::IsTryingToQuit()) + BrowserList::CloseAllBrowsers(true); - return NO; + return num_browsers == 0 ? YES : NO; } - (void)stopTryingToTerminateApplication:(NSApplication*)app { @@ -271,9 +268,9 @@ void RecordLastRunAppBundlePath() { // There better be no browser windows left at this point. CHECK_EQ(BrowserList::size(), 0u); - // Release the reference to the browser process. Once all the browsers get - // dealloc'd, it will stop the RunLoop and fall back into main(). - g_browser_process->ReleaseModule(); + // Tell BrowserList not to keep the browser process alive. Once all the + // browsers get dealloc'd, it will stop the RunLoop and fall back into main(). + BrowserList::EndKeepAlive(); // Close these off if they have open windows. [prefsController_ close]; @@ -455,11 +452,9 @@ void RecordLastRunAppBundlePath() { // This is called after profiles have been loaded and preferences registered. // It is safe to access the default profile here. - (void)applicationDidFinishLaunching:(NSNotification*)notify { - // Hold an extra ref to the BrowserProcess singleton so it doesn't go away - // when all the browser windows get closed. We'll release it on quit which - // will be the signal to exit. - DCHECK(g_browser_process); - g_browser_process->AddRefModule(); + // Notify BrowserList to keep the application running so it doesn't go away + // when all the browser windows get closed. + BrowserList::StartKeepAlive(); bookmarkMenuBridge_.reset(new BookmarkMenuBridge([self defaultProfile])); historyMenuBridge_.reset(new HistoryMenuBridge([self defaultProfile])); diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index bd3fac1..1460041 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -83,6 +83,7 @@ #include "chrome/common/automation_constants.h" #include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" +#include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/json_value_serializer.h" #include "chrome/common/net/url_request_context_getter.h" @@ -3341,7 +3342,8 @@ void TestingAutomationProvider::OnBrowserRemoving(const Browser* browser) { // For backwards compatibility with the testing automation interface, we // want the automation provider (and hence the process) to go away when the // last browser goes away. - if (BrowserList::size() == 1) { + if (BrowserList::size() == 1 && !CommandLine::ForCurrentProcess()->HasSwitch( + switches::kKeepAliveForTest)) { // If you change this, update Observer for NotificationType::SESSION_END // below. MessageLoop::current()->PostTask(FROM_HERE, diff --git a/chrome/browser/background_contents_service.cc b/chrome/browser/background_contents_service.cc index f35420b..c10d9d2 100644 --- a/chrome/browser/background_contents_service.cc +++ b/chrome/browser/background_contents_service.cc @@ -9,7 +9,7 @@ #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "base/values.h" -#include "chrome/browser/browser_process.h" +#include "chrome/browser/browser_list.h" #include "chrome/browser/extensions/extensions_service.h" #include "chrome/browser/pref_service.h" #include "chrome/browser/profile.h" @@ -39,7 +39,8 @@ const wchar_t kFrameNameKey[] = L"name"; BackgroundContentsService::BackgroundContentsService( Profile* profile, const CommandLine* command_line) - : prefs_(NULL) { + : prefs_(NULL), + always_keep_alive_(command_line->HasSwitch(switches::kKeepAliveForTest)) { // Don't load/store preferences if the proper switch is not enabled, or if // the parent profile is off the record. if (!profile->IsOffTheRecord() && @@ -83,6 +84,14 @@ void BackgroundContentsService::StartObserving(Profile* profile) { // BackgroundContents. registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, Source<Profile>(profile)); + + if (always_keep_alive_ && !profile->IsOffTheRecord()) { + // For testing, keep the browser process alive until there is an explicit + // shutdown. + registrar_.Add(this, NotificationType::APP_TERMINATING, + NotificationService::AllSources()); + BrowserList::StartKeepAlive(); + } } void BackgroundContentsService::Observe(NotificationType type, @@ -111,6 +120,11 @@ void BackgroundContentsService::Observe(NotificationType type, ShutdownAssociatedBackgroundContents( ASCIIToUTF16(Details<Extension>(details)->id())); break; + case NotificationType::APP_TERMINATING: + // Performing an explicit shutdown, so allow the browser process to exit. + DCHECK(always_keep_alive_); + BrowserList::EndKeepAlive(); + break; default: NOTREACHED(); break; @@ -172,8 +186,12 @@ void BackgroundContentsService::CreateBackgroundContents( BackgroundContents* contents = new BackgroundContents( SiteInstance::CreateSiteInstanceForURL(profile, url), MSG_ROUTING_NONE); - contents_map_[application_id].contents = contents; - contents_map_[application_id].frame_name = frame_name; + // TODO(atwilson): Change this to send a BACKGROUND_CONTENTS_CREATED + // notification when we have a listener outside of BackgroundContentsService. + BackgroundContentsOpenedDetails details = {contents, + frame_name, + application_id}; + BackgroundContentsOpened(&details); RenderViewHost* render_view_host = contents->render_view_host(); // TODO(atwilson): Create RenderViews asynchronously to avoid increasing @@ -233,10 +251,8 @@ void BackgroundContentsService::BackgroundContentsOpened( BackgroundContentsOpenedDetails* details) { // If this is the first BackgroundContents loaded, kick ourselves into // persistent mode. - // TODO(atwilson): Enable this when we support running with no active windows - // on all platforms (http://crbug.com/45275). - // if (contents_map_.empty()) - // g_browser_process->AddRefModule(); + if (contents_map_.empty()) + BrowserList::StartKeepAlive(); // Add the passed object to our list. Should not already be tracked. DCHECK(!IsTracked(details->contents)); @@ -260,10 +276,8 @@ void BackgroundContentsService::BackgroundContentsShutdown( contents_map_.erase(appid); // If we have no more BackgroundContents active, then stop keeping the browser // process alive. - // TODO(atwilson): Enable this when we support running with no active windows - // on all platforms (http://crbug.com/45275). - // if (contents_map_.empty()) - // g_browser_process->ReleaseModule(); + if (contents_map_.empty()) + BrowserList::EndKeepAlive(); } BackgroundContents* BackgroundContentsService::GetAppBackgroundContents( diff --git a/chrome/browser/background_contents_service.h b/chrome/browser/background_contents_service.h index 48a9a3c..73d9944 100644 --- a/chrome/browser/background_contents_service.h +++ b/chrome/browser/background_contents_service.h @@ -108,6 +108,10 @@ class BackgroundContentsService : private NotificationObserver { typedef std::map<string16, BackgroundContentsInfo> BackgroundContentsMap; BackgroundContentsMap contents_map_; + // If true, should always keep the browser process alive regardless of whether + // there are any BackgroundContents (used for manual/automated testing). + bool always_keep_alive_; + DISALLOW_COPY_AND_ASSIGN(BackgroundContentsService); }; diff --git a/chrome/browser/background_contents_service_unittest.cc b/chrome/browser/background_contents_service_unittest.cc index 8e4394e..6383e2f 100644 --- a/chrome/browser/background_contents_service_unittest.cc +++ b/chrome/browser/background_contents_service_unittest.cc @@ -8,6 +8,7 @@ #include "base/scoped_ptr.h" #include "base/utf_string_conversions.h" #include "chrome/browser/background_contents_service.h" +#include "chrome/browser/browser_list.h" #include "chrome/browser/pref_service.h" #include "chrome/browser/tab_contents/background_contents.h" #include "chrome/common/chrome_switches.h" @@ -112,20 +113,14 @@ TEST_F(BackgroundContentsServiceTest, Create) { TEST_F(BackgroundContentsServiceTest, BackgroundContentsCreateDestroy) { TestingProfile profile; BackgroundContentsService service(&profile, command_line_.get()); - // TODO(atwilson): Enable the refcount part of the test once we fix the - // APP_TERMINATING notification to be sent at the proper time. - // (http://crbug.com/45275). - // - // TestingBrowserProcess* process = - // static_cast<TestingBrowserProcess*>(g_browser_process); - // unsigned int starting_ref_count = process->module_ref_count(); MockBackgroundContents* contents = new MockBackgroundContents(&profile); EXPECT_FALSE(service.IsTracked(contents)); + EXPECT_FALSE(BrowserList::WillKeepAlive()); contents->SendOpenedNotification(); - // EXPECT_EQ(starting_ref_count+1, process->module_ref_count()); + EXPECT_TRUE(BrowserList::WillKeepAlive()); EXPECT_TRUE(service.IsTracked(contents)); delete contents; - // EXPECT_EQ(starting_ref_count, process->module_ref_count()); + EXPECT_FALSE(BrowserList::WillKeepAlive()); EXPECT_FALSE(service.IsTracked(contents)); } diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index 51c7764..3ad0276 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -808,13 +808,12 @@ void Browser::OnWindowClosing() { bool exiting = false; -#if defined(OS_MACOSX) - // On Mac, closing the last window isn't usually a sign that the app is - // shutting down. - bool should_quit_if_last_browser = browser_shutdown::IsTryingToQuit(); -#else - bool should_quit_if_last_browser = true; -#endif + // Application should shutdown on last window close if the user is explicitly + // trying to quit, or if there is nothing keeping the browser alive (such as + // AppController on the Mac, or BackgroundContentsService for background + // pages). + bool should_quit_if_last_browser = + browser_shutdown::IsTryingToQuit() || !BrowserList::WillKeepAlive(); if (should_quit_if_last_browser && BrowserList::size() == 1) { browser_shutdown::OnShutdownStarting(browser_shutdown::WINDOW_CLOSE); diff --git a/chrome/browser/browser_init.cc b/chrome/browser/browser_init.cc index 97a7743..e9c2087 100644 --- a/chrome/browser/browser_init.cc +++ b/chrome/browser/browser_init.cc @@ -460,7 +460,8 @@ bool BrowserInit::LaunchBrowser( } #endif - if (command_line.HasSwitch(switches::kRestoreBackgroundContents)) { + if (command_line.HasSwitch(switches::kRestoreBackgroundContents) || + command_line.HasSwitch(switches::kKeepAliveForTest)) { // Create status icons StatusTrayManager* tray = g_browser_process->status_tray_manager(); if (tray) diff --git a/chrome/browser/browser_list.cc b/chrome/browser/browser_list.cc index 1b41be7..ed0991a 100644 --- a/chrome/browser/browser_list.cc +++ b/chrome/browser/browser_list.cc @@ -172,10 +172,10 @@ void BrowserList::RemoveBrowser(Browser* browser) { // however, many UI tests rely on this behavior so leave it be for now and // simply ignore the behavior on the Mac outside of unit tests. // TODO(andybons): Fix the UI tests to Do The Right Thing. - bool close_app_non_mac = (browsers_.size() == 1); + bool closing_last_browser = (browsers_.size() == 1); NotificationService::current()->Notify( NotificationType::BROWSER_CLOSED, - Source<Browser>(browser), Details<bool>(&close_app_non_mac)); + Source<Browser>(browser), Details<bool>(&closing_last_browser)); // Send out notifications before anything changes. Do some basic checking to // try to catch evil observers that change the list from under us. @@ -189,13 +189,24 @@ void BrowserList::RemoveBrowser(Browser* browser) { // If the last Browser object was destroyed, make sure we try to close any // remaining dependent windows too. if (browsers_.empty()) { - AllBrowsersClosed(); - delete activity_observer; activity_observer = NULL; } g_browser_process->ReleaseModule(); + + // If we're exiting, send out the APP_TERMINATING notification to allow other + // modules to shut themselves down. + if (browsers_.empty() && + (browser_shutdown::IsTryingToQuit() || + g_browser_process->IsShuttingDown())) { + // Last browser has just closed, and this is a user-initiated quit or there + // is no module keeping the app alive, so send out our notification. + NotificationService::current()->Notify(NotificationType::APP_TERMINATING, + NotificationService::AllSources(), + NotificationService::NoDetails()); + AllBrowsersClosedAndAppExiting(); + } } // static @@ -210,10 +221,22 @@ void BrowserList::RemoveObserver(BrowserList::Observer* observer) { // static void BrowserList::CloseAllBrowsers(bool use_post) { + // Tell everyone that we are shutting down. + browser_shutdown::SetTryingToQuit(true); + // Before we close the browsers shutdown all session services. That way an // exit can restore all browsers open before exiting. ProfileManager::ShutdownSessionServices(); + // If there are no browsers, send the APP_TERMINATING action here. Otherwise, + // it will be sent by RemoveBrowser() when the last browser has closed. + if (browsers_.empty()) { + NotificationService::current()->Notify(NotificationType::APP_TERMINATING, + NotificationService::AllSources(), + NotificationService::NoDetails()); + AllBrowsersClosedAndAppExiting(); + return; + } for (BrowserList::const_iterator i = BrowserList::begin(); i != BrowserList::end();) { if (use_post) { @@ -309,10 +332,36 @@ bool BrowserList::HasBrowserWithProfile(Profile* profile) { } // static -bool BrowserList::IsInPersistentMode() { - // TODO(atwilson): check the boolean state variable that you will set for - // persisent instances. - return false; +int BrowserList::keep_alive_count_ = 0; + +// static +void BrowserList::StartKeepAlive() { + // Increment the browser process refcount as long as we're keeping the + // application alive. + if (!WillKeepAlive()) + g_browser_process->AddRefModule(); + keep_alive_count_++; +} + +// static +void BrowserList::EndKeepAlive() { + DCHECK(keep_alive_count_ > 0); + keep_alive_count_--; + // Allow the app to shutdown again. + if (!WillKeepAlive()) { + g_browser_process->ReleaseModule(); + // If there are no browsers open and we aren't already shutting down, + // initiate a shutdown. Also skips shutdown if this is a unit test + // (MessageLoop::current() == null). + if (browsers_.empty() && !browser_shutdown::IsTryingToQuit() && + MessageLoop::current()) + CloseAllBrowsers(true); + } +} + +// static +bool BrowserList::WillKeepAlive() { + return keep_alive_count_ > 0; } // static diff --git a/chrome/browser/browser_list.h b/chrome/browser/browser_list.h index a6113a7..9c477c0 100644 --- a/chrome/browser/browser_list.h +++ b/chrome/browser/browser_list.h @@ -113,8 +113,19 @@ class BrowserList { // Returns true if there is at least one Browser with the specified profile. static bool HasBrowserWithProfile(Profile* profile); - // Returns true if browser is in persistent mode and false otherwise. - static bool IsInPersistentMode(); + // Tells the BrowserList to keep the application alive after the last Browser + // closes. This is implemented as a count, so callers should pair their calls + // to StartKeepAlive() with matching calls to EndKeepAlive() when they no + // longer need to keep the application running. + static void StartKeepAlive(); + + // Stops keeping the application alive after the last Browser is closed. + // Should match a previous call to StartKeepAlive(). + static void EndKeepAlive(); + + // Returns true if application will continue running after the last Browser + // closes. + static bool WillKeepAlive(); static const_iterator begin() { return browsers_.begin(); } static const_iterator end() { return browsers_.end(); } @@ -144,8 +155,8 @@ class BrowserList { // Returns true if at least one off the record session is active. static bool IsOffTheRecordSessionActive(); - // Called when the last browser is closed. - static void AllBrowsersClosed(); + // Called once there are no more browsers open and the application is exiting. + static void AllBrowsersClosedAndAppExiting(); private: // Helper method to remove a browser instance from a list of browsers @@ -154,6 +165,10 @@ class BrowserList { static BrowserVector browsers_; static BrowserVector last_active_browsers_; static ObserverList<Observer> observers_; + + // Counter of calls to StartKeepAlive(). If non-zero, the application will + // continue running after the last browser has exited. + static int keep_alive_count_; }; class TabContents; diff --git a/chrome/browser/browser_list_gtk.cc b/chrome/browser/browser_list_gtk.cc index 8633121..b6e0f06 100644 --- a/chrome/browser/browser_list_gtk.cc +++ b/chrome/browser/browser_list_gtk.cc @@ -7,7 +7,7 @@ #include <gtk/gtk.h> // static -void BrowserList::AllBrowsersClosed() { +void BrowserList::AllBrowsersClosedAndAppExiting() { // Close non-browser windows. GList* window_list = gtk_window_list_toplevels(); g_list_foreach(window_list, (GFunc)g_object_ref, NULL); diff --git a/chrome/browser/browser_list_mac.mm b/chrome/browser/browser_list_mac.mm index f65a3e5..5c2ed9a 100644 --- a/chrome/browser/browser_list_mac.mm +++ b/chrome/browser/browser_list_mac.mm @@ -8,8 +8,7 @@ #import "chrome/browser/chrome_browser_application_mac.h" // static -void BrowserList::AllBrowsersClosed() { - // Only terminate after all browsers are closed if trying quit. - if (browser_shutdown::IsTryingToQuit()) - chrome_browser_application_mac::Terminate(); +void BrowserList::AllBrowsersClosedAndAppExiting() { + // Last browser is closed, so call back to controller to shutdown the app. + chrome_browser_application_mac::Terminate(); } diff --git a/chrome/browser/browser_list_win.cc b/chrome/browser/browser_list_win.cc index a66ddec..37ee4fc 100644 --- a/chrome/browser/browser_list_win.cc +++ b/chrome/browser/browser_list_win.cc @@ -7,6 +7,6 @@ #include "views/window/window.h" // static -void BrowserList::AllBrowsersClosed() { +void BrowserList::AllBrowsersClosedAndAppExiting() { views::Window::CloseAllSecondaryWindows(); } diff --git a/chrome/browser/browser_main_mac.mm b/chrome/browser/browser_main_mac.mm index f0f0861..34cca50 100644 --- a/chrome/browser/browser_main_mac.mm +++ b/chrome/browser/browser_main_mac.mm @@ -77,9 +77,6 @@ void WillInitializeMainMessageLoop(const MainFunctionParams& parameters) { void DidEndMainMessageLoop() { AppController* appController = [NSApp delegate]; [appController didEndMainMessageLoop]; - NotificationService::current()->Notify(NotificationType::APP_TERMINATING, - NotificationService::AllSources(), - NotificationService::NoDetails()); } void RecordBreakpadStatusUMA(MetricsService* metrics) { diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index bee3eaf..2fac31a 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -547,7 +547,8 @@ void BrowserProcessImpl::DoInspectorFilesCheck() { bool BrowserProcessImpl::CanAutorestartForUpdate() const { // Check if browser is in the background and if it needs to be restarted to // apply a pending update. - return BrowserList::IsInPersistentMode() && Upgrade::IsUpdatePendingRestart(); + return BrowserList::size() == 0 && !BrowserList::WillKeepAlive() && + Upgrade::IsUpdatePendingRestart(); } // Switches enumerated here will be removed when a background instance of diff --git a/chrome/browser/browser_shutdown.cc b/chrome/browser/browser_shutdown.cc index dd541ef..7e5fded 100644 --- a/chrome/browser/browser_shutdown.cc +++ b/chrome/browser/browser_shutdown.cc @@ -43,10 +43,8 @@ using base::TimeDelta; namespace browser_shutdown { -#if defined(OS_MACOSX) // Whether the browser is trying to quit (e.g., Quit chosen from menu). bool g_trying_to_quit = false; -#endif // OS_MACOSX Time shutdown_started_; ShutdownType shutdown_type_ = NOT_VALID; @@ -262,7 +260,6 @@ void ReadLastShutdownInfo() { &ReadLastShutdownFile, type, num_procs, num_procs_slow)); } -#if defined(OS_MACOSX) void SetTryingToQuit(bool quitting) { g_trying_to_quit = quitting; } @@ -270,6 +267,5 @@ void SetTryingToQuit(bool quitting) { bool IsTryingToQuit() { return g_trying_to_quit; } -#endif } // namespace browser_shutdown diff --git a/chrome/browser/browser_shutdown.h b/chrome/browser/browser_shutdown.h index 3ab2c2e..c782f36 100644 --- a/chrome/browser/browser_shutdown.h +++ b/chrome/browser/browser_shutdown.h @@ -44,21 +44,26 @@ void Shutdown(); // Called at startup to create a histogram from our previous shutdown time. void ReadLastShutdownInfo(); -#if defined(OS_MACOSX) -// On Mac, closing the last window does not automatically quit the application. -// To actually quit, set a flag which makes final window closure trigger a quit. -// If the quit is aborted, then the flag should be reset (but see notes below on -// the proper way to do this, i.e., usually not using |SetTryingToQuit()|). +// There are various situations where the browser process should continue to +// run after the last browser window has closed - the Mac always continues +// running until the user explicitly quits, and on Windows/Linux the application +// should not shutdown when the last browser window closes if there are any +// BackgroundContents running. +// When the user explicitly chooses to shutdown the app (via the "Exit" or +// "Quit" menu items) BrowserList will call SetTryingToQuit() to tell itself to +// initiate a shutdown when the last window closes. +// If the quit is aborted, then the flag should be reset. -// This is a low-level mutator; in general, don't call it, except from -// appropriate places in the app controller. To quit, use usual means, e.g., -// using |chrome_browser_application_mac::Terminate()|. To stop quitting, use -// |chrome_browser_application_mac::CancelTerminate()|. +// This is a low-level mutator; in general, don't call SetTryingToQuit(true), +// except from appropriate places in BrowserList. To quit, use usual means, +// e.g., using |chrome_browser_application_mac::Terminate()| on the Mac, or +// |BrowserList::CloseAllWindowsAndExit()| on other platforms. To stop quitting, +// use |chrome_browser_application_mac::CancelTerminate()| on the Mac; other +// platforms can call SetTryingToQuit(false) directly. void SetTryingToQuit(bool quitting); // General accessor. bool IsTryingToQuit(); -#endif // OS_MACOSX } // namespace browser_shutdown diff --git a/chrome/browser/browser_uitest.cc b/chrome/browser/browser_uitest.cc index eb66e13..2058b18 100644 --- a/chrome/browser/browser_uitest.cc +++ b/chrome/browser/browser_uitest.cc @@ -329,4 +329,30 @@ TEST_F(AppModeTest, EnableAppModeTest) { EXPECT_EQ(Browser::TYPE_APP, type); } +// Tests to ensure that the browser continues running in the background after +// the last window closes. +class RunInBackgroundTest : public UITest { + public: + RunInBackgroundTest() { + launch_arguments_.AppendSwitch(switches::kKeepAliveForTest); + } +}; + +TEST_F(RunInBackgroundTest, RunInBackgroundBasicTest) { + // Close the browser window, then open a new one - the browser should keep + // running. + scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0)); + ASSERT_TRUE(browser.get()); + int window_count; + ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count)); + EXPECT_EQ(1, window_count); + ASSERT_TRUE(browser->RunCommand(IDC_CLOSE_WINDOW)); + ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count)); + EXPECT_EQ(0, window_count); + ASSERT_TRUE(IsBrowserRunning()); + ASSERT_TRUE(automation()->OpenNewBrowserWindow(Browser::TYPE_NORMAL, true)); + ASSERT_TRUE(automation()->GetBrowserWindowCount(&window_count)); + EXPECT_EQ(1, window_count); +} + } // namespace diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc index cd040f8..3cde54e 100644 --- a/chrome/browser/extensions/extension_process_manager.cc +++ b/chrome/browser/extensions/extension_process_manager.cc @@ -51,13 +51,8 @@ ExtensionProcessManager::ExtensionProcessManager(Profile* profile) NotificationService::AllSources()); registrar_.Add(this, NotificationType::RENDERER_PROCESS_CLOSED, NotificationService::AllSources()); -#if defined(OS_WIN) || defined(OS_LINUX) - registrar_.Add(this, NotificationType::BROWSER_CLOSED, - NotificationService::AllSources()); -#elif defined(OS_MACOSX) registrar_.Add(this, NotificationType::APP_TERMINATING, NotificationService::AllSources()); -#endif } ExtensionProcessManager::~ExtensionProcessManager() { @@ -285,22 +280,14 @@ void ExtensionProcessManager::Observe(NotificationType type, UnregisterExtensionProcess(host->id()); break; } -#if defined(OS_WIN) || defined(OS_LINUX) - case NotificationType::BROWSER_CLOSED: { + + case NotificationType::APP_TERMINATING: { // Close background hosts when the last browser is closed so that they // have time to shutdown various objects on different threads. Our // destructor is called too late in the shutdown sequence. - bool app_closing = *Details<bool>(details).ptr(); - if (app_closing) - CloseBackgroundHosts(); - break; - } -#elif defined(OS_MACOSX) - case NotificationType::APP_TERMINATING: { CloseBackgroundHosts(); break; } -#endif default: NOTREACHED(); diff --git a/chrome/browser/js_modal_dialog.cc b/chrome/browser/js_modal_dialog.cc index 111aa01..7c42c71 100644 --- a/chrome/browser/js_modal_dialog.cc +++ b/chrome/browser/js_modal_dialog.cc @@ -5,6 +5,7 @@ #include "chrome/browser/js_modal_dialog.h" #include "base/string_util.h" +#include "chrome/browser/browser_shutdown.h" #include "chrome/browser/extensions/extension_host.h" #include "chrome/browser/tab_contents/tab_contents.h" #include "chrome/common/notification_service.h" @@ -90,6 +91,11 @@ void JavaScriptAppModalDialog::InitNotifications() { } void JavaScriptAppModalDialog::OnCancel() { + // If we are shutting down and this is an onbeforeunload dialog, cancel the + // shutdown. + if (is_before_unload_dialog_) + browser_shutdown::SetTryingToQuit(false); + // We need to do this before WM_DESTROY (WindowClosing()) as any parent frame // will receive its activation messages before this dialog receives // WM_DESTROY. The parent frame would then try to activate any modal dialogs diff --git a/chrome/browser/profile_manager.cc b/chrome/browser/profile_manager.cc index 1e948a3..8770b1d 100644 --- a/chrome/browser/profile_manager.cc +++ b/chrome/browser/profile_manager.cc @@ -39,6 +39,8 @@ // static void ProfileManager::ShutdownSessionServices() { ProfileManager* pm = g_browser_process->profile_manager(); + if (!pm) // Is NULL when running unit tests. + return; for (ProfileManager::const_iterator i = pm->begin(); i != pm->end(); ++i) (*i)->ShutdownSessionService(); } diff --git a/chrome/browser/tab_contents/background_contents.cc b/chrome/browser/tab_contents/background_contents.cc index 2e65d9d..f8c466d 100644 --- a/chrome/browser/tab_contents/background_contents.cc +++ b/chrome/browser/tab_contents/background_contents.cc @@ -34,8 +34,13 @@ BackgroundContents::BackgroundContents(SiteInstance* site_instance, session_storage_namespace_id); render_view_host_->AllowScriptToClose(true); + // Close ourselves when the application is shutting down. + registrar_.Add(this, NotificationType::APP_TERMINATING, + NotificationService::AllSources()); + // Register for our parent profile to shutdown, so we can shut ourselves down - // as well. + // as well (should only be called for OTR profiles, as we should receive + // APP_TERMINATING before non-OTR profiles are destroyed). registrar_.Add(this, NotificationType::PROFILE_DESTROYED, Source<Profile>(profile)); } @@ -51,7 +56,8 @@ void BackgroundContents::Observe(NotificationType type, // TODO(rafaelw): Implement pagegroup ref-counting so that non-persistent // background pages are closed when the last referencing frame is closed. switch (type.value) { - case NotificationType::PROFILE_DESTROYED: { + case NotificationType::PROFILE_DESTROYED: + case NotificationType::APP_TERMINATING: { delete this; break; } diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index ccef475..3c4255a 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -564,6 +564,9 @@ const char kIssue35198Permission[] = "issue35198-permission"; // Specifies the flags passed to JS engine const char kJavaScriptFlags[] = "js-flags"; +// Used for testing - keeps browser alive after last browser window closes. +const char kKeepAliveForTest[] = "keep-alive-for-test"; + // Load an extension from the specified directory. const char kLoadExtension[] = "load-extension"; diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index ecc92fb..70a8268 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -170,6 +170,7 @@ extern const char kIssue35198CrxDirBrowser[]; extern const char kIssue35198ExtraLogging[]; extern const char kIssue35198Permission[]; extern const char kJavaScriptFlags[]; +extern const char kKeepAliveForTest[]; extern const char kLoadExtension[]; extern const char kLoadPlugin[]; extern const char kExtraPluginDir[]; diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h index bf0d053..1bafff6 100644 --- a/chrome/common/notification_type.h +++ b/chrome/common/notification_type.h @@ -198,10 +198,11 @@ class NotificationType { // This message is sent after a window has been closed. The source is a // Source<Browser> containing the affected Browser. Details is a boolean - // that if true indicates that the application will be closed as a result of - // this browser window closure (i.e. this was the last opened browser - // window on win/linux). Note that the boolean pointed to by details is - // only valid for the duration of this call. + // that if true indicates that the last browser window has closed - this + // does not indicate that the application is exiting (observers should + // listen for APP_TERMINATING if they want to detect when the application + // will shut down). Note that the boolean pointed to by details is only + // valid for the duration of this call. BROWSER_CLOSED, // This message is sent when the last window considered to be an @@ -214,11 +215,16 @@ class NotificationType { // This message is sent when the application is made active (Mac OS X only // at present). No source or details are passed. APP_ACTIVATED, +#endif - // This message is sent when the application is terminating (Mac OS X only - // at present). No source or details are passed. + // This message is sent when the application is terminating (the last + // browser window has shutdown as part of an explicit user-initiated exit, + // or the user closed the last browser window on Windows/Linux and there are + // no BackgroundContents keeping the browser running). No source or details + // are passed. APP_TERMINATING, +#if defined(OS_MACOSX) // This notification is sent when the app has no key window, such as when // all windows are closed but the app is still active. No source or details // are provided. diff --git a/chrome/test/testing_browser_process.h b/chrome/test/testing_browser_process.h index c69af1c..a954e10 100644 --- a/chrome/test/testing_browser_process.h +++ b/chrome/test/testing_browser_process.h @@ -133,10 +133,6 @@ class TestingBrowserProcess : public BrowserProcess { return --module_ref_count_; } - unsigned int module_ref_count() { - return module_ref_count_; - } - virtual bool IsShuttingDown() { return false; } |