7 files changed, 150 insertions, 80 deletions

diff --git a/base/waitable_event_watcher.h b/base/waitable_event_watcher.h
index a158a7d..c6309f4 100644
--- a/base/waitable_event_watcher.h
+++ b/base/waitable_event_watcher.h
@@ -106,6 +106,13 @@ class WaitableEventWatcher
   // ---------------------------------------------------------------------------
   WaitableEvent* GetWatchedEvent();
 
+  // ---------------------------------------------------------------------------
+  // Return the delegate, or NULL if there is no delegate.
+  // ---------------------------------------------------------------------------
+  Delegate* delegate() {
+    return delegate_;
+  }
+
  private:
   WaitableEvent* event_;
 
@@ -131,7 +138,6 @@ class WaitableEventWatcher
 
   void OnObjectSignaled();
 
-  Delegate* delegate_;
   ObjectWatcherHelper helper_;
   ObjectWatcher watcher_;
 #else
@@ -146,6 +152,8 @@ class WaitableEventWatcher
   AsyncCallbackTask* callback_task_;
   scoped_refptr<WaitableEvent::WaitableEventKernel> kernel_;
 #endif
+
+  Delegate* delegate_;
 };
 
 }  // namespace base
diff --git a/base/waitable_event_watcher_posix.cc b/base/waitable_event_watcher_posix.cc
index 1910c5f..eca2cb3 100644
--- a/base/waitable_event_watcher_posix.cc
+++ b/base/waitable_event_watcher_posix.cc
@@ -122,7 +122,8 @@ WaitableEventWatcher::WaitableEventWatcher()
     : event_(NULL),
       message_loop_(NULL),
       cancel_flag_(NULL),
