From 952928dc79c2f204ef905d691ee0898bbfc357e3 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Wed, 3 Jan 2018 18:59:12 +0900 Subject: [PATCH] Singleton must be created on request The creation of singleton relies on the `userData` dir, which can be changed by user, we have to ensure singleton uses the `userData` dir set by user. --- atom/browser/api/atom_api_app.cc | 35 ++++++++++++------- atom/browser/api/atom_api_app.h | 4 +-- atom/browser/atom_browser_main_parts.cc | 19 ++-------- atom/browser/atom_browser_main_parts.h | 4 --- .../chrome/browser/process_singleton.h | 8 ++--- .../chrome/browser/process_singleton_posix.cc | 10 ++++-- .../chrome/browser/process_singleton_win.cc | 7 ++-- 7 files changed, 41 insertions(+), 46 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 81227239d0b4..1dd9a5d82ea2 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -565,6 +565,11 @@ void App::OnWindowAllClosed() { void App::OnQuit() { int exitCode = AtomBrowserMainParts::Get()->GetExitCode(); Emit("quit", exitCode); + + if (process_singleton_) { + process_singleton_->Cleanup(); + process_singleton_.reset(); + } } void App::OnOpenFile(bool* prevent_default, const std::string& file_path) { @@ -592,12 +597,15 @@ void App::OnFinishLaunching(const base::DictionaryValue& launch_info) { Emit("ready", launch_info); } -void App::OnAccessibilitySupportChanged() { - Emit("accessibility-support-changed", IsAccessibilitySupportEnabled()); -} - void App::OnPreMainMessageLoopRun() { content::BrowserChildProcessObserver::Add(this); + if (process_singleton_) { + process_singleton_->OnBrowserReady(); + } +} + +void App::OnAccessibilitySupportChanged() { + Emit("accessibility-support-changed", IsAccessibilitySupportEnabled()); } #if defined(OS_MACOSX) @@ -848,18 +856,19 @@ std::string App::GetLocale() { bool App::MakeSingleInstance( const ProcessSingleton::NotificationCallback& callback) { - auto process_singleton = AtomBrowserMainParts::Get()->process_singleton(); - if (process_singleton_created_) + if (process_singleton_) return false; - process_singleton->RegisterSingletonNotificationCallback( - base::Bind(NotificationCallbackWrapper, callback)); + base::FilePath user_dir; + PathService::Get(brightray::DIR_USER_DATA, &user_dir); + process_singleton_.reset(new ProcessSingleton( + user_dir, base::Bind(NotificationCallbackWrapper, callback))); - switch (process_singleton->NotifyOtherProcessOrCreate()) { + switch (process_singleton_->NotifyOtherProcessOrCreate()) { case ProcessSingleton::NotifyResult::LOCK_ERROR: case ProcessSingleton::NotifyResult::PROFILE_IN_USE: case ProcessSingleton::NotifyResult::PROCESS_NOTIFIED: { - process_singleton_created_ = true; + process_singleton_.reset(); return true; } case ProcessSingleton::NotifyResult::PROCESS_NONE: @@ -869,9 +878,9 @@ bool App::MakeSingleInstance( } void App::ReleaseSingleInstance() { - auto process_singleton = AtomBrowserMainParts::Get()->process_singleton(); - if (process_singleton) { - process_singleton->Cleanup(); + if (process_singleton_) { + process_singleton_->Cleanup(); + process_singleton_.reset(); } } diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index 559c4ccde45d..5226664bfd66 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -218,6 +218,8 @@ class App : public AtomBrowserClient::Delegate, JumpListResult SetJumpList(v8::Local val, mate::Arguments* args); #endif // defined(OS_WIN) + std::unique_ptr process_singleton_; + #if defined(USE_NSS_CERTS) std::unique_ptr certificate_manager_model_; #endif @@ -232,8 +234,6 @@ class App : public AtomBrowserClient::Delegate, std::unique_ptr>; ProcessMetricMap app_metrics_; - bool process_singleton_created_ = false; - DISALLOW_COPY_AND_ASSIGN(App); }; diff --git a/atom/browser/atom_browser_main_parts.cc b/atom/browser/atom_browser_main_parts.cc index 4595a9462023..07ddfca689bd 100644 --- a/atom/browser/atom_browser_main_parts.cc +++ b/atom/browser/atom_browser_main_parts.cc @@ -128,10 +128,6 @@ void AtomBrowserMainParts::PostEarlyInitialization() { bridge_task_runner_ = new BridgeTaskRunner; base::ThreadTaskRunnerHandle handle(bridge_task_runner_); - base::FilePath user_dir; - PathService::Get(brightray::DIR_USER_DATA, &user_dir); - process_singleton_.reset(new ProcessSingleton(user_dir)); - // The ProxyResolverV8 has setup a complete V8 environment, in order to // avoid conflicts we only initialize our V8 environment after that. js_env_.reset(new JavascriptEnvironment); @@ -188,9 +184,6 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() { bridge_task_runner_->MessageLoopIsReady(); bridge_task_runner_ = nullptr; - // Notify observers that main thread message loop was initialized. - Browser::Get()->PreMainMessageLoopRun(); - #if defined(USE_X11) libgtkui::GtkInitFromCommandLine(*base::CommandLine::ForCurrentProcess()); #endif @@ -202,8 +195,8 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() { Browser::Get()->DidFinishLaunching(*empty_info); #endif - if (process_singleton_) - process_singleton_->OnBrowserReady(); + // Notify observers that main thread message loop was initialized. + Browser::Get()->PreMainMessageLoopRun(); } bool AtomBrowserMainParts::MainMessageLoopRun(int* result_code) { @@ -241,12 +234,4 @@ void AtomBrowserMainParts::PostMainMessageLoopRun() { } } -void AtomBrowserMainParts::PostDestroyThreads() { - brightray::BrowserMainParts::PostDestroyThreads(); - if (process_singleton_) { - process_singleton_->Cleanup(); - process_singleton_.reset(); - } -} - } // namespace atom diff --git a/atom/browser/atom_browser_main_parts.h b/atom/browser/atom_browser_main_parts.h index e9f453473a25..a4c3bbed5619 100644 --- a/atom/browser/atom_browser_main_parts.h +++ b/atom/browser/atom_browser_main_parts.h @@ -11,7 +11,6 @@ #include "base/callback.h" #include "base/timer/timer.h" #include "brightray/browser/browser_main_parts.h" -#include "chrome/browser/process_singleton.h" #include "content/public/browser/browser_context.h" class BrowserProcess; @@ -45,7 +44,6 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts { base::Closure RegisterDestructionCallback(const base::Closure& callback); Browser* browser() { return browser_.get(); } - ProcessSingleton* process_singleton() { return process_singleton_.get(); } protected: // content::BrowserMainParts: @@ -59,7 +57,6 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts { #if defined(OS_MACOSX) void PreMainMessageLoopStart() override; #endif - void PostDestroyThreads() override; private: #if defined(OS_POSIX) @@ -88,7 +85,6 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts { std::unique_ptr atom_bindings_; std::unique_ptr node_env_; std::unique_ptr node_debugger_; - std::unique_ptr process_singleton_; base::Timer gc_timer_; diff --git a/chromium_src/chrome/browser/process_singleton.h b/chromium_src/chrome/browser/process_singleton.h index 4a61f770b922..3ee6ca8140fc 100644 --- a/chromium_src/chrome/browser/process_singleton.h +++ b/chromium_src/chrome/browser/process_singleton.h @@ -62,7 +62,8 @@ class ProcessSingleton { base::Callback; - explicit ProcessSingleton(const base::FilePath& user_data_dir); + ProcessSingleton(const base::FilePath& user_data_dir, + const NotificationCallback& notification_callback); ~ProcessSingleton(); // Notify another process, if available. Otherwise sets ourselves as the @@ -98,11 +99,6 @@ class ProcessSingleton { const ShouldKillRemoteProcessCallback& display_dialog_callback); #endif - void RegisterSingletonNotificationCallback( - const NotificationCallback& callback) { - notification_callback_ = callback; - } - protected: // Notify another process, if available. // Returns true if another process was found and notified, false if we should diff --git a/chromium_src/chrome/browser/process_singleton_posix.cc b/chromium_src/chrome/browser/process_singleton_posix.cc index da81098ccdec..eff93fea32d6 100644 --- a/chromium_src/chrome/browser/process_singleton_posix.cc +++ b/chromium_src/chrome/browser/process_singleton_posix.cc @@ -85,6 +85,7 @@ #include "base/strings/sys_string_conversions.h" #include "base/strings/utf_string_conversions.h" #include "base/threading/platform_thread.h" +#include "base/threading/thread_restrictions.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" #include "base/timer/timer.h" @@ -716,9 +717,13 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK( /////////////////////////////////////////////////////////////////////////////// // ProcessSingleton // -ProcessSingleton::ProcessSingleton(const base::FilePath& user_data_dir) - : current_pid_(base::GetCurrentProcId()) { +ProcessSingleton::ProcessSingleton( + const base::FilePath& user_data_dir, + const NotificationCallback& notification_callback) + : notification_callback_(notification_callback), + current_pid_(base::GetCurrentProcId()) { // The user_data_dir may have not been created yet. + base::ThreadRestrictions::ScopedAllowIO allow_io; base::CreateDirectoryAndGetError(user_data_dir, nullptr); socket_path_ = user_data_dir.Append(kSingletonSocketFilename); @@ -957,6 +962,7 @@ void ProcessSingleton::DisablePromptForTesting() { } bool ProcessSingleton::Create() { + base::ThreadRestrictions::ScopedAllowIO allow_io; int sock; sockaddr_un addr; diff --git a/chromium_src/chrome/browser/process_singleton_win.cc b/chromium_src/chrome/browser/process_singleton_win.cc index ec82f4929592..6629a6ff3b1c 100644 --- a/chromium_src/chrome/browser/process_singleton_win.cc +++ b/chromium_src/chrome/browser/process_singleton_win.cc @@ -182,8 +182,11 @@ bool TerminateAppWithError() { } // namespace -ProcessSingleton::ProcessSingleton(const base::FilePath& user_data_dir) - : is_virtualized_(false), +ProcessSingleton::ProcessSingleton( + const base::FilePath& user_data_dir, + const NotificationCallback& notification_callback) + : notification_callback_(notification_callback), + is_virtualized_(false), lock_file_(INVALID_HANDLE_VALUE), user_data_dir_(user_data_dir), should_kill_remote_process_callback_(base::Bind(&TerminateAppWithError)) {