Fixes up the race condition that exists with deleting a file vs watching it.

The tests created a file and attempted to hook a watcher on it on a separate thread
but it was deleting it before the thread had a chance to watch it.

BUG=77217,77064, 69641
TEST=BUILD

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

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

3 files changed, 46 insertions, 21 deletions

diff --git a/chrome/common/service_process_util_posix.cc b/chrome/common/service_process_util_posix.cc
index 5592edf..bce541a 100644
--- a/chrome/common/service_process_util_posix.cc
+++ b/chrome/common/service_process_util_posix.cc
@@ -7,6 +7,7 @@
 #include "base/basictypes.h"
 #include "base/eintr_wrapper.h"
 #include "base/message_loop_proxy.h"
+#include "base/synchronization/waitable_event.h"
 
 namespace {
 int g_signal_socket = -1;
@@ -58,11 +59,18 @@ ServiceProcessState::StateData::StateData() : set_action_(false) {
   memset(&old_action_, 0, sizeof(old_action_));
 }
 
-void ServiceProcessState::StateData::SignalReady() {
+void ServiceProcessState::StateData::SignalReady(base::WaitableEvent* signal,
+                                                 bool* success) {
   CHECK_EQ(g_signal_socket, -1);
-  CHECK(MessageLoopForIO::current()->WatchFileDescriptor(
+  CHECK(!signal->IsSignaled());
+  *success = MessageLoopForIO::current()->WatchFileDescriptor(
       sockets_[0], true, MessageLoopForIO::WATCH_READ,
-      &watcher_, shut_down_monitor_.get()));
+      &watcher_, shut_down_monitor_.get());
+  if (!*success) {
+    LOG(ERROR) << "WatchFileDescriptor";
+    signal->Signal();
+    return;
+  }
   g_signal_socket = sockets_[1];
 
   // Set up signal handler for SIGTERM.
@@ -70,23 +78,31 @@ void ServiceProcessState::StateData::SignalReady() {
   action.sa_sigaction = SigTermHandler;
   sigemptyset(&action.sa_mask);
   action.sa_flags = SA_SIGINFO;
-  if (sigaction(SIGTERM, &action, &old_action_) == 0) {
-    // If the old_action is not default, somebody else has installed a
-    // a competing handler. Our handler is going to override it so it
-    // won't be called. If this occurs it needs to be fixed.
-    DCHECK_EQ(old_action_.sa_handler, SIG_DFL);
-    set_action_ = true;
+  *success = sigaction(SIGTERM, &action, &old_action_) == 0;
+  if (!*success) {
+    PLOG(ERROR) << "sigaction";
+    signal->Signal();
+    return;
+  }
+
+  // If the old_action is not default, somebody else has installed a
+  // a competing handler. Our handler is going to override it so it
+  // won't be called. If this occurs it needs to be fixed.
+  DCHECK_EQ(old_action_.sa_handler, SIG_DFL);
+  set_action_ = true;
+
 #if defined(OS_LINUX)
-    initializing_lock_.reset();
+  initializing_lock_.reset();
 #endif  // OS_LINUX
 #if defined(OS_MACOSX)
-    if (!WatchExecutable()) {
-      LOG(ERROR) << "WatchExecutable";
-    }
-#endif  // OS_MACOSX
-  } else {
-    PLOG(ERROR) << "sigaction";
+  *success = WatchExecutable();
+  if (!*success) {
+    LOG(ERROR) << "WatchExecutable";
+    signal->Signal();
+    return;
   }
+#endif  // OS_MACOSX
+  signal->Signal();
 }
 
 ServiceProcessState::StateData::~StateData() {
@@ -136,9 +152,15 @@ bool ServiceProcessState::SignalReady(
     PLOG(ERROR) << "pipe";
     return false;
   }
+  base::WaitableEvent signal_ready(true, false);
+  bool success = false;
+
   message_loop_proxy->PostTask(FROM_HERE,
-      NewRunnableMethod(state_, &ServiceProcessState::StateData::SignalReady));
-  return true;
+      NewRunnableMethod(state_, &ServiceProcessState::StateData::SignalReady,
+                        &signal_ready,
+                        &success));
+  signal_ready.Wait();
+  return success;
 }
 
 void ServiceProcessState::TearDownState() {
diff --git a/chrome/common/service_process_util_posix.h b/chrome/common/service_process_util_posix.h
index d053de4..ba12920 100644
--- a/chrome/common/service_process_util_posix.h
+++ b/chrome/common/service_process_util_posix.h
@@ -29,6 +29,10 @@ CFDictionaryRef CreateServiceProcessLaunchdPlist(CommandLine* cmd_line,
                                                  bool for_auto_launch);
 #endif  // OS_MACOSX
 
+namespace base {
+class WaitableEvent;
+}
+
 // Watches for |kShutDownMessage| to be written to the file descriptor it is
 // watching. When it reads |kShutDownMessage|, it performs |shutdown_task_|.
 // Used here to monitor the socket listening to g_signal_socket.
@@ -57,7 +61,7 @@ struct ServiceProcessState::StateData
 
   // WatchFileDescriptor needs to be set up by the thread that is going
   // to be monitoring it.
-  void SignalReady();
+  void SignalReady(base::WaitableEvent* signal, bool* success);
 
 
   // TODO(jhawkins): Either make this a class or rename these public member
diff --git a/chrome/common/service_process_util_unittest.cc b/chrome/common/service_process_util_unittest.cc
index 840c1a5..e542f09 100644
--- a/chrome/common/service_process_util_unittest.cc
+++ b/chrome/common/service_process_util_unittest.cc
@@ -493,8 +493,7 @@ TEST_F(ServiceProcessStateFileManipulationTest, DeleteFile) {
   ASSERT_TRUE(mock_launchd()->delete_called());
 }
 
-// Flaky on mac.  http://crbug.com/77217
-TEST_F(ServiceProcessStateFileManipulationTest, FLAKY_DeleteBundle) {
+TEST_F(ServiceProcessStateFileManipulationTest, DeleteBundle) {
   GetIOMessageLoopProxy()->PostTask(
       FROM_HERE,
       NewRunnableFunction(&DeleteFunc, bundle_path()));