Added the service process pid to the shared memory data. This will be used for tests. Also cleaned up some of the method names in service_process_utils.cc to make them clearer.

BUG=None.
TEST=Browser tests.

Review URL: http://codereview.chromium.org/3727002

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

5 files changed, 96 insertions, 46 deletions

diff --git a/chrome/browser/service/service_process_control.cc b/chrome/browser/service/service_process_control.cc
index acccbda..ad3c326 100644
--- a/chrome/browser/service/service_process_control.cc
+++ b/chrome/browser/service/service_process_control.cc
@@ -54,7 +54,7 @@ class ServiceProcessControl::Launcher
   void DoDetectLaunched(Task* task) {
     const uint32 kMaxLaunchDetectRetries = 10;
 
-    launched_ = CheckServiceProcessRunning();
+    launched_ = CheckServiceProcessReady();
     if (launched_ || (retry_count_ >= kMaxLaunchDetectRetries)) {
       BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
           NewRunnableMethod(this, &Launcher::Notify, task));
@@ -130,7 +130,7 @@ void ServiceProcessControl::Launch(Task* task) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // If the service process is already running then connects to it.
-  if (CheckServiceProcessRunning()) {
+  if (CheckServiceProcessReady()) {
     ConnectInternal(task);
     return;
   }
diff --git a/chrome/browser/service/service_process_control_browsertest.cc b/chrome/browser/service/service_process_control_browsertest.cc
index 5c59e87..c9038e6 100644
--- a/chrome/browser/service/service_process_control_browsertest.cc
+++ b/chrome/browser/service/service_process_control_browsertest.cc
@@ -47,6 +47,8 @@ class ServiceProcessControlBrowserTest
 
   void WaitForShutdown() {
     // We will keep trying every second till we hit the terminate timeout.
+    // TODO(sanjeevr): Use GetServiceProcessPid() to wait for termination.
+    // Will do this in a separate CL.
     int retries_left = TestTimeouts::wait_for_terminate_timeout_ms()/1000;
     MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
@@ -58,7 +60,7 @@ class ServiceProcessControlBrowserTest
   }
 
   void DoDetectShutdown(int retries_left) {
-    bool service_is_running = CheckServiceProcessRunning();
+    bool service_is_running = CheckServiceProcessReady();
     if (!retries_left)
       EXPECT_FALSE(service_is_running);
     if (retries_left && service_is_running) {
@@ -148,6 +150,13 @@ IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest, ForceShutdown) {
   WaitForShutdown();
 }
 
+IN_PROC_BROWSER_TEST_F(ServiceProcessControlBrowserTest, CheckPid) {
+  EXPECT_EQ(0, GetServiceProcessPid());
+  // Launch the service process.
+  LaunchServiceProcessControl();
+  EXPECT_NE(static_cast<base::ProcessId>(0), GetServiceProcessPid());
+}
+
 #endif
 
 DISABLE_RUNNABLE_METHOD_REFCOUNT(ServiceProcessControlBrowserTest);
diff --git a/chrome/common/service_process_util.cc b/chrome/common/service_process_util.cc
index 6710059..7eb6f71 100644
--- a/chrome/common/service_process_util.cc
+++ b/chrome/common/service_process_util.cc
@@ -5,6 +5,7 @@
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
+#include "base/process_util.h"
 #include "base/shared_memory.h"
 #include "base/string16.h"
 #include "base/string_util.h"
@@ -28,6 +29,12 @@ namespace {
 // of the version string is an int64.
 const uint32 kMaxVersionStringLength = 256;
 
+// The structure that gets written to shared memory.
+struct ServiceProcessSharedData {
+  char service_process_version[kMaxVersionStringLength];
+  base::ProcessId service_process_pid;
+};
+
 // Return a name that is scoped to this instance of the service process. We
 // use the user-data-dir as a scoping prefix.
 std::string GetServiceProcessScopedName(const std::string& append_str) {
@@ -61,6 +68,29 @@ std::string GetServiceProcessSharedMemName() {
   return GetServiceProcessScopedName("_service_shmem");
 }
 
+// Reads the named shared memory to get the shared data. Returns false if no
+// matching shared memory was found.
+bool GetServiceProcessSharedData(std::string* version, base::ProcessId* pid) {
+  scoped_ptr<base::SharedMemory> shared_mem_service_data;
+  shared_mem_service_data.reset(new base::SharedMemory());
+  ServiceProcessSharedData* service_data = NULL;
+  if (shared_mem_service_data.get() &&
+      shared_mem_service_data->Open(GetServiceProcessSharedMemName(), true) &&
+      shared_mem_service_data->Map(sizeof(ServiceProcessSharedData))) {
+    service_data = reinterpret_cast<ServiceProcessSharedData*>(
+        shared_mem_service_data->memory());
+    // Make sure the version in shared memory is null-terminated. If it is not,
+    // treat it as invalid.
+    if (version && memchr(service_data->service_process_version, '\0',
+                          sizeof(service_data->service_process_version)))
+      *version = service_data->service_process_version;
+    if (pid)
+      *pid = service_data->service_process_pid;
+    return true;
+  }
+  return false;
+}
+
 enum ServiceProcessRunningState {
   SERVICE_NOT_RUNNING,
   SERVICE_OLDER_VERSION_RUNNING,
@@ -71,17 +101,10 @@ enum ServiceProcessRunningState {
 ServiceProcessRunningState GetServiceProcessRunningState(
     std::string* service_version_out) {
   std::string version;
-  scoped_ptr<base::SharedMemory> shared_mem_version;
-  shared_mem_version.reset(new base::SharedMemory());
-  if (shared_mem_version.get() &&
-      shared_mem_version->Open(GetServiceProcessSharedMemName(), true) &&
-      shared_mem_version->Map(kMaxVersionStringLength) &&
-      memchr(shared_mem_version->memory(), '\0', kMaxVersionStringLength)) {
-    version =
-        static_cast<const char *>(shared_mem_version->memory());
-  } else {
+  GetServiceProcessSharedData(&version, NULL);
+  if (version.empty())
     return SERVICE_NOT_RUNNING;
-  }
+
   // At this time we have a version string. Set the out param if it exists.
   if (service_version_out)
     *service_version_out = version;
@@ -120,9 +143,9 @@ ServiceProcessRunningState GetServiceProcessRunningState(
 }
 
 #if defined(OS_WIN)
-string16 GetServiceProcessEventName() {
+string16 GetServiceProcessReadyEventName() {
   return UTF8ToWide(
-      GetServiceProcessScopedVersionedName("Service Process Lock"));
+      GetServiceProcessScopedVersionedName("_service_ready"));
 }
 
 string16 GetServiceProcessShutdownEventName() {
@@ -165,10 +188,10 @@ FilePath GetServiceProcessLockFilePath() {
 #endif  // defined(OS_WIN)
 
 struct ServiceProcessGlobalState {
-  scoped_ptr<base::SharedMemory> shared_mem_version;
+  scoped_ptr<base::SharedMemory> shared_mem_service_data;
 #if defined(OS_WIN)
-  // An event that is signaled when a service process is running.
-  ScopedHandle running_event;
+  // An event that is signaled when a service process is ready.
+  ScopedHandle ready_event;
   scoped_ptr<ServiceProcessShutdownMonitor> shutdown_monitor;
 #endif  // defined(OS_WIN)
 };
@@ -210,10 +233,10 @@ bool TakeServiceProcessSingletonLock() {
   // mechanism. For that the shared mem class needs to return whether it created
   // new instance or opened an existing one.
 #if defined(OS_WIN)
-  string16 event_name = GetServiceProcessEventName();
+  string16 event_name = GetServiceProcessReadyEventName();
   CHECK(event_name.length() <= MAX_PATH);
-  ScopedHandle service_process_running_event;
-  service_process_running_event.Set(
+  ScopedHandle service_process_ready_event;
+  service_process_ready_event.Set(
       CreateEvent(NULL, TRUE, FALSE, event_name.c_str()));
   DWORD error = GetLastError();
   if ((error == ERROR_ALREADY_EXISTS) || (error == ERROR_ACCESS_DENIED)) {
@@ -221,8 +244,8 @@ bool TakeServiceProcessSingletonLock() {
     g_service_globals = NULL;
     return false;
   }
-  DCHECK(service_process_running_event.IsValid());
-  g_service_globals->running_event.Set(service_process_running_event.Take());
+  DCHECK(service_process_ready_event.IsValid());
+  g_service_globals->ready_event.Set(service_process_ready_event.Take());
 #else
   // TODO(sanjeevr): Implement singleton mechanism for other platforms.
   NOTIMPLEMENTED();
@@ -241,31 +264,36 @@ bool TakeServiceProcessSingletonLock() {
     return false;
   }
 
-  scoped_ptr<base::SharedMemory> shared_mem_version;
-  shared_mem_version.reset(new base::SharedMemory());
-  if (!shared_mem_version.get())
+  scoped_ptr<base::SharedMemory> shared_mem_service_data;
+  shared_mem_service_data.reset(new base::SharedMemory());
+  if (!shared_mem_service_data.get())
     return false;
 
-  uint32 alloc_size = kMaxVersionStringLength;
-  if (!shared_mem_version->Create(GetServiceProcessSharedMemName(), false,
-                                  true, alloc_size))
+  uint32 alloc_size = sizeof(ServiceProcessSharedData);
+  if (!shared_mem_service_data->Create(GetServiceProcessSharedMemName(), false,
+                                       true, alloc_size))
     return false;
 
-  if (!shared_mem_version->Map(alloc_size))
+  if (!shared_mem_service_data->Map(alloc_size))
     return false;
 
-  memset(shared_mem_version->memory(), 0, alloc_size);
-  memcpy(shared_mem_version->memory(), version_info.Version().c_str(),
+  memset(shared_mem_service_data->memory(), 0, alloc_size);
+  ServiceProcessSharedData* shared_data =
+      reinterpret_cast<ServiceProcessSharedData*>(
+          shared_mem_service_data->memory());
+  memcpy(shared_data->service_process_version, version_info.Version().c_str(),
          version_info.Version().length());
-  g_service_globals->shared_mem_version.reset(shared_mem_version.release());
+  shared_data->service_process_pid = base::GetCurrentProcId();
+  g_service_globals->shared_mem_service_data.reset(
+      shared_mem_service_data.release());
   return true;
 }
 
-void SignalServiceProcessRunning(Task* shutdown_task) {
+void SignalServiceProcessReady(Task* shutdown_task) {
 #if defined(OS_WIN)
   DCHECK(g_service_globals != NULL);
-  DCHECK(g_service_globals->running_event.IsValid());
-  SetEvent(g_service_globals->running_event.Get());
+  DCHECK(g_service_globals->ready_event.IsValid());
+  SetEvent(g_service_globals->ready_event.Get());
   g_service_globals->shutdown_monitor.reset(
       new ServiceProcessShutdownMonitor(shutdown_task));
   g_service_globals->shutdown_monitor->Start();
@@ -311,9 +339,9 @@ bool ForceServiceProcessShutdown(const std::string& version) {
 #endif    // defined(OS_WIN)
 }
 
-bool CheckServiceProcessRunning() {
+bool CheckServiceProcessReady() {
 #if defined(OS_WIN)
-  string16 event_name = GetServiceProcessEventName();
+  string16 event_name = GetServiceProcessReadyEventName();
   ScopedHandle event(
       OpenEvent(SYNCHRONIZE | READ_CONTROL, false, event_name.c_str()));
   if (!event.IsValid())
@@ -327,3 +355,9 @@ bool CheckServiceProcessRunning() {
 #endif
 }
 
+base::ProcessId GetServiceProcessPid() {
+  base::ProcessId pid = 0;
+  GetServiceProcessSharedData(NULL, &pid);
+  return pid;
+}
+
diff --git a/chrome/common/service_process_util.h b/chrome/common/service_process_util.h
index c84069a..8876426 100644
--- a/chrome/common/service_process_util.h
+++ b/chrome/common/service_process_util.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/process.h"
 #include "base/task.h"
 
 // Return the IPC channel to connect to the service process.
@@ -20,11 +21,11 @@ std::string GetServiceProcessChannelName();
 // Returns false is another service process is already running.
 bool TakeServiceProcessSingletonLock();
 
-// Signal that the service process is running.
+// Signal that the service process is ready.
 // This method is called when the service process is running and initialized.
 // |shutdown_task| is invoked when we get a shutdown request from another
-// process (in the same thread that called SignalServiceProcessRunning).
-void SignalServiceProcessRunning(Task* shutdown_task);
+// process (in the same thread that called SignalServiceProcessReady).
+void SignalServiceProcessReady(Task* shutdown_task);
 
 // Signal that the service process is stopped.
 void SignalServiceProcessStopped();
@@ -44,10 +45,16 @@ bool ForceServiceProcessShutdown(const std::string& version);
 
 // --------------------------------------------------------------------------
 
-// This method checks that if the service process is running (ready to receive
-// IPC commands).
+// This method checks that if the service process is ready to receive
+// IPC commands.
+bool CheckServiceProcessReady();
 
-bool CheckServiceProcessRunning();
+// Returns the process id of the currently running service process. Returns 0
+// if no service process is running.
+// Note: DO NOT use this check whether the service process is ready because
+// a non-zero return value only means that the process is running and not that
+// it is ready to receive IPC commands. This method is only exposed for testing.
+base::ProcessId GetServiceProcessPid();
 // --------------------------------------------------------------------------
 
 #endif  // CHROME_COMMON_SERVICE_PROCESS_UTIL_H_
diff --git a/chrome/service/service_process.cc b/chrome/service/service_process.cc
index 3af4d2f..140a313 100644
--- a/chrome/service/service_process.cc
+++ b/chrome/service/service_process.cc
@@ -112,8 +112,8 @@ bool ServiceProcess::Initialize(MessageLoop* message_loop,
   ipc_server_->Init();
 
   // After the IPC server has started we signal that the service process is
-  // running.
-  SignalServiceProcessRunning(
+  // ready.
+  SignalServiceProcessReady(
       NewRunnableMethod(this, &ServiceProcess::Shutdown));
 
   // See if we need to stay running.