diff options
author | gab@chromium.org <gab@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-22 10:10:46 +0000 |
---|---|---|
committer | gab@chromium.org <gab@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-22 10:10:46 +0000 |
commit | 612d3bf582e66cc7a7f8a16bd39f30a20fb37b72 (patch) | |
tree | b2934d006cb0b0c99e4e90aceaf3c604db96d1f5 /chrome | |
parent | dd98fbe822b8a5039ac5c2c293ae870080484519 (diff) | |
download | chromium_src-612d3bf582e66cc7a7f8a16bd39f30a20fb37b72.zip chromium_src-612d3bf582e66cc7a7f8a16bd39f30a20fb37b72.tar.gz chromium_src-612d3bf582e66cc7a7f8a16bd39f30a20fb37b72.tar.bz2 |
In WinAura, also kill the Metro viewer process in AttemptExit().
This results in an IPC channel error in the browser process which releases Ash's reference to g_browser_process, this is necessary for the browser process to terminate.
This is a precursor to Ash browser_tests (otherwise when browser_tests try to exit Chrome by closing all browser windows, Ash stays up and the test never ends...!).
BUG=235648, 232842, 179830
TEST=
A) Local hack of Ash browser_tests works!!
B) Exit Chrome from the wrench menu on the native Desktop while Ash is running and make sure that the viewer is killed (whether it was sleeping or not) which subsequently results in the browser process going away with a clean exit :).
C) Exit Chrome from the wrench menu on the native Desktop when Ash is NOT running and make sure it closes fine (as it did before this CL).
Review URL: https://chromiumcodereview.appspot.com/14576015
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@201476 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
14 files changed, 150 insertions, 24 deletions
diff --git a/chrome/browser/browser_process_platform_part.h b/chrome/browser/browser_process_platform_part.h index 1e0574b..7abe825 100644 --- a/chrome/browser/browser_process_platform_part.h +++ b/chrome/browser/browser_process_platform_part.h @@ -8,8 +8,12 @@ #include "build/build_config.h" // Include the appropriate BrowserProcessPlatformPart based on the platform. -#if defined(OS_CHROMEOS) +#if defined(OS_ANDROID) +#include "chrome/browser/browser_process_platform_part_android.h" +#elif defined(OS_CHROMEOS) #include "chrome/browser/browser_process_platform_part_chromeos.h" +#elif defined(OS_MACOSX) && !defined(OS_IOS) +#include "chrome/browser/browser_process_platform_part_mac.h" #elif defined(OS_WIN) && defined(USE_AURA) #include "chrome/browser/browser_process_platform_part_aurawin.h" #else diff --git a/chrome/browser/browser_process_platform_part_android.cc b/chrome/browser/browser_process_platform_part_android.cc new file mode 100644 index 0000000..4fdede0 --- /dev/null +++ b/chrome/browser/browser_process_platform_part_android.cc @@ -0,0 +1,17 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/browser_process_platform_part_mac.h" +#include "chrome/browser/lifetime/application_lifetime_android.h" + +BrowserProcessPlatformPart::BrowserProcessPlatformPart() { +} + +BrowserProcessPlatformPart::~BrowserProcessPlatformPart() { +} + +void BrowserProcessPlatformPart::AttemptExit() { + // Tell the Java code to finish() the Activity. + chrome::TerminateAndroid(); +} diff --git a/chrome/browser/browser_process_platform_part_android.h b/chrome/browser/browser_process_platform_part_android.h new file mode 100644 index 0000000..d9717f4 --- /dev/null +++ b/chrome/browser/browser_process_platform_part_android.h @@ -0,0 +1,23 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_ANDROID_H_ +#define CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_ANDROID_H_ + +#include "base/compiler_specific.h" +#include "chrome/browser/browser_process_platform_part_base.h" + +class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase { + public: + BrowserProcessPlatformPart(); + virtual ~BrowserProcessPlatformPart(); + + // Overridden from BrowserProcessPlatformPartBase: + virtual void AttemptExit() OVERRIDE; + + private: + DISALLOW_COPY_AND_ASSIGN(BrowserProcessPlatformPart); +}; + +#endif // CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_ANDROID_H_ diff --git a/chrome/browser/browser_process_platform_part_aurawin.cc b/chrome/browser/browser_process_platform_part_aurawin.cc index aaca529..f1730f9 100644 --- a/chrome/browser/browser_process_platform_part_aurawin.cc +++ b/chrome/browser/browser_process_platform_part_aurawin.cc @@ -5,6 +5,10 @@ #include "chrome/browser/browser_process_platform_part_aurawin.h" #include "base/command_line.h" +#include "base/logging.h" +#include "base/process_util.h" +#include "base/win/windows_version.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/metro_viewer/metro_viewer_process_host_win.h" #include "chrome/common/chrome_switches.h" @@ -20,7 +24,8 @@ void BrowserProcessPlatformPart::OnMetroViewerProcessTerminated() { void BrowserProcessPlatformPart::PlatformSpecificCommandLineProcessing( const CommandLine& command_line) { - if (command_line.HasSwitch(switches::kViewerConnection) && + if (base::win::GetVersion() >= base::win::VERSION_WIN8 && + command_line.HasSwitch(switches::kViewerConnection) && !metro_viewer_process_host_.get()) { // Tell the metro viewer process host to connect to the given IPC channel. metro_viewer_process_host_.reset( @@ -28,3 +33,24 @@ void BrowserProcessPlatformPart::PlatformSpecificCommandLineProcessing( command_line.GetSwitchValueASCII(switches::kViewerConnection))); } } + +void BrowserProcessPlatformPart::AttemptExit() { + // On WinAura, the regular exit path is fine except on Win8+, where Ash might + // be active in Metro and won't go away even if all browsers are closed. The + // viewer process, whose host holds a reference to g_browser_process, needs to + // be killed as well. + BrowserProcessPlatformPartBase::AttemptExit(); + + if (base::win::GetVersion() >= base::win::VERSION_WIN8 && + metro_viewer_process_host_) { + base::ProcessId viewer_id = + metro_viewer_process_host_->GetViewerProcessId(); + if (viewer_id == base::kNullProcessId) + return; + // The viewer doesn't hold any state so it is fine to kill it before it + // cleanly exits. This will trigger MetroViewerProcessHost::OnChannelError() + // which will cleanup references to g_browser_process. + bool success = base::KillProcessById(viewer_id, 0, true); + DCHECK(success); + } +} diff --git a/chrome/browser/browser_process_platform_part_aurawin.h b/chrome/browser/browser_process_platform_part_aurawin.h index a227abd..b1b74a0 100644 --- a/chrome/browser/browser_process_platform_part_aurawin.h +++ b/chrome/browser/browser_process_platform_part_aurawin.h @@ -5,7 +5,7 @@ #ifndef CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_AURAWIN_H_ #define CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_AURAWIN_H_ -#include "base/basictypes.h" +#include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" #include "chrome/browser/browser_process_platform_part_base.h" @@ -22,6 +22,7 @@ class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase { // Overridden from BrowserProcessPlatformPartBase: virtual void PlatformSpecificCommandLineProcessing( const CommandLine& command_line) OVERRIDE; + virtual void AttemptExit() OVERRIDE; private: // Hosts the channel for the Windows 8 metro viewer process which runs in diff --git a/chrome/browser/browser_process_platform_part_base.cc b/chrome/browser/browser_process_platform_part_base.cc index c647f49..1da8917 100644 --- a/chrome/browser/browser_process_platform_part_base.cc +++ b/chrome/browser/browser_process_platform_part_base.cc @@ -2,7 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/logging.h" #include "chrome/browser/browser_process_platform_part_base.h" +#include "chrome/browser/lifetime/application_lifetime.h" BrowserProcessPlatformPartBase::BrowserProcessPlatformPartBase() { } @@ -16,3 +18,14 @@ void BrowserProcessPlatformPartBase::PlatformSpecificCommandLineProcessing( void BrowserProcessPlatformPartBase::StartTearDown() { } + +void BrowserProcessPlatformPartBase::AttemptExit() { +// chrome::CloseAllBrowsers() doesn't link on OS_IOS and OS_ANDROID, but +// OS_ANDROID overrides this method already and OS_IOS never calls this. +#if defined(OS_IOS) || defined(OS_ANDROID) + NOTREACHED(); +#else + // On most platforms, closing all windows causes the application to exit. + chrome::CloseAllBrowsers(); +#endif +} diff --git a/chrome/browser/browser_process_platform_part_base.h b/chrome/browser/browser_process_platform_part_base.h index f63a441..1494abc 100644 --- a/chrome/browser/browser_process_platform_part_base.h +++ b/chrome/browser/browser_process_platform_part_base.h @@ -24,6 +24,9 @@ class BrowserProcessPlatformPartBase { // Called from BrowserProcessImpl::StartTearDown(). virtual void StartTearDown(); + // Called from AttemptExitInternal(). + virtual void AttemptExit(); + private: DISALLOW_COPY_AND_ASSIGN(BrowserProcessPlatformPartBase); }; diff --git a/chrome/browser/browser_process_platform_part_chromeos.h b/chrome/browser/browser_process_platform_part_chromeos.h index b5a8bdd..3eb1236 100644 --- a/chrome/browser/browser_process_platform_part_chromeos.h +++ b/chrome/browser/browser_process_platform_part_chromeos.h @@ -5,7 +5,7 @@ #ifndef CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_CHROMEOS_H_ #define CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_CHROMEOS_H_ -#include "base/basictypes.h" +#include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" #include "base/threading/non_thread_safe.h" #include "chrome/browser/browser_process_platform_part_base.h" diff --git a/chrome/browser/browser_process_platform_part_mac.h b/chrome/browser/browser_process_platform_part_mac.h new file mode 100644 index 0000000..2077fef --- /dev/null +++ b/chrome/browser/browser_process_platform_part_mac.h @@ -0,0 +1,23 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_MAC_H_ +#define CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_MAC_H_ + +#include "base/compiler_specific.h" +#include "chrome/browser/browser_process_platform_part_base.h" + +class BrowserProcessPlatformPart : public BrowserProcessPlatformPartBase { + public: + BrowserProcessPlatformPart(); + virtual ~BrowserProcessPlatformPart(); + + // Overridden from BrowserProcessPlatformPartBase: + virtual void AttemptExit() OVERRIDE; + + private: + DISALLOW_COPY_AND_ASSIGN(BrowserProcessPlatformPart); +}; + +#endif // CHROME_BROWSER_BROWSER_PROCESS_PLATFORM_PART_MAC_H_ diff --git a/chrome/browser/browser_process_platform_part_mac.mm b/chrome/browser/browser_process_platform_part_mac.mm new file mode 100644 index 0000000..a279de6 --- /dev/null +++ b/chrome/browser/browser_process_platform_part_mac.mm @@ -0,0 +1,19 @@ +// Copyright (c) 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/browser_process_platform_part_mac.h" +#include "chrome/browser/chrome_browser_application_mac.h" + +BrowserProcessPlatformPart::BrowserProcessPlatformPart() { +} + +BrowserProcessPlatformPart::~BrowserProcessPlatformPart() { +} + +void BrowserProcessPlatformPart::AttemptExit() { + // On the Mac, the application continues to run once all windows are closed. + // Terminate will result in a CloseAllBrowsers() call, and once (and if) + // that is done, will cause the application to exit cleanly. + chrome_browser_application_mac::Terminate(); +} diff --git a/chrome/browser/lifetime/application_lifetime.cc b/chrome/browser/lifetime/application_lifetime.cc index fdb60e7..87109c5 100644 --- a/chrome/browser/lifetime/application_lifetime.cc +++ b/chrome/browser/lifetime/application_lifetime.cc @@ -12,6 +12,7 @@ #include "base/prefs/pref_service.h" #include "build/build_config.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/browser_process_platform_part.h" #include "chrome/browser/browser_shutdown.h" #include "chrome/browser/download/download_service.h" #include "chrome/browser/metrics/thread_watcher.h" @@ -31,14 +32,6 @@ #include "content/public/browser/navigation_details.h" #include "content/public/browser/notification_service.h" -#if defined(OS_ANDROID) -#include "chrome/browser/lifetime/application_lifetime_android.h" -#endif - -#if defined(OS_MACOSX) -#include "chrome/browser/chrome_browser_application_mac.h" -#endif - #if defined(OS_CHROMEOS) #include "base/chromeos/chromeos_version.h" #include "chrome/browser/chromeos/boot_times_loader.h" @@ -92,18 +85,7 @@ void AttemptExitInternal() { content::NotificationService::AllSources(), content::NotificationService::NoDetails()); -#if defined(OS_ANDROID) - // Tell the Java code to finish() the Activity. - TerminateAndroid(); -#elif defined(OS_MACOSX) - // On the Mac, the application continues to run once all windows are closed. - // Terminate will result in a CloseAllBrowsers() call, and once (and if) - // that is done, will cause the application to exit cleanly. - chrome_browser_application_mac::Terminate(); -#else - // On most platforms, closing all windows causes the application to exit. - CloseAllBrowsers(); -#endif + g_browser_process->platform_part()->AttemptExit(); } void CloseAllBrowsers() { diff --git a/chrome/browser/metro_viewer/metro_viewer_process_host_win.cc b/chrome/browser/metro_viewer/metro_viewer_process_host_win.cc index ad279b1..b2c2d21 100644 --- a/chrome/browser/metro_viewer/metro_viewer_process_host_win.cc +++ b/chrome/browser/metro_viewer/metro_viewer_process_host_win.cc @@ -57,6 +57,12 @@ MetroViewerProcessHost::MetroViewerProcessHost( MetroViewerProcessHost::~MetroViewerProcessHost() { } +base::ProcessId MetroViewerProcessHost::GetViewerProcessId() { + if (channel_) + return channel_->peer_pid(); + return base::kNullProcessId; +} + bool MetroViewerProcessHost::Send(IPC::Message* msg) { return channel_->Send(msg); } diff --git a/chrome/browser/metro_viewer/metro_viewer_process_host_win.h b/chrome/browser/metro_viewer/metro_viewer_process_host_win.h index 1527d52..765877b 100644 --- a/chrome/browser/metro_viewer/metro_viewer_process_host_win.h +++ b/chrome/browser/metro_viewer/metro_viewer_process_host_win.h @@ -8,6 +8,7 @@ #include <string> #include "base/memory/scoped_ptr.h" +#include "base/process_util.h" #include "base/threading/non_thread_safe.h" #include "ipc/ipc_listener.h" #include "ipc/ipc_sender.h" @@ -24,6 +25,10 @@ class MetroViewerProcessHost : public IPC::Listener, explicit MetroViewerProcessHost(const std::string& ipc_channel_name); virtual ~MetroViewerProcessHost(); + // Returns the process id of the viewer process if one is connected to this + // host, returns base::kNullProcessId otherwise. + base::ProcessId GetViewerProcessId(); + private: // IPC::Sender implementation: virtual bool Send(IPC::Message* msg) OVERRIDE; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 5918e27..85fef34 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -278,12 +278,16 @@ 'browser/browser_process_impl.cc', 'browser/browser_process_impl.h', 'browser/browser_process_platform_part.h', + 'browser/browser_process_platform_part_android.cc', + 'browser/browser_process_platform_part_android.h', 'browser/browser_process_platform_part_aurawin.cc', 'browser/browser_process_platform_part_aurawin.h', 'browser/browser_process_platform_part_base.cc', 'browser/browser_process_platform_part_base.h', 'browser/browser_process_platform_part_chromeos.cc', 'browser/browser_process_platform_part_chromeos.h', + 'browser/browser_process_platform_part_mac.h', + 'browser/browser_process_platform_part_mac.mm', 'browser/browser_shutdown.cc', 'browser/browser_shutdown.h', 'browser/browser_util_win.cc', |