diff options
author | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-18 04:26:02 +0000 |
---|---|---|
committer | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-18 04:26:02 +0000 |
commit | d0b8e5f9bff9553c75bf78bdebeff855db03ebc9 (patch) | |
tree | a92d7037f4c91b353f6214015d43e445438db4ff /chrome_frame/custom_sync_call_context.h | |
parent | 898183ccf5e387abbcd4183e8fa3af0c7ba92e0a (diff) | |
download | chromium_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/custom_sync_call_context.h')
-rw-r--r-- | chrome_frame/custom_sync_call_context.h | 123 |
1 files changed, 123 insertions, 0 deletions
diff --git a/chrome_frame/custom_sync_call_context.h b/chrome_frame/custom_sync_call_context.h new file mode 100644 index 0000000..fcb786a --- /dev/null +++ b/chrome_frame/custom_sync_call_context.h @@ -0,0 +1,123 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +#ifndef CHROME_FRAME_CUSTOM_SYNC_CALL_CONTEXT_H_ +#define CHROME_FRAME_CUSTOM_SYNC_CALL_CONTEXT_H_ + +#include <vector> +#include "base/ref_counted.h" +#include "chrome_frame/sync_msg_reply_dispatcher.h" +#include "chrome_frame/chrome_frame_automation.h" +#include "ipc/ipc_sync_message.h" + +// TODO(ananta) +// Move the implementations of these classes to the source file. + +// Class that maintains context during the async load/install extension +// operation. When done, InstallExtensionComplete is posted back to the UI +// thread so that the users of ChromeFrameAutomationClient can be notified. +class InstallExtensionContext + : public SyncMessageReplyDispatcher::SyncMessageCallContext { + public: + typedef Tuple1<AutomationMsg_ExtensionResponseValues> output_type; + + InstallExtensionContext(ChromeFrameAutomationClient* client, + const FilePath& crx_path, void* user_data) : client_(client), + crx_path_(crx_path), user_data_(user_data) { + } + + ~InstallExtensionContext() { + } + + void Completed(AutomationMsg_ExtensionResponseValues res) { + client_->PostTask(FROM_HERE, NewRunnableMethod(client_.get(), + &ChromeFrameAutomationClient::InstallExtensionComplete, crx_path_, + user_data_, res)); + } + + private: + scoped_refptr<ChromeFrameAutomationClient> client_; + FilePath crx_path_; + void* user_data_; +}; + +// Class that maintains context during the async retrieval of fetching the +// list of enabled extensions. When done, GetEnabledExtensionsComplete is +// posted back to the UI thread so that the users of +// ChromeFrameAutomationClient can be notified. +class GetEnabledExtensionsContext + : public SyncMessageReplyDispatcher::SyncMessageCallContext { + public: + typedef Tuple1<std::vector<FilePath> > output_type; + + GetEnabledExtensionsContext( + ChromeFrameAutomationClient* client, void* user_data) : client_(client), + user_data_(user_data) { + extension_directories_ = new std::vector<FilePath>(); + } + + ~GetEnabledExtensionsContext() { + // ChromeFrameAutomationClient::GetEnabledExtensionsComplete takes + // ownership of extension_directories_. + } + + std::vector<FilePath>* extension_directories() { + return extension_directories_; + } + + void Completed( + std::vector<FilePath> result) { + (*extension_directories_) = result; + client_->PostTask(FROM_HERE, NewRunnableMethod(client_.get(), + &ChromeFrameAutomationClient::GetEnabledExtensionsComplete, + user_data_, extension_directories_)); + } + + private: + scoped_refptr<ChromeFrameAutomationClient> client_; + std::vector<FilePath>* extension_directories_; + void* user_data_; +}; + +// Class that maintains contextual information for the create and connect +// external tab operations. +class CreateExternalTabContext + : public SyncMessageReplyDispatcher::SyncMessageCallContext { + public: + typedef Tuple3<HWND, HWND, int> output_type; + explicit CreateExternalTabContext(ChromeFrameAutomationClient* client) + : client_(client) { + } + + void Completed(HWND chrome_window, HWND tab_window, int tab_handle) { + client_->PostTask(FROM_HERE, + NewRunnableMethod(client_.get(), + &ChromeFrameAutomationClient::CreateExternalTabComplete, + chrome_window, tab_window, tab_handle)); + } + + private: + scoped_refptr<ChromeFrameAutomationClient> client_; +}; + +// This class maintains context information for the BeginNavigate operations +// pertaining to the external tab. +class BeginNavigateContext + : public SyncMessageReplyDispatcher::SyncMessageCallContext { + public: + explicit BeginNavigateContext(ChromeFrameAutomationClient* client) + : client_(client) {} + + typedef Tuple1<AutomationMsg_NavigationResponseValues> output_type; + + void Completed(AutomationMsg_NavigationResponseValues response) { + client_->BeginNavigateCompleted(response); + } + + private: + scoped_refptr<ChromeFrameAutomationClient> client_; +}; + +#endif // CHROME_FRAME_CUSTOM_SYNC_CALL_CONTEXT_H_ + + |