-      callback_task_(NULL) {
+      callback_task_(NULL),
+      delegate_(NULL) {
 }
 
 WaitableEventWatcher::~WaitableEventWatcher() {
@@ -159,6 +160,9 @@ bool WaitableEventWatcher::StartWatching
 
   AutoLock locked(kernel->lock_);
 
+  delegate_ = delegate;
+  event_ = event;
+
   if (kernel->signaled_) {
     if (!kernel->manual_reset_)
       kernel->signaled_ = false;
@@ -172,7 +176,6 @@ bool WaitableEventWatcher::StartWatching
   message_loop_ = current_ml;
   current_ml->AddDestructionObserver(this);
 
-  event_ = event;
   kernel_ = kernel;
   waiter_ = new AsyncWaiter(current_ml, callback_task_, cancel_flag_);
   event->Enqueue(waiter_);
@@ -181,6 +184,8 @@ bool WaitableEventWatcher::StartWatching
 }
 
 void WaitableEventWatcher::StopWatching() {
+  delegate_ = NULL;
+
   if (message_loop_) {
     message_loop_->RemoveDestructionObserver(this);
     message_loop_ = NULL;
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 2d55eca..d395f08 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -889,7 +889,11 @@ void BrowserRenderProcessHost::BadMessageTerminateProcess(
 void BrowserRenderProcessHost::OnChannelError() {
   // Our child process has died.  If we didn't expect it, it's a crash.
   // In any case, we need to let everyone know it's gone.
-  DCHECK(channel_.get());
+  // The OnChannelError notification can fire multiple times due to nested sync
+  // calls to a renderer. If we don't have a valid channel here it means we
+  // already handled the error.
+  if (!channel_.get())
+    return;
 
   bool child_exited;
   bool did_crash;
diff --git a/ipc/ipc_sync_channel.cc b/ipc/ipc_sync_channel.cc
index de9b434a..c5ec446 100644
--- a/ipc/ipc_sync_channel.cc
+++ b/ipc/ipc_sync_channel.cc
@@ -148,6 +148,14 @@ class SyncChannel::ReceivedSyncMsgQueue :
     }
   }
 
+  base::WaitableEventWatcher* top_send_done_watcher() {
+    return top_send_done_watcher_;
+  }
+
+  void set_top_send_done_watcher(base::WaitableEventWatcher* watcher) {
+    top_send_done_watcher_ = watcher;
+  }
+
  private:
   // See the comment in SyncChannel::SyncChannel for why this event is created
   // as manual reset.
@@ -155,7 +163,8 @@ class SyncChannel::ReceivedSyncMsgQueue :
       dispatch_event_(true, false),
       listener_message_loop_(MessageLoop::current()),
       task_pending_(false),
-      listener_count_(0) {
+      listener_count_(0),
+      top_send_done_watcher_(NULL) {
   }
 
   // Holds information about a queued synchronous message or reply.
@@ -178,6 +187,11 @@ class SyncChannel::ReceivedSyncMsgQueue :
   Lock message_lock_;
   bool task_pending_;
   int listener_count_;
+
+  // The current send done event watcher for this thread. Used to maintain
+  // a local global stack of send done watchers to ensure that nested sync
+  // message loops complete correctly.
+  base::WaitableEventWatcher* top_send_done_watcher_;
 };
 
 base::LazyInstance<base::ThreadLocalPointer<SyncChannel::ReceivedSyncMsgQueue> >
@@ -424,15 +438,39 @@ void SyncChannel::WaitForReply(WaitableEvent* pump_messages_event) {
 }
 
 void SyncChannel::WaitForReplyWithNestedMessageLoop() {
-  WaitableEvent* old_done_event = send_done_watcher_.GetWatchedEvent();
-  send_done_watcher_.StopWatching();
-  send_done_watcher_.StartWatching(sync_context()->GetSendDoneEvent(), this);
+  base::WaitableEventWatcher send_done_watcher;
+
+  ReceivedSyncMsgQueue* sync_msg_queue = sync_context()->received_sync_msgs();
+  DCHECK(sync_msg_queue != NULL);
+
+  base::WaitableEventWatcher* old_send_done_event_watcher =
+      sync_msg_queue->top_send_done_watcher();
+
+  base::WaitableEventWatcher::Delegate* old_delegate = NULL;
+  base::WaitableEvent* old_event = NULL;
+
+  // Maintain a local global stack of send done delegates to ensure that
+  // nested sync calls complete in the correct sequence, i.e. the
+  // outermost call completes first, etc.
+  if (old_send_done_event_watcher) {
+    old_delegate = old_send_done_event_watcher->delegate();
+    old_event = old_send_done_event_watcher->GetWatchedEvent();
+    old_send_done_event_watcher->StopWatching();
+  }
+
+  sync_msg_queue->set_top_send_done_watcher(&send_done_watcher);
+
+  send_done_watcher.StartWatching(sync_context()->GetSendDoneEvent(), this);
   bool old_state = MessageLoop::current()->NestableTasksAllowed();
+
   MessageLoop::current()->SetNestableTasksAllowed(true);
   MessageLoop::current()->Run();
   MessageLoop::current()->SetNestableTasksAllowed(old_state);
-  if (old_done_event)
-    send_done_watcher_.StartWatching(old_done_event, this);
+
+  sync_msg_queue->set_top_send_done_watcher(old_send_done_event_watcher);
+  if (old_send_done_event_watcher) {
+    old_send_done_event_watcher->StartWatching(old_event, old_delegate);
+  }
 }
 
 void SyncChannel::OnWaitableEventSignaled(WaitableEvent* event) {
diff --git a/ipc/ipc_sync_channel.h b/ipc/ipc_sync_channel.h
index bfc9eac..1c7360d 100644
--- a/ipc/ipc_sync_channel.h
+++ b/ipc/ipc_sync_channel.h
@@ -92,6 +92,10 @@ class SyncChannel : public ChannelProxy,
 
     base::WaitableEvent* shutdown_event() { return shutdown_event_; }
 
+    ReceivedSyncMsgQueue* received_sync_msgs() {
+      return received_sync_msgs_;
+    }
+
    private:
     // IPC::ChannelProxy methods that we override.
 
@@ -151,7 +155,6 @@ class SyncChannel : public ChannelProxy,
   bool sync_messages_with_no_timeout_allowed_;
 
   // Used to signal events between the IPC and listener threads.
-  base::WaitableEventWatcher send_done_watcher_;
   base::WaitableEventWatcher dispatch_watcher_;
 
   DISALLOW_EVIL_CONSTRUCTORS(SyncChannel);
diff --git a/ipc/ipc_sync_channel_unittest.cc b/ipc/ipc_sync_channel_unittest.cc
index f6bf10e..3219000 100644
--- a/ipc/ipc_sync_channel_unittest.cc
+++ b/ipc/ipc_sync_channel_unittest.cc
@@ -134,6 +134,10 @@ class Worker : public Channel::Listener, public Message::Sender {
     Send(reply_msg);
   }
 
+  virtual void OnNestedTestMsg(Message* reply_msg) {
+    NOTREACHED();
+  }
+
  private:
   base::Thread* ListenerThread() {
     return overrided_thread_ ? overrided_thread_ : &listener_thread_;
@@ -179,6 +183,8 @@ class Worker : public Channel::Listener, public Message::Sender {
      IPC_MESSAGE_HANDLER_DELAY_REPLY(SyncChannelTestMsg_Double, OnDoubleDelay)
      IPC_MESSAGE_HANDLER_DELAY_REPLY(SyncChannelTestMsg_AnswerToLife,
                                      OnAnswerDelay)
+     IPC_MESSAGE_HANDLER_DELAY_REPLY(SyncChannelNestedTestMsg_String,
+                                     OnNestedTestMsg)
     IPC_END_MESSAGE_MAP()
   }
 
@@ -640,100 +646,101 @@ TEST_F(IPCSyncChannelTest, Multiple) {
 
 namespace {
 
-class QueuedReplyServer1 : public Worker {
+// This class provides server side functionality to test the case where
+// multiple sync channels are in use on the same thread on the client and
+// nested calls are issued.
+class QueuedReplyServer : public Worker {
  public:
-  QueuedReplyServer1(bool pump_during_send)
-    : Worker("test_channel1", Channel::MODE_SERVER),
-      pump_during_send_(pump_during_send) { }
-  void Run() {
-    SendDouble(pump_during_send_, true);
-    Done();
-  }
-
-  bool pump_during_send_;
-};
-
-class QueuedReplyClient1 : public Worker {
- public:
-  QueuedReplyClient1(WaitableEvent* client1_msg_received,
-                     WaitableEvent* server2_can_reply) :
-      Worker("test_channel1", Channel::MODE_CLIENT),
-      client1_msg_received_(client1_msg_received),
-      server2_can_reply_(server2_can_reply) { }
-
-  void OnDouble(int in, int* out) {
-    client1_msg_received_->Signal();
-    *out = in * 2;
-    server2_can_reply_->Wait();
+   QueuedReplyServer(base::Thread* listener_thread,
+                     const std::string& channel_name,
+                     const std::string& reply_text)
+      : Worker(channel_name, Channel::MODE_SERVER),
+        reply_text_(reply_text) {
+    Worker::OverrideThread(listener_thread);
+  }
+
+  virtual void OnNestedTestMsg(Message* reply_msg) {
+    LOG(INFO) << __FUNCTION__ << " Sending reply: "
+              << reply_text_.c_str();
+    SyncChannelNestedTestMsg_String::WriteReplyParams(
+        reply_msg, reply_text_);
+    Send(reply_msg);
     Done();
   }
 
  private:
-  WaitableEvent *client1_msg_received_, *server2_can_reply_;
+  std::string reply_text_;
 };
 
-class QueuedReplyServer2 : public Worker {
+// The QueuedReplyClient class provides functionality to test the case where
+// multiple sync channels are in use on the same thread and they make nested
+// sync calls, i.e. while the first channel waits for a response it makes a
+// sync call on another channel.
+// The callstack should unwind correctly, i.e. the outermost call should
+// complete first, and so on.
+class QueuedReplyClient : public Worker {
  public:
-  explicit QueuedReplyServer2(WaitableEvent* server2_can_reply) :
-      Worker("test_channel2", Channel::MODE_SERVER),
-      server2_can_reply_(server2_can_reply) { }
-
-  void OnAnswer(int* result) {
-    server2_can_reply_->Signal();
-
-    // give client1's reply time to reach the server listener thread
-    PlatformThread::Sleep(200);
-
-    *result = 42;
-    Done();
+  QueuedReplyClient(base::Thread* listener_thread,
+                    const std::string& channel_name,
+                    const std::string& expected_text,
+                    bool pump_during_send)
+      : Worker(channel_name, Channel::MODE_CLIENT),
+        expected_text_(expected_text),
+        pump_during_send_(pump_during_send) {
+    Worker::OverrideThread(listener_thread);
   }
 
-  WaitableEvent *server2_can_reply_;
-};
-
-class QueuedReplyClient2 : public Worker {
- public:
-  explicit QueuedReplyClient2(
-    WaitableEvent* client1_msg_received, bool pump_during_send)
-    : Worker("test_channel2", Channel::MODE_CLIENT),
-      client1_msg_received_(client1_msg_received),
-      pump_during_send_(pump_during_send){ }
+  virtual void Run() {
+    std::string response;
+    SyncMessage* msg = new SyncChannelNestedTestMsg_String(&response);
+    if (pump_during_send_)
+      msg->EnableMessagePumping();
+    bool result = Send(msg);
+    DCHECK(result);
+    DCHECK(response == expected_text_);
 
-  void Run() {
-    client1_msg_received_->Wait();
-    SendAnswerToLife(pump_during_send_, base::kNoTimeout, true);
+    LOG(INFO) << __FUNCTION__ << " Received reply: "
+              << response.c_str();
     Done();
   }
 
-  WaitableEvent *client1_msg_received_;
+ private:
   bool pump_during_send_;
+  std::string expected_text_;
 };
 
-void QueuedReply(bool server_pump, bool client_pump) {
+void QueuedReply(bool client_pump) {
   std::vector<Worker*> workers;
 
-  // A shared worker thread so that server1 and server2 run on one thread.
-  base::Thread worker_thread("QueuedReply");
-  ASSERT_TRUE(worker_thread.Start());
+  // A shared worker thread for servers
+  base::Thread server_worker_thread("QueuedReply_ServerListener");
+  ASSERT_TRUE(server_worker_thread.Start());
 
-  WaitableEvent client1_msg_received(false, false);
-  WaitableEvent server2_can_reply(false, false);
+  base::Thread client_worker_thread("QueuedReply_ClientListener");
+  ASSERT_TRUE(client_worker_thread.Start());
 
   Worker* worker;
 
-  worker = new QueuedReplyServer2(&server2_can_reply);
-  worker->OverrideThread(&worker_thread);
+  worker = new QueuedReplyServer(&server_worker_thread,
+                                 "QueuedReply_Server1",
+                                 "Got first message");
   workers.push_back(worker);
 
-  worker = new QueuedReplyClient2(&client1_msg_received, client_pump);
+  worker = new QueuedReplyServer(&server_worker_thread,
+                                 "QueuedReply_Server2",
+                                 "Got second message");
   workers.push_back(worker);
 
-  worker = new QueuedReplyServer1(server_pump);
-  worker->OverrideThread(&worker_thread);
+  worker = new QueuedReplyClient(&client_worker_thread,
+                                 "QueuedReply_Server1",
+                                 "Got first message",
+                                 client_pump);
   workers.push_back(worker);
 
-  worker = new QueuedReplyClient1(
-      &client1_msg_received, &server2_can_reply);
+  worker = new QueuedReplyClient(&client_worker_thread,
+                                 "QueuedReply_Server2",
+                                 "Got second message",
+                                 client_pump);
   workers.push_back(worker);
 
   RunTest(workers);
@@ -745,11 +752,11 @@ void QueuedReply(bool server_pump, bool client_pump) {
 // synchronous messages.  This tests that if during the response to another
 // message the reply to the original messages comes, it is queued up correctly
 // and the original Send is unblocked later.
+// We also test that the send call stacks unwind correctly when the channel
+// pumps messages while waiting for a response.
 TEST_F(IPCSyncChannelTest, QueuedReply) {
-  QueuedReply(false, false);
-  QueuedReply(false, true);
-  QueuedReply(true, false);
-  QueuedReply(true, true);
+  QueuedReply(false);
+  QueuedReply(true);
 }
 
 //-----------------------------------------------------------------------------
diff --git a/ipc/ipc_sync_message_unittest.h b/ipc/ipc_sync_message_unittest.h
index 06c9af0..f4e98c1 100644
--- a/ipc/ipc_sync_message_unittest.h
+++ b/ipc/ipc_sync_message_unittest.h
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <string>
+
 #include "ipc/ipc_message_macros.h"
 
 IPC_BEGIN_MESSAGES(Test)
@@ -14,6 +16,9 @@ IPC_BEGIN_MESSAGES(Test)
                               int /* in */,
                               int /* out */)
 
+  IPC_SYNC_MESSAGE_CONTROL0_1(SyncChannelNestedTestMsg_String,
+                              std::string)
+
   // out1 is false
   IPC_SYNC_MESSAGE_CONTROL0_1(Msg_C_0_1, bool)