From 37b76dc3b721d4fcad4459af36df4b204b7b630b Mon Sep 17 00:00:00 2001 From: "ananta@chromium.org" Date: Wed, 2 Dec 2009 00:48:55 +0000 Subject: Speculative fix for ChromeFrame crash in bug http://code.google.com/p/chromium/issues/detail?id=29025 The crash occurs while dereferencing the automation channel to send out the SetCookie IPC message on the automation channel to the host browser. Based on what I could see from the crash dump and the code it seems like there could be a scenario where the AutomationResourceContext object could be destroyed while the AutomationCookieStore object is still around and thus ends up with a stale pointer which crashes when dereferenced. Fix is to ensure that all related code paths hold on to a refcounted AutomationResourceContext instance. I will look into whether it is possible to come up with a unit test for this. Bug=29025 Review URL: http://codereview.chromium.org/450020 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33524 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/automation/automation_profile_impl.cc | 11 ++++++----- chrome/browser/automation/automation_profile_impl.h | 3 ++- chrome/browser/external_tab_container.cc | 2 +- chrome/browser/external_tab_container.h | 3 ++- 4 files changed, 11 insertions(+), 8 deletions(-) (limited to 'chrome') diff --git a/chrome/browser/automation/automation_profile_impl.cc b/chrome/browser/automation/automation_profile_impl.cc index 62d9cde..ce0949e 100644 --- a/chrome/browser/automation/automation_profile_impl.cc +++ b/chrome/browser/automation/automation_profile_impl.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "chrome/browser/automation/automation_profile_impl.h" +#include "chrome/browser/automation/automation_resource_message_filter.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/net/chrome_url_request_context.h" #include "chrome/browser/profile.h" @@ -49,7 +50,7 @@ class AutomationURLRequestContext : public ChromeURLRequestContext { class AutomationCookieStore : public net::CookieStore { public: AutomationCookieStore(net::CookieStore* original_cookie_store, - IPC::Message::Sender* automation_client, + AutomationResourceMessageFilter* automation_client, int tab_handle) : original_cookie_store_(original_cookie_store), automation_client_(automation_client), @@ -116,7 +117,7 @@ class AutomationCookieStore : public net::CookieStore { } net::CookieStore* original_cookie_store_; - IPC::Message::Sender* automation_client_; + scoped_refptr automation_client_; int tab_handle_; private: @@ -127,7 +128,7 @@ class Factory : public ChromeURLRequestContextFactory { public: Factory(ChromeURLRequestContextGetter* original_context_getter, Profile* profile, - IPC::Message::Sender* automation_client, + AutomationResourceMessageFilter* automation_client, int tab_handle) : ChromeURLRequestContextFactory(profile), original_context_getter_(original_context_getter), @@ -151,14 +152,14 @@ class Factory : public ChromeURLRequestContextFactory { private: scoped_refptr original_context_getter_; - IPC::Message::Sender* automation_client_; + scoped_refptr automation_client_; int tab_handle_; }; ChromeURLRequestContextGetter* CreateAutomationURLRequestContextForTab( int tab_handle, Profile* profile, - IPC::Message::Sender* automation_client) { + AutomationResourceMessageFilter* automation_client) { ChromeURLRequestContextGetter* original_context = static_cast( profile->GetRequestContext()); diff --git a/chrome/browser/automation/automation_profile_impl.h b/chrome/browser/automation/automation_profile_impl.h index f2bc6c2..5bfba8e 100644 --- a/chrome/browser/automation/automation_profile_impl.h +++ b/chrome/browser/automation/automation_profile_impl.h @@ -9,6 +9,7 @@ class Profile; class ChromeURLRequestContextGetter; +class AutomationResourceMessageFilter; namespace AutomationRequestContext { @@ -17,7 +18,7 @@ namespace AutomationRequestContext { ChromeURLRequestContextGetter* CreateAutomationURLRequestContextForTab( int tab_handle, Profile* profile, - IPC::Message::Sender* automation_client); + AutomationResourceMessageFilter* automation_client); } diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc index ab25698..141a76b 100644 --- a/chrome/browser/external_tab_container.cc +++ b/chrome/browser/external_tab_container.cc @@ -401,7 +401,7 @@ bool ExternalTabContainer::CanDownload(int request_id) { ::AllowSetForegroundWindow(ASFW_ANY); ChromeThread::PostTask(ChromeThread::IO, FROM_HERE, - NewRunnableMethod(automation_resource_message_filter_, + NewRunnableMethod(automation_resource_message_filter_.get(), &AutomationResourceMessageFilter::SendDownloadRequestToHost, 0, tab_handle_, request_id)); } diff --git a/chrome/browser/external_tab_container.h b/chrome/browser/external_tab_container.h index 56317d8..4461f7e 100644 --- a/chrome/browser/external_tab_container.h +++ b/chrome/browser/external_tab_container.h @@ -205,7 +205,8 @@ class ExternalTabContainer : public TabContentsDelegate, scoped_ptr external_context_menu_; // A message filter to load resources via automation - AutomationResourceMessageFilter* automation_resource_message_filter_; + scoped_refptr + automation_resource_message_filter_; // If all the url requests for this tab are to be loaded via automation. bool load_requests_via_automation_; -- cgit v1.1