diff options
author | cmasone@google.com <cmasone@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-07 03:09:33 +0000 |
---|---|---|
committer | cmasone@google.com <cmasone@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-07 03:09:33 +0000 |
commit | edb7c860f97a0fadc7cda85b5a4581941a7f00bf (patch) | |
tree | a6100359380ee849545fad19bbe68db5972dbb21 /chrome | |
parent | 1f070f059c645668e6b0446b726f5a08613c20d0 (diff) | |
download | chromium_src-edb7c860f97a0fadc7cda85b5a4581941a7f00bf.zip chromium_src-edb7c860f97a0fadc7cda85b5a4581941a7f00bf.tar.gz chromium_src-edb7c860f97a0fadc7cda85b5a4581941a7f00bf.tar.bz2 |
Adding a SIGTERM handler for OS_POSIX builds. This is needed so that Chrome can shut down gracefully when many posix-based system halt or reboot while Chrome is open.
SIGTERM may come in on any thread, so the handler creates a Task object that wraps up a call to BrowserList::CloseAllBrowsers(true) and Posts it to the message loop of the UI thread. Thus, we both get out of the signal handler quickly and can deal with the signal on any thread.
BUG=23551
TEST=covered by BrowserTest.PosixSessionEnd
Review URL: http://codereview.chromium.org/255036
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@28225 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r-- | chrome/browser/browser_main.cc | 25 | ||||
-rw-r--r-- | chrome/browser/browser_uitest.cc | 52 | ||||
-rw-r--r-- | chrome/test/ui/ui_test.cc | 13 |
3 files changed, 79 insertions, 11 deletions
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc index 03ee3f9..2bb6981 100644 --- a/chrome/browser/browser_main.cc +++ b/chrome/browser/browser_main.cc @@ -159,6 +159,24 @@ void RunUIMessageLoop(BrowserProcess* browser_process) { void SIGCHLDHandler(int signal) { } +// See comment below, where sigaction is called. +void SIGTERMHandler(int signal) { + DCHECK_EQ(signal, SIGTERM); + LOG(WARNING) << "Addressing SIGTERM on " << PlatformThread::CurrentId(); + MessageLoop* main_loop = ChromeThread::GetMessageLoop(ChromeThread::UI); + if (main_loop) { + main_loop->PostTask(FROM_HERE, + NewRunnableFunction( + BrowserList::CloseAllBrowsers, true)); + } + // Reinstall the default handler. We had one shot at graceful shutdown. + LOG(WARNING) << "Posted task to UI thread; resetting SIGTERM handler."; + struct sigaction term_action; + memset(&term_action, 0, sizeof(term_action)); + term_action.sa_handler = SIG_DFL; + CHECK(sigaction(SIGTERM, &term_action, NULL) == 0); +} + // Sets the file descriptor soft limit to |max_descriptors| or the OS hard // limit, whichever is lower. void SetFileDescriptorLimit(unsigned int max_descriptors) { @@ -237,6 +255,13 @@ int BrowserMain(const MainFunctionParams& parameters) { action.sa_handler = SIGCHLDHandler; CHECK(sigaction(SIGCHLD, &action, NULL) == 0); + // We need to handle SIGTERM, because that is how many POSIX-based distros ask + // processes to quit gracefully at shutdown time. + struct sigaction term_action; + memset(&term_action, 0, sizeof(term_action)); + term_action.sa_handler = SIGTERMHandler; + CHECK(sigaction(SIGTERM, &term_action, NULL) == 0); + const std::wstring fd_limit_string = parsed_command_line.GetSwitchValue(switches::kFileDescriptorLimit); int fd_limit = 0; diff --git a/chrome/browser/browser_uitest.cc b/chrome/browser/browser_uitest.cc index 0a3fee9..25b4397 100644 --- a/chrome/browser/browser_uitest.cc +++ b/chrome/browser/browser_uitest.cc @@ -23,7 +23,49 @@ namespace { +// Delay to let the browser shut down before trying more brutal methods. +static const int kWaitForTerminateMsec = 30000; + class BrowserTest : public UITest { + + protected: + void TerminateBrowser() { +#if defined(OS_WIN) + scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0)); + ASSERT_TRUE(browser->TerminateSession()); +#elif defined(OS_POSIX) + // There's nothing to do here if the browser is not running. + if (IsBrowserRunning()) { + automation()->SetFilteredInet(false); + + int window_count = 0; + EXPECT_TRUE(automation()->GetBrowserWindowCount(&window_count)); + + // Now, drop the automation IPC channel so that the automation provider in + // the browser notices and drops its reference to the browser process. + automation()->Disconnect(); + + EXPECT_EQ(kill(process_, SIGTERM), 0); + + // Wait for the browser process to quit. It should have quit when it got + // SIGTERM. + int timeout = kWaitForTerminateMsec; +#ifdef WAIT_FOR_DEBUGGER_ON_OPEN + timeout = 500000; +#endif + if (!base::WaitForSingleProcess(process_, timeout)) { + // We need to force the browser to quit because it didn't quit fast + // enough. Take no chance and kill every chrome processes. + CleanupAppProcesses(); + } + + // Don't forget to close the handle + base::CloseProcessHandle(process_); + process_ = NULL; + } +#endif // OS_POSIX + } + }; class VisibleBrowserTest : public UITest { @@ -36,14 +78,18 @@ class VisibleBrowserTest : public UITest { #if defined(OS_WIN) // The browser should quit quickly if it receives a WM_ENDSESSION message. TEST_F(BrowserTest, WindowsSessionEnd) { +#elif defined(OS_POSIX) +// The browser should quit gracefully and quickly if it receives a SIGTERM. +TEST_F(BrowserTest, PosixSessionEnd) { +#endif +#if defined(OS_WIN) || defined(OS_POSIX) FilePath test_file(test_data_directory_); test_file = test_file.AppendASCII("title1.html"); NavigateToURL(net::FilePathToFileURL(test_file)); PlatformThread::Sleep(action_timeout_ms()); - scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0)); - ASSERT_TRUE(browser->TerminateSession()); + TerminateBrowser(); PlatformThread::Sleep(action_timeout_ms()); ASSERT_FALSE(IsBrowserRunning()); @@ -69,7 +115,7 @@ TEST_F(BrowserTest, WindowsSessionEnd) { &exited_cleanly)); ASSERT_TRUE(exited_cleanly); } -#endif +#endif // OS_WIN || OS_POSIX // Test that scripts can fork a new renderer process for a tab in a particular // case (which matches following a link in Gmail). The script must open a new diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc index eac4367..30ed274 100644 --- a/chrome/test/ui/ui_test.cc +++ b/chrome/test/ui/ui_test.cc @@ -613,8 +613,6 @@ int UITest::GetBrowserProcessCount() { return GetRunningChromeProcesses(data_dir).size(); } -#if defined(OS_WIN) -// TODO(port): Port GetRunningChromeProcesses and sort out one w/string issue. static DictionaryValue* LoadDictionaryValueFromPath(const FilePath& path) { if (path.empty()) return NULL; @@ -634,13 +632,12 @@ DictionaryValue* UITest::GetLocalState() { } DictionaryValue* UITest::GetDefaultProfilePreferences() { - std::wstring path; - PathService::Get(chrome::DIR_USER_DATA, &path); - file_util::AppendToPath(&path, chrome::kNotSignedInProfile); - file_util::AppendToPath(&path, chrome::kPreferencesFilename); - return LoadDictionaryValueFromPath(FilePath::FromWStringHack(path)); + std::wstring path_wstring; + PathService::Get(chrome::DIR_USER_DATA, &path_wstring); + file_util::AppendToPath(&path_wstring, chrome::kNotSignedInProfile); + FilePath path(FilePath::FromWStringHack(path_wstring)); + return LoadDictionaryValueFromPath(path.Append(chrome::kPreferencesFilename)); } -#endif // OS_WIN int UITest::GetTabCount() { scoped_refptr<BrowserProxy> first_window(automation()->GetBrowserWindow(0)); |