summaryrefslogtreecommitdiffstats
path: root/chrome_frame/sync_msg_reply_dispatcher.cc
diff options
context:
space:
mode:
authorananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-05-18 04:26:02 +0000
committerananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-05-18 04:26:02 +0000
commitd0b8e5f9bff9553c75bf78bdebeff855db03ebc9 (patch)
treea92d7037f4c91b353f6214015d43e445438db4ff /chrome_frame/sync_msg_reply_dispatcher.cc
parent898183ccf5e387abbcd4183e8fa3af0c7ba92e0a (diff)
downloadchromium_src-d0b8e5f9bff9553c75bf78bdebeff855db03ebc9.zip
chromium_src-d0b8e5f9bff9553c75bf78bdebeff855db03ebc9.tar.gz
chromium_src-d0b8e5f9bff9553c75bf78bdebeff855db03ebc9.tar.bz2
Relanding this fix with the fixes to get chrome frame tests to pass.
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 TBR=stoyan Review URL: http://codereview.chromium.org/2119004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@47494 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome_frame/sync_msg_reply_dispatcher.cc')
-rw-r--r--chrome_frame/sync_msg_reply_dispatcher.cc51
1 files changed, 28 insertions, 23 deletions
diff --git a/chrome_frame/sync_msg_reply_dispatcher.cc b/chrome_frame/sync_msg_reply_dispatcher.cc
index b301698..1b51516 100644
--- a/chrome_frame/sync_msg_reply_dispatcher.cc
+++ b/chrome_frame/sync_msg_reply_dispatcher.cc
@@ -6,56 +6,61 @@
#include "ipc/ipc_sync_message.h"
-void SyncMessageReplyDispatcher::Push(IPC::SyncMessage* msg, void* callback,
+void SyncMessageReplyDispatcher::Push(IPC::SyncMessage* msg,
+ SyncMessageCallContext* context,
void* key) {
- MessageSent pending(IPC::SyncMessage::GetMessageId(*msg),
- msg->type(), callback, key);
+ context->message_type_ = msg->type();
+ context->id_ = IPC::SyncMessage::GetMessageId(*msg);
+ context->key_ = key;
+
AutoLock lock(message_queue_lock_);
- message_queue_.push_back(pending);
+ message_queue_.push_back(context);
}
-bool SyncMessageReplyDispatcher::HandleMessageType(const IPC::Message& msg,
- const MessageSent& origin) {
+bool SyncMessageReplyDispatcher::HandleMessageType(
+ const IPC::Message& msg, SyncMessageCallContext* context) {
return false;
}
bool SyncMessageReplyDispatcher::OnMessageReceived(const IPC::Message& msg) {
- MessageSent origin;
- if (!Pop(msg, &origin)) {
+ SyncMessageCallContext* context = GetContext(msg);
+ // No context e.g. no return values and/or don't care
+ if (!context) {
return false;
}
- // No callback e.g. no return values and/or don't care
- if (origin.callback == NULL)
- return true;
-
- return HandleMessageType(msg, origin);
+ return HandleMessageType(msg, context);
}
void SyncMessageReplyDispatcher::Cancel(void* key) {
DCHECK(key != NULL);
AutoLock lock(message_queue_lock_);
- PendingSyncMessageQueue::iterator it;
- for (it = message_queue_.begin(); it != message_queue_.end(); ++it) {
- if (it->key == key) {
- it->key = NULL;
+ PendingSyncMessageQueue::iterator it = message_queue_.begin();
+ while (it != message_queue_.end()) {
+ SyncMessageCallContext* context = *it;
+ if (context->key_ == key) {
+ it = message_queue_.erase(it);
+ delete context;
+ } else {
+ ++it;
}
}
}
-bool SyncMessageReplyDispatcher::Pop(const IPC::Message& msg, MessageSent* t) {
+SyncMessageReplyDispatcher::SyncMessageCallContext*
+ SyncMessageReplyDispatcher::GetContext(const IPC::Message& msg) {
if (!msg.is_reply())
- return false;
+ return NULL;
int id = IPC::SyncMessage::GetMessageId(msg);
AutoLock lock(message_queue_lock_);
PendingSyncMessageQueue::iterator it;
for (it = message_queue_.begin(); it != message_queue_.end(); ++it) {
- if (it->id == id) {
- *t = *it;
+ SyncMessageCallContext* context = *it;
+ if (context->id_ == id) {
message_queue_.erase(it);
- return true;
+ return context;
}
}
- return false;
+ return NULL;
}