Revert 79307 - 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

TBR=dmaclach@chromium.org
Review URL: http://codereview.chromium.org/6736004

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

3 files changed, 21 insertions, 46 deletions

diff --git a/chrome/common/service_process_util_posix.cc b/chrome/common/service_process_util_posix.cc
index bce541a..5592edf 100644
--- a/chrome/common/service_process_util_posix.cc
+++ b/chrome/common/service_process_util_posix.cc
@@ -7,7 +7,6 @@
 #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;
@@ -59,18 +58,11 @@ ServiceProcessState::StateData::StateData() : set_action_(false) {
   memset(&old_action_, 0, sizeof(old_action_));
 }
 
-void ServiceProcessState::StateData::SignalReady(base::WaitableEvent* signal,
-                                                 bool* success) {
+void ServiceProcessState::StateData::SignalReady() {
   CHECK_EQ(g_signal_socket, -1);
-  CHECK(!signal->IsSignaled());
-  *success = MessageLoopForIO::current()->WatchFileDescriptor(
+  CHECK(MessageLoopForIO::current()->WatchFileDescriptor(
       sockets_[0], true, MessageLoopForIO::WATCH_READ,
-      &watcher_, shut_down_monitor_.get());
-  if (!*success) {
-    LOG(ERROR) << "WatchFileDescriptor";
-    signal->Signal();
-    return;
-  }
+      &watcher_, shut_down_monitor_.get()));
   g_signal_socket = sockets_[1];
 
   // Set up signal handler for SIGTERM.
@@ -78,31 +70,23 @@ void ServiceProcessState::StateData::SignalReady(base::WaitableEvent* signal,
   action.sa_sigaction = SigTermHandler;
   sigemptyset(&action.sa_mask);
   action.sa_flags = SA_SIGINFO;
-  *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 (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;
 #if defined(OS_LINUX)
-  initializing_lock_.reset();
+    initializing_lock_.reset();
 #endif  // OS_LINUX
 #if defined(OS_MACOSX)
-  *success = WatchExecutable();
-  if (!*success) {
-    LOG(ERROR) << "WatchExecutable";
-    signal->Signal();
-    return;
-  }
+    if (!WatchExecutable()) {
+      LOG(ERROR) << "WatchExecutable";
+    }
 #endif  // OS_MACOSX
-  signal->Signal();
+  } else {
+    PLOG(ERROR) << "sigaction";
+  }
 }
 
 ServiceProcessState::StateData::~StateData() {
@@ -152,15 +136,9 @@ 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,
-                        &signal_ready,
-                        &success));
-  signal_ready.Wait();
-  return success;
+      NewRunnableMethod(state_, &ServiceProcessState::StateData::SignalReady));
+  return true;
 }
 
 void ServiceProcessState::TearDownState() {
diff --git a/chrome/common/service_process_util_posix.h b/chrome/common/service_process_util_posix.h
index ba12920..d053de4 100644
--- a/chrome/common/service_process_util_posix.h
+++ b/chrome/common/service_process_util_posix.h
@@ -29,10 +29,6 @@ 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.
@@ -61,7 +57,7 @@ struct ServiceProcessState::StateData
 
   // WatchFileDescriptor needs to be set up by the thread that is going
   // to be monitoring it.
-  void SignalReady(base::WaitableEvent* signal, bool* success);
+  void SignalReady();
 
 
   // 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 e542f09..840c1a5 100644
--- a/chrome/common/service_process_util_unittest.cc
+++ b/chrome/common/service_process_util_unittest.cc
@@ -493,7 +493,8 @@ TEST_F(ServiceProcessStateFileManipulationTest, DeleteFile) {
   ASSERT_TRUE(mock_launchd()->delete_called());
 }
 
-TEST_F(ServiceProcessStateFileManipulationTest, DeleteBundle) {
+// Flaky on mac.  http://crbug.com/77217
+TEST_F(ServiceProcessStateFileManipulationTest, FLAKY_DeleteBundle) {
   GetIOMessageLoopProxy()->PostTask(
       FROM_HERE,
       NewRunnableFunction(&DeleteFunc, bundle_path()));