Extract locking behaviour from ProcessSingleton.

This refactoring continues the division of the behaviour of ProcessSingleton into two parts:
* The protocol for establishing a server process and communicating between the client and server.
* How the server processes command-line invocations.

Very small behavioural change:
* If an error occurs while parsing the command-line received via COPY_DATA, the modal dialog (if any) is no longer flashed and raised to foreground.

The motivation for this change is that I wish to introduce some more sophisticated behaviour when queuing messages during startup. See the follow-up CL (in-progress) at https://codereview.chromium.org/12674028/ .

BUG=170726,170734,225693

Review URL: https://chromiumcodereview.appspot.com/12096114

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@195264 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chrome_browser_main.cc

@@ -747,12 +747,8 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() {
   bool is_first_run = false;
   // Android's first run is done in Java instead of native.
 #if !defined(OS_ANDROID)
-
-  process_singleton_.reset(new ProcessSingleton(
+  process_singleton_.reset(new ChromeProcessSingleton(
       user_data_dir_, base::Bind(&ProcessSingletonNotificationCallback)));
-  // Ensure ProcessSingleton won't process messages too early. It will be
-  // unlocked in PostBrowserStart().
-  process_singleton_->Lock(NULL);
 
   bool force_first_run =
       parsed_command_line().HasSwitch(switches::kForceFirstRun);
@@ -1179,8 +1175,10 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
     // sucessfully grabbed above.
     int try_chrome_int;
     base::StringToInt(try_chrome, &try_chrome_int);
-    TryChromeDialogView::Result answer =
-        TryChromeDialogView::Show(try_chrome_int, process_singleton_.get());
+    TryChromeDialogView::Result answer = TryChromeDialogView::Show(
+        try_chrome_int,
+        base::Bind(&ChromeProcessSingleton::SetActiveModalDialog,
+                   base::Unretained(process_singleton_.get())));
     if (answer == TryChromeDialogView::NOT_NOW)
       return chrome::RESULT_CODE_NORMAL_EXIT_CANCEL;
     if (answer == TryChromeDialogView::UNINSTALL_CHROME)
@@ -1282,8 +1280,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
     first_run::AutoImport(profile_,
                           master_prefs_->homepage_defined,
                           master_prefs_->do_import_items,
-                          master_prefs_->dont_import_items,
-                          process_singleton_.get());
+                          master_prefs_->dont_import_items);
     // Note: this can pop the first run consent dialog on linux.
     first_run::DoPostImportTasks(profile_, master_prefs_->make_chrome_default);
 

chrome/browser/chrome_browser_main.h

@@ -10,6 +10,7 @@
 #include "base/metrics/field_trial.h"
 #include "base/tracked_objects.h"
 #include "chrome/browser/chrome_browser_field_trials.h"
+#include "chrome/browser/chrome_process_singleton.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/process_singleton.h"
 #include "chrome/browser/task_profiler/auto_tracking.h"
@@ -170,7 +171,7 @@ class ChromeBrowserMainParts : public content::BrowserMainParts {
 
   // Android doesn't support multiple browser processes, so it doesn't implement
   // ProcessSingleton.
-  scoped_ptr<ProcessSingleton> process_singleton_;
+  scoped_ptr<ChromeProcessSingleton> process_singleton_;
 #endif
   scoped_ptr<first_run::MasterPrefs> master_prefs_;
   bool record_search_engine_;

chrome/browser/chrome_process_singleton.cc

+// 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/chrome_process_singleton.h"
+
+ChromeProcessSingleton::ChromeProcessSingleton(
+    const base::FilePath& user_data_dir,
+    const ProcessSingleton::NotificationCallback& notification_callback)
+    : startup_lock_(notification_callback),
+      modal_dialog_lock_(startup_lock_.AsNotificationCallback()),
+      process_singleton_(user_data_dir,
+                         modal_dialog_lock_.AsNotificationCallback()) {
+}
+
+
+ChromeProcessSingleton::ChromeProcessSingleton(
+      const base::FilePath& user_data_dir,
+      const ProcessSingleton::NotificationCallback& notification_callback,
+      const ProcessSingletonModalDialogLock::SetForegroundWindowHandler&
+          set_foreground_window_handler)
+    : startup_lock_(notification_callback),
+      modal_dialog_lock_(startup_lock_.AsNotificationCallback(),
+                         set_foreground_window_handler),
+      process_singleton_(user_data_dir,
+                         modal_dialog_lock_.AsNotificationCallback()) {
+}
+
+ChromeProcessSingleton::~ChromeProcessSingleton() {
+}
+
+ProcessSingleton::NotifyResult
+    ChromeProcessSingleton::NotifyOtherProcessOrCreate() {
+  return process_singleton_.NotifyOtherProcessOrCreate();
+}
+
+void ChromeProcessSingleton::Cleanup() {
+  process_singleton_.Cleanup();
+}
+
+void ChromeProcessSingleton::SetActiveModalDialog(
+    gfx::NativeWindow active_dialog) {
+  modal_dialog_lock_.SetActiveModalDialog(active_dialog);
+}
+
+void ChromeProcessSingleton::Unlock() {
+  startup_lock_.Unlock();
+}

chrome/browser/chrome_process_singleton.h

+// 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_CHROME_PROCESS_SINGLETON_H_
+#define CHROME_BROWSER_CHROME_PROCESS_SINGLETON_H_
+
+#include "base/basictypes.h"
+#include "base/files/file_path.h"
+#include "chrome/browser/process_singleton.h"
+#include "chrome/browser/process_singleton_modal_dialog_lock.h"
+#include "chrome/browser/process_singleton_startup_lock.h"
+#include "ui/gfx/native_widget_types.h"
+
+// Composes a basic ProcessSingleton with ProcessSingletonStartupLock and
+// ProcessSingletonModalDialogLock.
+//
+// Notifications from ProcessSingleton will be discarded if a modal dialog is
+// active. Otherwise, until |Unlock()| is called, they will be queued up.
+// Once unlocked, notifications will be passed to the client-supplied
+// NotificationCallback.
+//
+// The client must ensure that SetActiveModalDialog is called appropriately when
+// dialogs are displayed or dismissed during startup. While a dialog is active:
+// 1. Neither this process nor the invoking process will handle the command
+//    line.
+// 2. The active dialog is brought to the foreground and/or the taskbar icon
+//    flashed (using ::SetForegroundWindow on Windows).
+class ChromeProcessSingleton {
+ public:
+  ChromeProcessSingleton(
+      const base::FilePath& user_data_dir,
+      const ProcessSingleton::NotificationCallback& notification_callback);
+
+  ChromeProcessSingleton(
+      const base::FilePath& user_data_dir,
+      const ProcessSingleton::NotificationCallback& notification_callback,
+      const ProcessSingletonModalDialogLock::SetForegroundWindowHandler&
+          set_foreground_window_handler);
+
+  ~ChromeProcessSingleton();
+
+  // Notify another process, if available. Otherwise sets ourselves as the
+  // singleton instance. Returns PROCESS_NONE if we became the singleton
+  // instance. Callers are guaranteed to either have notified an existing
+  // process or have grabbed the singleton (unless the profile is locked by an
+  // unreachable process).
+  ProcessSingleton::NotifyResult NotifyOtherProcessOrCreate();
+
+  // Clear any lock state during shutdown.
+  void Cleanup();
+
+  // Receives a handle to the active modal dialog, or NULL if the active dialog
+  // is dismissed.
+  void SetActiveModalDialog(gfx::NativeWindow active_dialog);
+
+  // Executes previously queued command-line invocations and allows future
+  // invocations to be executed immediately.
+  // This only has an effect the first time it is called.
+  void Unlock();
+
+ private:
+  // We compose these two locks with the client-supplied notification callback.
+  // First |modal_dialog_lock_| will discard any notifications that arrive while
+  // a modal dialog is active. Otherwise, it will pass the notification to
+  // |startup_lock_|, which will queue notifications until |Unlock()| is called.
+  // Notifications passing through both locks are finally delivered to our
+  // client.
+  ProcessSingletonStartupLock startup_lock_;
+  ProcessSingletonModalDialogLock modal_dialog_lock_;
+
+  // The basic ProcessSingleton
+  ProcessSingleton process_singleton_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeProcessSingleton);
+};
+
+#endif  // CHROME_BROWSER_CHROME_PROCESS_SINGLETON_H_

chrome/browser/process_singleton_win_unittest.cc → chrome/browser/chrome_process_singleton_win_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/process_singleton.h"
+#include "chrome/browser/chrome_process_singleton.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ -28,16 +28,19 @@ bool ClientCallback(const CommandLine& command_line,
 
 }  // namespace
 
-TEST(ProcessSingletonWinTest, Basic) {
+TEST(ChromeProcessSingletonTest, Basic) {
   base::ScopedTempDir profile_dir;
   ASSERT_TRUE(profile_dir.CreateUniqueTempDir());
 
   int callback_count = 0;
 
-  ProcessSingleton ps1(
+  ChromeProcessSingleton ps1(
       profile_dir.path(),
       base::Bind(&ServerCallback, base::Unretained(&callback_count)));
-  ProcessSingleton ps2(profile_dir.path(), base::Bind(&ClientCallback));
+  ps1.Unlock();
+
+  ChromeProcessSingleton ps2(profile_dir.path(), base::Bind(&ClientCallback));
+  ps2.Unlock();
 
   ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate();
 
@@ -50,19 +53,18 @@ TEST(ProcessSingletonWinTest, Basic) {
   ASSERT_EQ(1, callback_count);
 }
 
-#if !defined(USE_AURA)
-TEST(ProcessSingletonWinTest, Lock) {
+TEST(ChromeProcessSingletonTest, Lock) {
   base::ScopedTempDir profile_dir;
   ASSERT_TRUE(profile_dir.CreateUniqueTempDir());
 
   int callback_count = 0;
 
-  ProcessSingleton ps1(
+  ChromeProcessSingleton ps1(
       profile_dir.path(),
       base::Bind(&ServerCallback, base::Unretained(&callback_count)));
-  ps1.Lock(NULL);
 
-  ProcessSingleton ps2(profile_dir.path(), base::Bind(&ClientCallback));
+  ChromeProcessSingleton ps2(profile_dir.path(), base::Bind(&ClientCallback));
+  ps2.Unlock();
 
   ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate();
 
@@ -77,51 +79,53 @@ TEST(ProcessSingletonWinTest, Lock) {
   ASSERT_EQ(1, callback_count);
 }
 
-class TestableProcessSingleton : public ProcessSingleton {
- public:
-  TestableProcessSingleton(const base::FilePath& user_data_dir,
-                           const NotificationCallback& notification_callback)
-      : ProcessSingleton(user_data_dir, notification_callback),
-        called_set_foreground_window_(false) {}
-
-  bool called_set_foreground_window() { return called_set_foreground_window_; }
+#if defined(OS_WIN) && !defined(USE_AURA)
+namespace {
 
- protected:
-  virtual void DoSetForegroundWindow(HWND target_window) OVERRIDE {
-    called_set_foreground_window_ = true;
-  }
+void SetForegroundWindowHandler(bool* flag,
+                                gfx::NativeWindow /* target_window */) {
+  *flag = true;
+}
 
- private:
-  bool called_set_foreground_window_;
-};
+}  // namespace
 
-TEST(ProcessSingletonWinTest, LockWithModalDialog) {
+TEST(ChromeProcessSingletonTest, LockWithModalDialog) {
   base::ScopedTempDir profile_dir;
   ASSERT_TRUE(profile_dir.CreateUniqueTempDir());
 
   int callback_count = 0;
+  bool called_set_foreground_window = false;
 
-  TestableProcessSingleton ps1(
+  ChromeProcessSingleton ps1(
       profile_dir.path(),
-      base::Bind(&ServerCallback, base::Unretained(&callback_count)));
-  ps1.Lock(::GetShellWindow());
+      base::Bind(&ServerCallback, base::Unretained(&callback_count)),
+      base::Bind(&SetForegroundWindowHandler,
+                 base::Unretained(&called_set_foreground_window)));
+  ps1.SetActiveModalDialog(::GetShellWindow());
 
-  ProcessSingleton ps2(profile_dir.path(), base::Bind(&ClientCallback));
+  ChromeProcessSingleton ps2(profile_dir.path(), base::Bind(&ClientCallback));
+  ps2.Unlock();
 
   ProcessSingleton::NotifyResult result = ps1.NotifyOtherProcessOrCreate();
 
   ASSERT_EQ(ProcessSingleton::PROCESS_NONE, result);
   ASSERT_EQ(0, callback_count);
 
-  ASSERT_FALSE(ps1.called_set_foreground_window());
+  ASSERT_FALSE(called_set_foreground_window);
   result = ps2.NotifyOtherProcessOrCreate();
   ASSERT_EQ(ProcessSingleton::PROCESS_NOTIFIED, result);
-  ASSERT_TRUE(ps1.called_set_foreground_window());
+  ASSERT_TRUE(called_set_foreground_window);
 
   ASSERT_EQ(0, callback_count);
+  ps1.SetActiveModalDialog(NULL);
   ps1.Unlock();
-  // When a modal dialog is present, the new command-line invocation is silently
+  // The notification sent while a modal dialog was present was silently
   // dropped.
   ASSERT_EQ(0, callback_count);
+
+  // But now that the active modal dialog is NULL notifications will be handled.
+  result = ps2.NotifyOtherProcessOrCreate();
+  ASSERT_EQ(ProcessSingleton::PROCESS_NOTIFIED, result);
+  ASSERT_EQ(1, callback_count);
 }
-#endif  // !defined(USE_AURA)
+#endif  // defined(OS_WIN) && !defined(USE_AURA)

chrome/browser/first_run/first_run.cc

@@ -28,7 +28,6 @@
 #include "chrome/browser/importer/importer_list.h"
 #include "chrome/browser/importer/importer_progress_dialog.h"
 #include "chrome/browser/importer/importer_progress_observer.h"
-#include "chrome/browser/process_singleton.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
@@ -642,16 +641,8 @@ void AutoImport(
     Profile* profile,
     bool homepage_defined,
     int import_items,
-    int dont_import_items,
-    ProcessSingleton* process_singleton) {
+    int dont_import_items) {
 #if !defined(USE_AURA)
-  // We need to avoid dispatching new tabs when we are importing because
-  // that will lead to data corruption or a crash. Because there is no UI for
-  // the import process, we pass NULL as the window to bring to the foreground
-  // when a CopyData message comes in; this causes the message to be silently
-  // discarded, which is the correct behavior during the import process.
-  process_singleton->Lock(NULL);
-
   scoped_refptr<ImporterHost> importer_host;
   // TODO(csilv,mirandac): Out-of-process import has only been qualified on
   // MacOS X, so we will only use it on that platform since it is required.
@@ -727,7 +718,6 @@ void AutoImport(
 
   content::RecordAction(UserMetricsAction("FirstRunDef_Accept"));
 
-  process_singleton->Unlock();
   first_run::CreateSentinel();
 #endif  // !defined(USE_AURA)
   did_perform_profile_import = true;

chrome/browser/first_run/first_run.h

@@ -20,7 +20,6 @@ class CommandLine;
 class GURL;
 class PrefRegistrySyncable;
 class Profile;
-class ProcessSingleton;
 
 namespace base {
 class FilePath;
@@ -141,8 +140,7 @@ const CommandLine& GetExtraArgumentsForImportProcess();
 void AutoImport(Profile* profile,
                 bool homepage_defined,
                 int import_items,
-                int dont_import_items,
-                ProcessSingleton* process_singleton);
+                int dont_import_items);
 
 // Does remaining first run tasks for |profile| and makes Chrome default browser
 // if |make_chrome_default|. This can pop the first run consent dialog on linux.

chrome/browser/first_run/try_chrome_dialog_view.cc

@@ -55,14 +55,14 @@ const int kRadioGroupID = 1;
 // static
 TryChromeDialogView::Result TryChromeDialogView::Show(
     size_t flavor,
-    ProcessSingleton* process_singleton) {
+    const ActiveModalDialogListener& listener) {
   if (flavor > 10000) {
     // This is a test value. We want to make sure we exercise
     // returning this early. See TryChromeDialogBrowserTest test.
     return NOT_NOW;
   }
   TryChromeDialogView dialog(flavor);
-  return dialog.ShowModal(process_singleton);
+  return dialog.ShowModal(listener);
 }
 
 TryChromeDialogView::TryChromeDialogView(size_t flavor)
@@ -79,7 +79,7 @@ TryChromeDialogView::~TryChromeDialogView() {
 }
 
 TryChromeDialogView::Result TryChromeDialogView::ShowModal(
-    ProcessSingleton* process_singleton) {
+    const ActiveModalDialogListener& listener) {
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
 
   views::ImageView* icon = new views::ImageView();
@@ -297,14 +297,13 @@ TryChromeDialogView::Result TryChromeDialogView::ShowModal(
 #endif
   SetToastRegion(toast_window, preferred.width(), preferred.height());
 
-  // Time to show the window in a modal loop. The ProcessSingleton should
-  // already be locked and it will not process WM_COPYDATA requests. Change the
-  // window to bring to foreground if a request arrives.
-  CHECK(process_singleton->locked());
-  process_singleton->SetActiveModalDialog(popup_->GetNativeView());
+  // Time to show the window in a modal loop.
   popup_->Show();
+  if (!listener.is_null())
+    listener.Run(popup_->GetNativeView());
   MessageLoop::current()->Run();
-  process_singleton->SetActiveModalDialog(NULL);
+  if (!listener.is_null())
+    listener.Run(NULL);
   return result_;
 }
 

chrome/browser/first_run/try_chrome_dialog_view.h

@@ -58,6 +58,11 @@ class Widget;
 class TryChromeDialogView : public views::ButtonListener,
                             public views::LinkListener {
  public:
+  // Receives a handle to the active modal dialog, or NULL when the active
+  // dialog is dismissed.
+  typedef base::Callback<void(gfx::NativeWindow active_dialog)>
+      ActiveModalDialogListener;
+
   enum Result {
     TRY_CHROME,             // Launch chrome right now.
     TRY_CHROME_AS_DEFAULT,  // Launch chrome and make it the default.
@@ -71,18 +76,18 @@ class TryChromeDialogView : public views::ButtonListener,
   // above for the possible outcomes of the function. This is an experimental,
   // non-localized dialog.
   // |flavor| can be 0, 1, 2 or 3 and selects what strings to present.
-  // |process_singleton| needs to be valid and it will be locked while
-  // the dialog is shown.
+  // |listener| will be notified when the dialog becomes active and when it is
+  // dismissed.
   // Note that the dialog has no parent and it will position itself in a lower
   // corner of the screen. The dialog does not steal focus and does not have an
   // entry in the taskbar.
-  static Result Show(size_t flavor, ProcessSingleton* process_singleton);
+  static Result Show(size_t flavor, const ActiveModalDialogListener& listener);
 
  private:
   explicit TryChromeDialogView(size_t flavor);
   virtual ~TryChromeDialogView();
 
-  Result ShowModal(ProcessSingleton* process_singleton);
+  Result ShowModal(const ActiveModalDialogListener& listener);
 
   // Returns a screen rectangle that is fit to show the window. In particular
   // it has the following properties: a) is visible and b) is attached to the

chrome/browser/process_singleton.cc

-// Copyright (c) 2012 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/process_singleton.h"
-
-void ProcessSingleton::Unlock() {
-    DCHECK(CalledOnValidThread());
-    locked_ = false;
-    foreground_window_ = NULL;
-    // Replay the command lines of the messages which were received while the
-    // ProcessSingleton was locked. Only replay each message once.
-    std::set<DelayedStartupMessage> replayed_messages;
-    for (std::vector<DelayedStartupMessage>::const_iterator it =
-             saved_startup_messages_.begin();
-         it != saved_startup_messages_.end(); ++it) {
-      if (replayed_messages.find(*it) !=
-          replayed_messages.end())
-        continue;
-      notification_callback_.Run(CommandLine(it->first), it->second);
-      replayed_messages.insert(*it);
-    }
-    saved_startup_messages_.clear();
-}

chrome/browser/process_singleton.h

@@ -84,30 +84,6 @@ class ProcessSingleton : public base::NonThreadSafe {
   // Clear any lock state during shutdown.
   void Cleanup();
 
-  // Blocks the dispatch of CopyData messages. foreground_window refers
-  // to the window that should be set to the foreground if a CopyData message
-  // is received while the ProcessSingleton is locked.
-  void Lock(gfx::NativeWindow foreground_window) {
-    DCHECK(CalledOnValidThread());
-    locked_ = true;
-    foreground_window_ = foreground_window;
-  }
-
-  // Changes the foreground window without changing the locked state.
-  void SetActiveModalDialog(gfx::NativeWindow foreground_window) {
-    DCHECK(CalledOnValidThread());
-    foreground_window_ = foreground_window;
-  }
-
-  // Allows the dispatch of CopyData messages and replays the messages which
-  // were received when the ProcessSingleton was locked.
-  void Unlock();
-
-  bool locked() {
-    DCHECK(CalledOnValidThread());
-    return locked_;
-  }
-
 #if defined(OS_WIN)
   LRESULT WndProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam);
 #endif
@@ -138,17 +114,12 @@ class ProcessSingleton : public base::NonThreadSafe {
 #endif  // defined(OS_LINUX) || defined(OS_OPENBSD)
 
  private:
-  typedef std::pair<CommandLine::StringVector,
-                    base::FilePath> DelayedStartupMessage;
-
 #if !defined(OS_MACOSX)
   // Timeout for the current browser process to respond. 20 seconds should be
   // enough. It's only used in Windows and Linux implementations.
   static const int kTimeoutInSeconds = 20;
 #endif
 
-  bool locked_;
-  gfx::NativeWindow foreground_window_;
   NotificationCallback notification_callback_;  // Handler for notifications.
 
 #if defined(OS_WIN)
@@ -157,8 +128,6 @@ class ProcessSingleton : public base::NonThreadSafe {
 
   bool EscapeVirtualization(const base::FilePath& user_data_dir);
 
-  virtual void DoSetForegroundWindow(HWND target_window);
-
   HWND remote_window_;  // The HWND_MESSAGE of another browser.
   HWND window_;  // The HWND_MESSAGE window.
   bool is_virtualized_;  // Stuck inside Microsoft Softricity VM environment.
@@ -213,10 +182,6 @@ class ProcessSingleton : public base::NonThreadSafe {
   int lock_fd_;
 #endif
 
-  // If messages are received in the locked state, the corresponding command
-  // lines are saved here to be replayed later.
-  std::vector<DelayedStartupMessage> saved_startup_messages_;
-
   DISALLOW_COPY_AND_ASSIGN(ProcessSingleton);
 };
 

chrome/browser/process_singleton_linux.cc

@@ -567,16 +567,6 @@ void ProcessSingleton::LinuxWatcher::HandleMessage(
     SocketReader* reader) {
   DCHECK(ui_message_loop_ == MessageLoop::current());
   DCHECK(reader);
-  // If locked, it means we are not ready to process this message because
-  // we are probably in a first run critical phase.
-  if (parent_->locked()) {
-    DLOG(WARNING) << "Browser is locked";
-    parent_->saved_startup_messages_.push_back(
-        std::make_pair(argv, base::FilePath(current_dir)));
-    // Send back "ACK" message to prevent the client process from starting up.
-    reader->FinishWithACK(kACKToken, arraysize(kACKToken) - 1);
-    return;
-  }
 
   if (parent_->notification_callback_.Run(CommandLine(argv),
                                           base::FilePath(current_dir))) {
@@ -695,9 +685,7 @@ void ProcessSingleton::LinuxWatcher::SocketReader::FinishWithACK(
 ProcessSingleton::ProcessSingleton(
     const base::FilePath& user_data_dir,
     const NotificationCallback& notification_callback)
-    : locked_(false),
-      foreground_window_(NULL),
-      notification_callback_(notification_callback),
+    : notification_callback_(notification_callback),
       current_pid_(base::GetCurrentProcId()),
       ALLOW_THIS_IN_INITIALIZER_LIST(watcher_(new LinuxWatcher(this))) {
   socket_path_ = user_data_dir.Append(chrome::kSingletonSocketFilename);

chrome/browser/process_singleton_mac.cc

@@ -41,9 +41,7 @@ const int kMaxErrno = 102;
 ProcessSingleton::ProcessSingleton(
     const base::FilePath& user_data_dir,
     const NotificationCallback& /* notification_callback */)
-    : locked_(false),
-      foreground_window_(NULL),
-      lock_path_(user_data_dir.Append(chrome::kSingletonLockFilename)),
+    : lock_path_(user_data_dir.Append(chrome::kSingletonLockFilename)),
       lock_fd_(-1) {
 }
 

chrome/browser/process_singleton_modal_dialog_lock.cc

+// 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/process_singleton_modal_dialog_lock.h"
+
+#if defined(OS_WIN)
+#include <Windows.h>
+#endif
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+
+namespace {
+
+#if !defined(OS_WIN) || defined(USE_AURA)
+void DoSetForegroundWindow(gfx::NativeWindow /* target_window */) {
+  NOTREACHED();
+}
+#else
+void DoSetForegroundWindow(HWND target_window) {
+  ::SetForegroundWindow(target_window);
+}
+#endif
+
+}  // namespace
+
+ProcessSingletonModalDialogLock::ProcessSingletonModalDialogLock(
+    const ProcessSingleton::NotificationCallback& original_callback)
+    : active_dialog_(NULL),
+      original_callback_(original_callback),
+      set_foreground_window_handler_(base::Bind(&DoSetForegroundWindow)) {}
+
+ProcessSingletonModalDialogLock::ProcessSingletonModalDialogLock(
+    const ProcessSingleton::NotificationCallback& original_callback,
+    const SetForegroundWindowHandler& set_foreground_window_handler)
+    : active_dialog_(NULL),
+      original_callback_(original_callback),
+      set_foreground_window_handler_(set_foreground_window_handler) {}
+
+ProcessSingletonModalDialogLock::~ProcessSingletonModalDialogLock() {}
+
+void ProcessSingletonModalDialogLock::SetActiveModalDialog(
+    gfx::NativeWindow active_dialog) {
+  active_dialog_ = active_dialog;
+}
+
+ProcessSingleton::NotificationCallback
+ProcessSingletonModalDialogLock::AsNotificationCallback() {
+  return base::Bind(&ProcessSingletonModalDialogLock::NotificationCallbackImpl,
+                    base::Unretained(this));
+}
+
+bool ProcessSingletonModalDialogLock::NotificationCallbackImpl(
+      const CommandLine& command_line,
+      const base::FilePath& current_directory) {
+  if (active_dialog_ != NULL) {
+    set_foreground_window_handler_.Run(active_dialog_);
+    return true;
+  } else {
+    return original_callback_.Run(command_line, current_directory);
+  }
+}

chrome/browser/process_singleton_modal_dialog_lock.h

+// 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_PROCESS_SINGLETON_MODAL_DIALOG_LOCK_H_
+#define CHROME_BROWSER_PROCESS_SINGLETON_MODAL_DIALOG_LOCK_H_
+
+#include "base/basictypes.h"
+#include "base/callback_forward.h"
+#include "chrome/browser/process_singleton.h"
+#include "ui/gfx/native_widget_types.h"
+
+class CommandLine;
+
+namespace base {
+class FilePath;
+}
+
+// Provides a ProcessSingleton::NotificationCallback that prevents
+// command-line handling when a modal dialog is active during startup. The
+// client must ensure that SetActiveModalDialog is called appropriately when
+// such dialogs are displayed or dismissed.
+//
+// While a dialog is active, the ProcessSingleton notification
+// callback will handle but ignore notifications:
+// 1. Neither this process nor the invoking process will handle the command
+//    line.
+// 2. The active dialog is brought to the foreground and/or the taskbar icon
+//    flashed (using ::SetForegroundWindow on Windows).
+//
+// Otherwise, the notification is forwarded to a wrapped NotificationCallback.
+class ProcessSingletonModalDialogLock {
+ public:
+  typedef base::Callback<void(gfx::NativeWindow)> SetForegroundWindowHandler;
+  explicit ProcessSingletonModalDialogLock(
+      const ProcessSingleton::NotificationCallback& original_callback);
+
+  ProcessSingletonModalDialogLock(
+      const ProcessSingleton::NotificationCallback& original_callback,
+      const SetForegroundWindowHandler& set_foreground_window_handler);
+
+  ~ProcessSingletonModalDialogLock();
+
+  // Receives a handle to the active modal dialog, or NULL if the active dialog
+  // is dismissed.
+  void SetActiveModalDialog(gfx::NativeWindow active_dialog);
+
+  // Returns the ProcessSingleton::NotificationCallback.
+  // The callback is only valid during the lifetime of the
+  // ProcessSingletonModalDialogLock instance.
+  ProcessSingleton::NotificationCallback AsNotificationCallback();
+
+ private:
+  bool NotificationCallbackImpl(const CommandLine& command_line,
+                                const base::FilePath& current_directory);
+
+  gfx::NativeWindow active_dialog_;
+  ProcessSingleton::NotificationCallback original_callback_;
+  SetForegroundWindowHandler set_foreground_window_handler_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProcessSingletonModalDialogLock);
+};
+
+#endif  // CHROME_BROWSER_PROCESS_SINGLETON_MODAL_DIALOG_LOCK_H_

chrome/browser/process_singleton_startup_lock.cc

+// 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/process_singleton_startup_lock.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+
+ProcessSingletonStartupLock::ProcessSingletonStartupLock(
+    const ProcessSingleton::NotificationCallback& original_callback)
+    : locked_(true),
+      original_callback_(original_callback) {}
+
+ProcessSingletonStartupLock::~ProcessSingletonStartupLock() {}
+
+ProcessSingleton::NotificationCallback
+ProcessSingletonStartupLock::AsNotificationCallback() {
+  return base::Bind(&ProcessSingletonStartupLock::NotificationCallbackImpl,
+                    base::Unretained(this));
+}
+
+void ProcessSingletonStartupLock::Unlock() {
+  DCHECK(CalledOnValidThread());
+  locked_ = false;
+
+  // Replay the command lines of the messages which were received while the
+  // ProcessSingleton was locked. Only replay each message once.
+  std::set<DelayedStartupMessage> replayed_messages;
+  for (std::vector<DelayedStartupMessage>::const_iterator it =
+           saved_startup_messages_.begin();
+       it != saved_startup_messages_.end(); ++it) {
+    if (replayed_messages.find(*it) != replayed_messages.end())
+      continue;
+    original_callback_.Run(CommandLine(it->first), it->second);
+    replayed_messages.insert(*it);
+  }
+  saved_startup_messages_.clear();
+}
+
+bool ProcessSingletonStartupLock::NotificationCallbackImpl(
+    const CommandLine& command_line,
+    const base::FilePath& current_directory) {
+  if (locked_) {
+    // If locked, it means we are not ready to process this message because
+    // we are probably in a first run critical phase.
+    saved_startup_messages_.push_back(
+        std::make_pair(command_line.argv(), current_directory));
+    return true;
+  } else {
+    return original_callback_.Run(command_line, current_directory);
+  }
+}

chrome/browser/process_singleton_startup_lock.h

+// 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_PROCESS_SINGLETON_STARTUP_LOCK_H_
+#define CHROME_BROWSER_PROCESS_SINGLETON_STARTUP_LOCK_H_
+
+#include <set>
+#include <utility>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/threading/non_thread_safe.h"
+#include "chrome/browser/process_singleton.h"
+
+// Provides a ProcessSingleton::NotificationCallback that can queue up
+// command-line invocations during startup and execute them when startup
+// completes.
+//
+// The object starts in a locked state. |Unlock()| must be called
+// when the process is prepared to handle command-line invocations.
+//
+// Once unlocked, notifications are forwarded to a wrapped NotificationCallback.
+class ProcessSingletonStartupLock : public base::NonThreadSafe {
+ public:
+  explicit ProcessSingletonStartupLock(
+      const ProcessSingleton::NotificationCallback& original_callback);
+  ~ProcessSingletonStartupLock();
+
+  // Returns the ProcessSingleton::NotificationCallback.
+  // The callback is only valid during the lifetime of the
+  // ProcessSingletonStartupLock instance.
+  ProcessSingleton::NotificationCallback AsNotificationCallback();
+
+  // Executes previously queued command-line invocations and allows future
+  // invocations to be executed immediately.
+  void Unlock();
+
+  bool locked() { return locked_; }
+
+ private:
+  typedef std::pair<CommandLine::StringVector, base::FilePath>
+      DelayedStartupMessage;
+
+  bool NotificationCallbackImpl(const CommandLine& command_line,
+                                const base::FilePath& current_directory);
+
+  bool locked_;
+  std::vector<DelayedStartupMessage> saved_startup_messages_;
+  ProcessSingleton::NotificationCallback original_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProcessSingletonStartupLock);
+};
+
+#endif  // CHROME_BROWSER_PROCESS_SINGLETON_STARTUP_LOCK_H_

chrome/browser/process_singleton_win.cc

@@ -255,8 +255,7 @@ bool ProcessSingleton::EscapeVirtualization(
 ProcessSingleton::ProcessSingleton(
     const base::FilePath& user_data_dir,
     const NotificationCallback& notification_callback)
-    : window_(NULL), locked_(false), foreground_window_(NULL),
-      notification_callback_(notification_callback),
+    : window_(NULL), notification_callback_(notification_callback),
       is_virtualized_(false), lock_file_(INVALID_HANDLE_VALUE),
       user_data_dir_(user_data_dir) {
 }
@@ -538,28 +537,6 @@ void ProcessSingleton::Cleanup() {
 }
 
 LRESULT ProcessSingleton::OnCopyData(HWND hwnd, const COPYDATASTRUCT* cds) {
-  // If locked, it means we are not ready to process this message because
-  // we are probably in a first run critical phase.
-  if (locked_) {
-#if defined(USE_AURA)
-    NOTIMPLEMENTED();
-#else
-    // Attempt to place ourselves in the foreground / flash the task bar.
-    if (foreground_window_ != NULL && ::IsWindow(foreground_window_)) {
-      DoSetForegroundWindow(foreground_window_);
-    } else {
-      // Read the command line and store it. It will be replayed when the
-      // ProcessSingleton becomes unlocked.
-      CommandLine parsed_command_line(CommandLine::NO_PROGRAM);
-      base::FilePath current_directory;
-      if (ParseCommandLine(cds, &parsed_command_line, &current_directory))
-        saved_startup_messages_.push_back(
-            std::make_pair(parsed_command_line.argv(), current_directory));
-    }
-#endif
-    return TRUE;
-  }
-
   CommandLine parsed_command_line(CommandLine::NO_PROGRAM);
   base::FilePath current_directory;
   if (!ParseCommandLine(cds, &parsed_command_line, &current_directory))
@@ -568,10 +545,6 @@ LRESULT ProcessSingleton::OnCopyData(HWND hwnd, const COPYDATASTRUCT* cds) {
       TRUE : FALSE;
 }
 
-void ProcessSingleton::DoSetForegroundWindow(HWND target_window) {
-  ::SetForegroundWindow(target_window);
-}
-
 LRESULT ProcessSingleton::WndProc(HWND hwnd, UINT message,
                                   WPARAM wparam, LPARAM lparam) {
   switch (message) {

chrome/chrome_browser.gypi

@@ -348,6 +348,8 @@
         'browser/chrome_page_zoom.h',
         'browser/chrome_page_zoom_constants.cc',
         'browser/chrome_page_zoom_constants.h',
+        'browser/chrome_process_singleton.cc',
+        'browser/chrome_process_singleton.h',
         'browser/chrome_quota_permission_context.cc',
         'browser/chrome_quota_permission_context.h',
         'browser/chrome_to_mobile_service.cc',
@@ -1472,10 +1474,13 @@
         'browser/printing/printing_message_filter.h',
         'browser/process_info_snapshot.h',
         'browser/process_info_snapshot_mac.cc',
-        'browser/process_singleton.cc',
         'browser/process_singleton.h',
         'browser/process_singleton_linux.cc',
         'browser/process_singleton_mac.cc',
+        'browser/process_singleton_modal_dialog_lock.cc',
+        'browser/process_singleton_modal_dialog_lock.h',
+        'browser/process_singleton_startup_lock.cc',
+        'browser/process_singleton_startup_lock.h',
         'browser/process_singleton_win.cc',
         'browser/profiles/avatar_menu_model.cc',
         'browser/profiles/avatar_menu_model.h',
@@ -2684,6 +2689,7 @@
             # Not used by Android
             'browser/chrome_browser_main_posix.cc',
             'browser/chrome_browser_main_posix.h',
+            'browser/chrome_process_singleton.cc',
             'browser/process_singleton.cc',
           ],
           'sources/': [

chrome/chrome_tests_unit.gypi

@@ -521,6 +521,7 @@
         'browser/captive_portal/testing_utils.h',
         'browser/chrome_browser_application_mac_unittest.mm',
         'browser/chrome_page_zoom_unittest.cc',
+        'browser/chrome_process_singleton_win_unittest.cc',
         'browser/chromeos/accessibility/magnification_manager_unittest.cc',
         'browser/chromeos/contacts/contact_database_unittest.cc',
         'browser/chromeos/contacts/contact_manager_stub.cc',
@@ -1003,7 +1004,6 @@
         'browser/process_info_snapshot_mac_unittest.cc',
         'browser/process_singleton_linux_unittest.cc',
         'browser/process_singleton_mac_unittest.cc',
-        'browser/process_singleton_win_unittest.cc',
         'browser/profiles/avatar_menu_model_unittest.cc',
         'browser/profiles/dependency_graph_unittest.cc',
         'browser/profiles/gaia_info_update_service_unittest.cc',

chrome_frame/test/net/fake_external_tab.cc

@@ -791,7 +791,6 @@ int CFUrlRequestUnittestRunner::PreCreateThreads() {
           base::Unretained(this)));
   process_singleton_.reset(new ProcessSingleton(fake_chrome_->user_data(),
                                                 callback));
-  process_singleton_->Lock(NULL);
   return 0;
 }
 
@@ -822,8 +821,6 @@ void CFUrlRequestUnittestRunner::PreMainMessageLoopRun() {
       base::KillProcess(crash_service_, 0, false);
     ::ExitProcess(1);
   }
-
-  StartChromeFrameInHostBrowser();
 }
 
 bool CFUrlRequestUnittestRunner::MainMessageLoopRun(int* result_code) {
@@ -832,7 +829,7 @@ bool CFUrlRequestUnittestRunner::MainMessageLoopRun(int* result_code) {
 
   // We need to allow IO on the main thread for these tests.
   base::ThreadRestrictions::SetIOAllowed(true);
-  process_singleton_->Unlock();
+  StartChromeFrameInHostBrowser();
   StartInitializationTimeout();
   return false;
 }