Move many ProcessSingleton methods to "protected" visibility as an upcoming refactoring of ProcessSingleton on Windows requires that only NotifyOtherProcessOrCreate() be callable from the public interface.

This doesn't change any code logic, just moves things around.

BUG=None

Review URL: https://codereview.chromium.org/11415237

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

3 files changed, 62 insertions, 38 deletions

diff --git a/chrome/browser/process_singleton.h b/chrome/browser/process_singleton.h
index 2b03b2d..961a51f 100644
--- a/chrome/browser/process_singleton.h
+++ b/chrome/browser/process_singleton.h
@@ -67,16 +67,6 @@ class ProcessSingleton : public base::NonThreadSafe {
   explicit ProcessSingleton(const FilePath& user_data_dir);
   ~ProcessSingleton();
 
-  // Notify another process, if available.
-  // Returns true if another process was found and notified, false if we
-  // should continue with this process.
-  // Windows code roughly based on Mozilla.
-  //
-  // TODO(brettw): this will not handle all cases. If two process start up too
-  // close to each other, the Create() might not yet have happened for the
-  // first one, so this function won't find it.
-  NotifyResult NotifyOtherProcess();
-
   // Notify another process, if available. Otherwise sets ourselves as the
   // singleton instance and stores the provided callback for notification from
   // future processes. Returns PROCESS_NONE if we became the singleton
@@ -84,20 +74,14 @@ class ProcessSingleton : public base::NonThreadSafe {
   NotifyResult NotifyOtherProcessOrCreate(
       const NotificationCallback& notification_callback);
 
-#if defined(OS_LINUX) || defined(OS_OPENBSD)
-  // Exposed for testing.  We use a timeout on Linux, and in tests we want
-  // this timeout to be short.
-  NotifyResult NotifyOtherProcessWithTimeout(const CommandLine& command_line,
-                                             int timeout_seconds,
-                                             bool kill_unresponsive);
-  NotifyResult NotifyOtherProcessWithTimeoutOrCreate(
-      const CommandLine& command_line,
-      const NotificationCallback& notification_callback,
-      int timeout_seconds);
-  void OverrideCurrentPidForTesting(base::ProcessId pid);
-  void OverrideKillCallbackForTesting(const base::Callback<void(int)>& callback);
-  static void DisablePromptForTesting();
-#endif  // defined(OS_LINUX) || defined(OS_OPENBSD)
+  // Sets ourself up as the singleton instance.  Returns true on success.  If
+  // false is returned, we are not the singleton instance and the caller must
+  // exit. Otherwise, stores the provided callback for notification from
+  // future processes.
+  // NOTE: Most callers should generally prefer NotifyOtherProcessOrCreate() to
+  // this method, only callers for whom failure is prefered to notifying another
+  // process should call this directly.
+  bool Create(const NotificationCallback& notification_callback);
 
 #if defined(OS_WIN)
   // Used in specific cases to let us know that there is an existing instance
@@ -110,13 +94,6 @@ class ProcessSingleton : public base::NonThreadSafe {
   }
 #endif  // defined(OS_WIN)
 
-  // Sets ourself up as the singleton instance.  Returns true on success.  If
-  // false is returned, we are not the singleton instance and the caller must
-  // exit. Otherwise, stores the provided callback for notification from
-  // future processes.
-  bool Create(
-      const NotificationCallback& notification_callback);
-
   // Clear any lock state during shutdown.
   void Cleanup();
 
@@ -148,6 +125,36 @@ class ProcessSingleton : public base::NonThreadSafe {
   LRESULT WndProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam);
 #endif
 
+#if defined(OS_LINUX) || defined(OS_OPENBSD)
+  static void DisablePromptForTesting();
+#endif  // defined(OS_LINUX) || defined(OS_OPENBSD)
+
+ protected:
+  // Notify another process, if available.
+  // Returns true if another process was found and notified, false if we
+  // should continue with this process.
+  // Windows code roughly based on Mozilla.
+  //
+  // TODO(brettw): this will not handle all cases. If two processes start up too
+  // close to each other, the Create() might not yet have happened for the
+  // first one, so this function won't find it.
+  NotifyResult NotifyOtherProcess();
+
+#if defined(OS_LINUX) || defined(OS_OPENBSD)
+  // Exposed for testing.  We use a timeout on Linux, and in tests we want
+  // this timeout to be short.
+  NotifyResult NotifyOtherProcessWithTimeout(const CommandLine& command_line,
+                                             int timeout_seconds,
+                                            bool kill_unresponsive);
+  NotifyResult NotifyOtherProcessWithTimeoutOrCreate(
+      const CommandLine& command_line,
+      const NotificationCallback& notification_callback,
+      int timeout_seconds);
+  void OverrideCurrentPidForTesting(base::ProcessId pid);
+  void OverrideKillCallbackForTesting(
+      const base::Callback<void(int)>& callback);
+#endif  // defined(OS_LINUX) || defined(OS_OPENBSD)
+
  private:
   typedef std::pair<CommandLine::StringVector, FilePath> DelayedStartupMessage;
 
diff --git a/chrome/browser/process_singleton_linux_unittest.cc b/chrome/browser/process_singleton_linux_unittest.cc
index 3bc0066..06b96154 100644
--- a/chrome/browser/process_singleton_linux_unittest.cc
+++ b/chrome/browser/process_singleton_linux_unittest.cc
@@ -39,6 +39,17 @@ bool NotificationCallback(const CommandLine& command_line,
 
 class ProcessSingletonLinuxTest : public testing::Test {
  public:
+  // A ProcessSingleton exposing some protected methods for testing.
+  class TestableProcessSingleton : public ProcessSingleton {
+   public:
+    explicit TestableProcessSingleton(const FilePath& user_data_dir)
+        : ProcessSingleton(user_data_dir) {}
+    using ProcessSingleton::NotifyOtherProcessWithTimeout;
+    using ProcessSingleton::NotifyOtherProcessWithTimeoutOrCreate;
+    using ProcessSingleton::OverrideCurrentPidForTesting;
+    using ProcessSingleton::OverrideKillCallbackForTesting;
+  };
+
   ProcessSingletonLinuxTest()
       : kill_callbacks_(0),
         io_thread_(BrowserThread::IO),
@@ -99,14 +110,15 @@ class ProcessSingletonLinuxTest : public testing::Test {
     ASSERT_TRUE(helper->Run());
   }
 
-  ProcessSingleton* CreateProcessSingleton() {
-    return new ProcessSingleton(temp_dir_.path());
+  TestableProcessSingleton* CreateProcessSingleton() {
+    return new TestableProcessSingleton(temp_dir_.path());
   }
 
   ProcessSingleton::NotifyResult NotifyOtherProcess(
       bool override_kill,
       base::TimeDelta timeout) {
-    scoped_ptr<ProcessSingleton> process_singleton(CreateProcessSingleton());
+    scoped_ptr<TestableProcessSingleton> process_singleton(
+        CreateProcessSingleton());
     CommandLine command_line(CommandLine::ForCurrentProcess()->GetProgram());
     command_line.AppendArg("about:blank");
     if (override_kill) {
@@ -124,7 +136,8 @@ class ProcessSingletonLinuxTest : public testing::Test {
   ProcessSingleton::NotifyResult NotifyOtherProcessOrCreate(
       const std::string& url,
       base::TimeDelta timeout) {
-    scoped_ptr<ProcessSingleton> process_singleton(CreateProcessSingleton());
+    scoped_ptr<TestableProcessSingleton> process_singleton(
+        CreateProcessSingleton());
     CommandLine command_line(CommandLine::ForCurrentProcess()->GetProgram());
     command_line.AppendArg(url);
     return process_singleton->NotifyOtherProcessWithTimeoutOrCreate(
@@ -197,7 +210,7 @@ class ProcessSingletonLinuxTest : public testing::Test {
   base::WaitableEvent signal_event_;
 
   scoped_ptr<base::Thread> worker_thread_;
-  ProcessSingleton* process_singleton_on_thread_;
+  TestableProcessSingleton* process_singleton_on_thread_;
 
   std::vector<CommandLine::StringVector> callback_command_lines_;
 };
@@ -332,7 +345,8 @@ TEST_F(ProcessSingletonLinuxTest, NotifyOtherProcessOrCreate_DifferingHost) {
 TEST_F(ProcessSingletonLinuxTest, CreateFailsWithExistingBrowser) {
   CreateProcessSingletonOnThread();
 
-  scoped_ptr<ProcessSingleton> process_singleton(CreateProcessSingleton());
+  scoped_ptr<TestableProcessSingleton> process_singleton(
+      CreateProcessSingleton());
   process_singleton->OverrideCurrentPidForTesting(base::GetCurrentProcId() + 1);
   EXPECT_FALSE(process_singleton->Create(
                base::Bind(&NotificationCallback)));
@@ -342,7 +356,8 @@ TEST_F(ProcessSingletonLinuxTest, CreateFailsWithExistingBrowser) {
 // but with the old socket location.
 TEST_F(ProcessSingletonLinuxTest, CreateChecksCompatibilitySocket) {
   CreateProcessSingletonOnThread();
-  scoped_ptr<ProcessSingleton> process_singleton(CreateProcessSingleton());
+  scoped_ptr<TestableProcessSingleton> process_singleton(
+      CreateProcessSingleton());
   process_singleton->OverrideCurrentPidForTesting(base::GetCurrentProcId() + 1);
 
   // Do some surgery so as to look like the old configuration.
diff --git a/chrome_frame/test/net/fake_external_tab.cc b/chrome_frame/test/net/fake_external_tab.cc
index d99276a..35250c6 100644
--- a/chrome_frame/test/net/fake_external_tab.cc
+++ b/chrome_frame/test/net/fake_external_tab.cc
@@ -794,6 +794,8 @@ void CFUrlRequestUnittestRunner::PreMainMessageLoopRun() {
       base::Bind(
           &CFUrlRequestUnittestRunner::ProcessSingletonNotificationCallback,
           base::Unretained(this)));
+  // Call Create directly instead of NotifyOtherProcessOrCreate as failure is
+  // prefered to notifying another process here.
   if (!process_singleton_->Create(callback)) {
     LOG(FATAL) << "Failed to start up ProcessSingleton. Is another test "
                << "executable or Chrome Frame running?";