Make IPC::SyncChannel not duplicate the underlying MessageLoop implementation by pumping messages on its own. This fixes the problem of windowless plugins not painting on right click, and generally makes this class almost ported, other than using a generic version of events/locks.Through this change I've also cleaned up the class and hopefully made it more understandable.

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

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

1 files changed, 62 insertions, 31 deletions

diff --git a/chrome/common/ipc_sync_channel.h b/chrome/common/ipc_sync_channel.h
index 54c9fe9..6c03745 100644
--- a/chrome/common/ipc_sync_channel.h
+++ b/chrome/common/ipc_sync_channel.h
@@ -7,11 +7,12 @@
 
 #include <windows.h>
 #include <string>
-#include <stack>
-#include <queue>
+#include <deque>
 #include "base/basictypes.h"
 #include "base/lock.h"
+#include "base/object_watcher.h"
 #include "base/ref_counted.h"
+#include "base/scoped_handle.h"
 #include "chrome/common/ipc_channel_proxy.h"
 
 namespace IPC {
@@ -24,17 +25,17 @@ class SyncMessage;
 // is more than this object.  If the message loop goes away while this object
 // is running and it's used to send a message, then it will use the invalid
 // message loop pointer to proxy it to the ipc thread.
-class SyncChannel : public ChannelProxy {
+class SyncChannel : public ChannelProxy,
+                    public base::ObjectWatcher::Delegate {
  public:
   SyncChannel(const std::wstring& channel_id, Channel::Mode mode,
               Channel::Listener* listener, MessageFilter* filter,
               MessageLoop* ipc_message_loop, bool create_pipe_now,
-              HANDLE shutdown_handle);
+              HANDLE shutdown_event);
   ~SyncChannel();
 
   virtual bool Send(Message* message);
   virtual bool SendWithTimeout(Message* message, int timeout_ms);
-  bool UnblockListener(Message* message);
 
   // Whether we allow sending messages with no time-out.
   void set_sync_messages_with_no_timeout_allowed(bool value) {
@@ -48,74 +49,104 @@ class SyncChannel : public ChannelProxy {
   // SyncContext holds the per object data for SyncChannel, so that SyncChannel
   // can be deleted while it's being used in a different thread.  See
   // ChannelProxy::Context for more information.
-  class SyncContext : public Context {
+  class SyncContext : public Context,
+                      public base::ObjectWatcher::Delegate {
    public:
     SyncContext(Channel::Listener* listener,
                 MessageFilter* filter,
-                MessageLoop* ipc_thread);
+                MessageLoop* ipc_thread,
+                HANDLE shutdown_event);
 
     ~SyncContext();
 
     // Adds information about an outgoing sync message to the context so that
-    // we know how to deserialize the reply.  Returns a handle that's set when
-    // the reply has arrived.
-    HANDLE Push(IPC::SyncMessage* sync_msg);
+    // we know how to deserialize the reply.
+    void Push(IPC::SyncMessage* sync_msg);
 
-    // Returns true if the reply message was deserialized without any errors,
-    // or false otherwise.
-    bool reply_deserialize_result() { return reply_deserialize_result_; }
+    // Cleanly remove the top deserializer (and throw it away).  Returns the
+    // result of the Send call for that message.
+    bool Pop();
+
+    // Returns an event that's set when the send is complete, timed out or the
+    // process shut down.
+    HANDLE GetSendDoneEvent();
 
     // Returns an event that's set when an incoming message that's not the reply
     // needs to get dispatched (by calling SyncContext::DispatchMessages).
-    HANDLE blocking_event();
+    HANDLE GetDispatchEvent();
 
     void DispatchMessages();
-    void RemoveListener(Channel::Listener* listener);
 
     // Checks if the given message is blocking the listener thread because of a
     // synchronous send.  If it is, the thread is unblocked and true is returned.
     // Otherwise the function returns false.
-    bool UnblockListener(const Message* msg);
+    bool TryToUnblockListener(const Message* msg);
+
+    // Called on the IPC thread when a sync send that runs a nested message loop
+    // times out.
+    void OnSendTimeout(int message_id);
 
-    // Cleanly remove the top deserializer (and throw it away).
-    void PopDeserializer(bool close_reply_event);
+    HANDLE shutdown_event() { return shutdown_event_; }
 
    private:
-    void OnMessageReceived(const Message& msg);
-    void OnChannelError();
+    // IPC::ChannelProxy methods that we override.
+
+    // Called on the listener thread.
+   virtual void Clear();
+
+    // Called on the IPC thread.
+    virtual void OnMessageReceived(const Message& msg);
+    virtual void OnChannelError();
+    virtual void OnChannelOpened();
+    virtual void OnChannelClosed();
+
+    // Cancels all pending Send calls.
+    void CancelPendingSends();
+
+    // ObjectWatcher::Delegate implementation.
+    virtual void OnObjectSignaled(HANDLE object);
 
     // When sending a synchronous message, this structure contains an object that
     // knows how to deserialize the response.
     struct PendingSyncMsg {
       PendingSyncMsg(int id, IPC::MessageReplyDeserializer* d, HANDLE e) :
-          id(id), deserializer(d), reply_event(e) { }
+          id(id), deserializer(d), done_event(e), send_result(false) { }
       int id;
       IPC::MessageReplyDeserializer* deserializer;
-      HANDLE reply_event;
+      HANDLE done_event;
+      bool send_result;
     };
 
-    typedef std::stack<PendingSyncMsg> PendingSyncMessageQueue;
+    typedef std::deque<PendingSyncMsg> PendingSyncMessageQueue;
     PendingSyncMessageQueue deserializers_;
     Lock deserializers_lock_;
 
-    // This can't be a scoped_refptr because it needs to be released on the
-    // listener thread.
-    ReceivedSyncMsgQueue* received_sync_msgs_;
+    scoped_refptr<ReceivedSyncMsgQueue> received_sync_msgs_;
 
-    bool channel_closed_;
-    bool reply_deserialize_result_;
+    HANDLE shutdown_event_;
+    base::ObjectWatcher shutdown_watcher_;
   };
 
  private:
+  // ObjectWatcher::Delegate implementation.
+  virtual void OnObjectSignaled(HANDLE object);
+
   SyncContext* sync_context() { return reinterpret_cast<SyncContext*>(context()); }
 
-  // Copy of shutdown event that we get in constructor.
-  HANDLE shutdown_event_;
+  // Both these functions wait for a reply, timeout or process shutdown.  The
+  // latter one also runs a nested message loop in the meantime.
+  void WaitForReply(HANDLE pump_messages_event);
 
-  std::stack<HANDLE> pump_messages_events_;
+  // Runs a nested message loop until a reply arrives, times out, or the process
+  // shuts down.
+  void WaitForReplyWithNestedMessageLoop();
 
   bool sync_messages_with_no_timeout_allowed_;
 
+  // Used to signal events between the IPC and listener threads.
+  base::ObjectWatcher send_done_watcher_;
+  base::ObjectWatcher dispatch_watcher_;
+
   DISALLOW_EVIL_CONSTRUCTORS(SyncChannel);
 };