In ChromeFrame the ChromeFrameAutomationProxy object is created on the background proxy channel thread and is accessed

from the UI thread, the proxy channel thread and the IPC thread. This leads to a race condition when ChromeFrame is being
torn down which occurs because the ChromeFrameAutomationProxy pointer is being set to NULL in the UI thread/deleted in
the proxy background thread while it could be accessed while processing a callback in the IPC thread thus causing a crash.

Fix is to ensure that the IPC thread does not access the ChromeFrameAutomationProxy pointer. To achieve this the callbacks
are now individual context objects which when invoked forward the actions to the ChromeFrameAutomationClient object.

The CreateExternalTab and ConnectExternalTab callbacks now complete their actions on the UI thread.

While at this based on a discussion and lot of help from Stoyan we decided to clean up the sync message dispatching code
used by ChromeFrame by having callbacks now derive from a SyncMessageCallContext class to ensure that these get cleaned
up correctly in all cases. For e.g. if we don't receive a response for a message, etc and thus enable them to present
a consistent interface to be invoked when we receive a response for a IPc message.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=44245

Bug=44245

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

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

1 files changed, 47 insertions, 25 deletions

diff --git a/chrome_frame/sync_msg_reply_dispatcher.h b/chrome_frame/sync_msg_reply_dispatcher.h
index 724c007..7d47bd1 100644
--- a/chrome_frame/sync_msg_reply_dispatcher.h
+++ b/chrome_frame/sync_msg_reply_dispatcher.h
@@ -23,14 +23,22 @@
 // cases where you care about the return values of synchronous messages.
 //
 // Sample usage pattern:
+// Define a class which inherits from SyncMessageCallContext which specifies
+// the output_type tuple and has a Completed member function.
+// class SampleContext
+//     : public SyncMessageReplyDispatcher::SyncMessageCallContext {
+// public:
+//  typedef Tuple1<int> output_type;
+//  void Completed(int arg) {}
+// };
 //
 // // Add handling for desired message types.
 // class SyncMessageReplyDispatcherImpl : public SyncMessageReplyDispatcher {
 //   virtual bool HandleMessageType(const IPC::Message& msg,
-//                                  const MessageSent& origin) {
-//    switch (origin.type) {
+//                                  SyncMessageReplyDispatcher* context) {
+//    switch (context->message_type()) {
 //      case AutomationMsg_CreateExternalTab::ID:
-//        InvokeCallback<Tuple3<HWND, HWND, int> >(msg, origin);
+//        InvokeCallback<CreateExternalTabContext>(msg, context);
 //        break;
 //     [HANDLING FOR OTHER EXPECTED MESSAGE TYPES]
 //   }
@@ -40,29 +48,42 @@
 // IPC::SyncChannel channel_;
 // channel_.AddFilter(new SyncMessageReplyDispatcherImpl());
 //
-// sync_->Push(msg, NewCallback(this, CallbackMethod, this);
+// sync_->Push(msg, new SampleContext, this);
 // channel_->ChannelProxy::Send(msg);
 //
 class SyncMessageReplyDispatcher : public IPC::ChannelProxy::MessageFilter {
  public:
+  class SyncMessageCallContext {
+   public:
+    SyncMessageCallContext()
+        : id_(0),
+          message_type_(0),
+          key_(NULL) {}
+
+    virtual ~SyncMessageCallContext() {}
+
+    uint32 message_type() const {
+      return message_type_;
+    }
+
+   private:
+    int id_;
+    uint32 message_type_;
+    void* key_;
+
+    friend class SyncMessageReplyDispatcher;
+  };
+
   SyncMessageReplyDispatcher() {}
-  void Push(IPC::SyncMessage* msg, void* callback, void* key);
+  void Push(IPC::SyncMessage* msg, SyncMessageCallContext* context,
+            void* key);
   void Cancel(void* key);
 
  protected:
-  struct MessageSent {
-    MessageSent() {}
-    MessageSent(int id, uint32 type, void* callback, void* key)
-      : id(id), callback(callback), type(type), key(key) {}
-    int id;
-    void* callback;
-    void* key;
-    uint32 type;
-  };
+  typedef std::deque<SyncMessageCallContext*> PendingSyncMessageQueue;
 
-  typedef std::deque<MessageSent> PendingSyncMessageQueue;
+  SyncMessageCallContext* GetContext(const IPC::Message& msg);
 
-  bool Pop(const IPC::Message& msg, MessageSent* t);
   virtual bool OnMessageReceived(const IPC::Message& msg);
 
   // Child classes must implement a handler for the message types they are
@@ -70,21 +91,22 @@ class SyncMessageReplyDispatcher : public IPC::ChannelProxy::MessageFilter {
   // to any of the sync messages you are handling, then you don't have to
   // implement this.
   virtual bool HandleMessageType(const IPC::Message& msg,
-                                 const MessageSent& origin);
+                                 SyncMessageCallContext* context);
 
   template <typename T>
-  void InvokeCallback(const IPC::Message& msg, const MessageSent& origin) {
-    // Ensure we delete the callback
-    scoped_ptr<CallbackRunner<T> > c(
-        reinterpret_cast<CallbackRunner<T>*>(origin.callback));
-    if (origin.key == NULL) {  // Canceled
+  void InvokeCallback(const IPC::Message& msg,
+                      SyncMessageCallContext* call_context) {
+    if (!call_context || !call_context->key_) {
+      NOTREACHED() << "Invalid context parameter";
       return;
     }
 
-    T tmp;  // Acts as "initializer" for output parameters.
-    IPC::ParamDeserializer<T> deserializer(tmp);
+    T* context = static_cast<T*>(call_context);
+    T::output_type tmp;  // Acts as "initializer" for output parameters.
+    IPC::ParamDeserializer<T::output_type> deserializer(tmp);
     if (deserializer.MessageReplyDeserializer::SerializeOutputParameters(msg)) {
-      c->RunWithParams(deserializer.out_);
+      DispatchToMethod(context, &T::Completed, deserializer.out_);
+      delete context;
     } else {
       // TODO(stoyan): How to handle errors?
     }