John, please review everything.

agl, please review changes to the waitable_event_watcher code.

Multiple sync channels if used in the same listener thread could result in calls completing in the
wrong context at times. This happens if there are nested calls on these sync channels, i.e
1. Call from client 1 on channel 1 to server 1. Message pumping is enabled for the corresponding message
2. Call from client 2 on channel 2 to server 2, Message pumping is enabled for the corresponding message

Now if a reply for 1 arrives, it should be queued until reply 2 arrives. This does not happen which
causes 2 to terminate prematurely leading to crashes, 1 waiting indefinitely at times, etc.

The fix for this issue is to maintain a local global stack for the send done event watcher object.
The global object is in the form of a TLS. This ensures that we only watch for completed events
on the outermost sync channel.

The changes in the Waitable event watcher object are to return the current delegate which is needed
to start watching the old send watcher once we are done and to ensure that the event member is set even
if it was already signaled when StartWatching was called.

I have added a unit test in ipc_tests for this case. I removed the old QueuedReply based unit tests as
they did not test the actual nested call case.

While debugging these issues I also found some issues in BrowserRenderProcessHost::OnChannelError where
it would delete a sync channel while it was in use. Based on a discussion with jam we decided to DeleteSoon
the sync channel pointer. However this broke some browser ui tests which relied on the timing of the OnChannelError notification. 
We decided to leave the existing code as is for now. I removed the DCHECK on channel as it would fire repeatedly if the channel died
while multiple sync calls were waiting to complete leading to OnChannelError firing multiple times.

Bug=24427

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

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

1 files changed, 5 insertions, 0 deletions

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)