From 674b382acde1e6f5f8919fff715388a37afcedee Mon Sep 17 00:00:00 2001 From: "ananta@chromium.org" Date: Wed, 4 Aug 2010 04:02:51 +0000 Subject: ChromeFrame currently overrides the request context for intercepting network requests and cookie requests to route them over the automation channel. This adds needless complexity and race conditions between registering a request context for a renderer process as the first one wins. We no longer override the request context in ChromeFrame. For cookie requests we look up the registered render view map and on finding one we route the request over the automation channel. Fixes bug http://code.google.com/p/chromium/issues/detail?id=51103 Bug=51103 Review URL: http://codereview.chromium.org/3036038 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54867 0039d316-1c4b-4281-b951-d872f2087c98 --- .../renderer_host/browser_render_process_host.cc | 6 +-- .../renderer_host/browser_render_process_host.h | 3 +- .../renderer_host/mock_render_process_host.cc | 3 +- .../renderer_host/mock_render_process_host.h | 3 +- chrome/browser/renderer_host/render_process_host.h | 3 +- chrome/browser/renderer_host/render_view_host.cc | 5 +- chrome/browser/renderer_host/render_view_host.h | 3 +- .../renderer_host/resource_message_filter.cc | 54 +++++++++++++--------- .../renderer_host/resource_message_filter.h | 13 +++++- .../renderer_host/test/test_render_view_host.cc | 3 +- .../renderer_host/test/test_render_view_host.h | 3 +- 11 files changed, 55 insertions(+), 44 deletions(-) (limited to 'chrome/browser/renderer_host') diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc index bcd8c2b..796aa710 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.cc +++ b/chrome/browser/renderer_host/browser_render_process_host.cc @@ -247,8 +247,7 @@ BrowserRenderProcessHost::~BrowserRenderProcessHost() { NotificationService::NoDetails()); } -bool BrowserRenderProcessHost::Init(bool is_extensions_process, - URLRequestContextGetter* request_context) { +bool BrowserRenderProcessHost::Init(bool is_extensions_process) { // calling Init() more than once does nothing, this makes it more convenient // for the view host which may not be sure in some cases if (channel_.get()) @@ -271,8 +270,7 @@ bool BrowserRenderProcessHost::Init(bool is_extensions_process, PluginService::GetInstance(), g_browser_process->print_job_manager(), profile(), - widget_helper_, - request_context); + widget_helper_); std::wstring renderer_prefix; #if defined(OS_POSIX) diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h index 23de0315..c72f4d7 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.h +++ b/chrome/browser/renderer_host/browser_render_process_host.h @@ -60,8 +60,7 @@ class BrowserRenderProcessHost : public RenderProcessHost, ~BrowserRenderProcessHost(); // RenderProcessHost implementation (public portion). - virtual bool Init(bool is_extensions_process, - URLRequestContextGetter* request_context); + virtual bool Init(bool is_extensions_process); virtual int GetNextRoutingID(); virtual void CancelResourceRequests(int render_widget_id); virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); diff --git a/chrome/browser/renderer_host/mock_render_process_host.cc b/chrome/browser/renderer_host/mock_render_process_host.cc index 21fa34d..9c8eac3 100644 --- a/chrome/browser/renderer_host/mock_render_process_host.cc +++ b/chrome/browser/renderer_host/mock_render_process_host.cc @@ -20,8 +20,7 @@ MockRenderProcessHost::~MockRenderProcessHost() { delete transport_dib_; } -bool MockRenderProcessHost::Init(bool is_extensions_process, - URLRequestContextGetter* request_context) { +bool MockRenderProcessHost::Init(bool is_extensions_process) { return true; } diff --git a/chrome/browser/renderer_host/mock_render_process_host.h b/chrome/browser/renderer_host/mock_render_process_host.h index 0dc5e02..608821d 100644 --- a/chrome/browser/renderer_host/mock_render_process_host.h +++ b/chrome/browser/renderer_host/mock_render_process_host.h @@ -31,8 +31,7 @@ class MockRenderProcessHost : public RenderProcessHost { int bad_msg_count() const { return bad_msg_count_; } // RenderProcessHost implementation (public portion). - virtual bool Init(bool is_extensions_process, - URLRequestContextGetter* request_context); + virtual bool Init(bool is_extensions_process); virtual int GetNextRoutingID(); virtual void CancelResourceRequests(int render_widget_id); virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); diff --git a/chrome/browser/renderer_host/render_process_host.h b/chrome/browser/renderer_host/render_process_host.h index 82c3572..45b643f 100644 --- a/chrome/browser/renderer_host/render_process_host.h +++ b/chrome/browser/renderer_host/render_process_host.h @@ -149,8 +149,7 @@ class RenderProcessHost : public IPC::Channel::Sender, // be called once before the object can be used, but can be called after // that with no effect. Therefore, if the caller isn't sure about whether // the process has been created, it should just call Init(). - virtual bool Init(bool is_extensions_process, - URLRequestContextGetter* request_context) = 0; + virtual bool Init(bool is_extensions_process) = 0; // Gets the next available routing id. virtual int GetNextRoutingID() = 0; diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index c3a7713..b8a03f4 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -146,15 +146,14 @@ RenderViewHost::~RenderViewHost() { NotificationService::NoDetails()); } -bool RenderViewHost::CreateRenderView( - URLRequestContextGetter* request_context, const string16& frame_name) { +bool RenderViewHost::CreateRenderView(const string16& frame_name) { DCHECK(!IsRenderViewLive()) << "Creating view twice"; // The process may (if we're sharing a process with another host that already // initialized it) or may not (we have our own process or the old process // crashed) have been initialized. Calling Init multiple times will be // ignored, so this is safe. - if (!process()->Init(is_extension_process_, request_context)) + if (!process()->Init(is_extension_process_)) return false; DCHECK(process()->HasConnection()); DCHECK(process()->profile()); diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h index ff5ff94..0fda85e 100644 --- a/chrome/browser/renderer_host/render_view_host.h +++ b/chrome/browser/renderer_host/render_view_host.h @@ -109,8 +109,7 @@ class RenderViewHost : public RenderWidgetHost { // Set up the RenderView child process. Virtual because it is overridden by // TestRenderViewHost. If the |frame_name| parameter is non-empty, it is used // as the name of the new top-level frame. - virtual bool CreateRenderView(URLRequestContextGetter* request_context, - const string16& frame_name); + virtual bool CreateRenderView(const string16& frame_name); // Returns true if the RenderView is active and has not crashed. Virtual // because it is overridden by TestRenderViewHost. diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc index 3d60d80..e28146b 100644 --- a/chrome/browser/renderer_host/resource_message_filter.cc +++ b/chrome/browser/renderer_host/resource_message_filter.cc @@ -19,6 +19,7 @@ #include "base/utf_string_conversions.h" #include "base/worker_pool.h" #include "chrome/browser/appcache/appcache_dispatcher_host.h" +#include "chrome/browser/automation/automation_resource_message_filter.h" #include "chrome/browser/browser_about_handler.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/child_process_security_policy.h" @@ -201,8 +202,7 @@ ResourceMessageFilter::ResourceMessageFilter( PluginService* plugin_service, printing::PrintJobManager* print_job_manager, Profile* profile, - RenderWidgetHelper* render_widget_helper, - URLRequestContextGetter* request_context) + RenderWidgetHelper* render_widget_helper) : Receiver(RENDER_PROCESS, child_id), channel_(NULL), resource_dispatcher_host_(resource_dispatcher_host), @@ -210,7 +210,6 @@ ResourceMessageFilter::ResourceMessageFilter( print_job_manager_(print_job_manager), profile_(profile), ALLOW_THIS_IN_INITIALIZER_LIST(resolve_proxy_msg_helper_(this, NULL)), - request_context_(request_context), media_request_context_(profile->GetRequestContextForMedia()), extensions_request_context_(profile->GetRequestContextForExtensions()), extensions_message_service_(profile->GetExtensionMessageService()), @@ -234,6 +233,8 @@ ResourceMessageFilter::ResourceMessageFilter( ALLOW_THIS_IN_INITIALIZER_LIST(geolocation_dispatcher_host_( GeolocationDispatcherHost::New( this->id(), profile->GetGeolocationPermissionContext()))) { + request_context_ = profile_->GetRequestContext(); + DCHECK(request_context_); DCHECK(media_request_context_); DCHECK(audio_renderer_host_.get()); @@ -560,16 +561,22 @@ void ResourceMessageFilter::OnSetCookie(const IPC::Message& message, ChromeURLRequestContext* context = GetRequestContextForURL(url); SetCookieCompletion* callback = - new SetCookieCompletion(id(), message.routing_id(), url, cookie, context); - - int policy = net::OK; - if (context->cookie_policy()) { - policy = context->cookie_policy()->CanSetCookie( - url, first_party_for_cookies, cookie, callback); - if (policy == net::ERR_IO_PENDING) - return; + new SetCookieCompletion(id(), message.routing_id(), url, cookie, + context); + + // If this render view is associated with an automation channel, aka + // ChromeFrame then we need to set cookies in the external host. + if (!AutomationResourceMessageFilter::SetCookiesForUrl(url, cookie, + callback)) { + int policy = net::OK; + if (context->cookie_policy()) { + policy = context->cookie_policy()->CanSetCookie( + url, first_party_for_cookies, cookie, callback); + if (policy == net::ERR_IO_PENDING) + return; + } + callback->Run(policy); } - callback->Run(policy); } void ResourceMessageFilter::OnGetCookies(const GURL& url, @@ -581,16 +588,20 @@ void ResourceMessageFilter::OnGetCookies(const GURL& url, new GetCookiesCompletion(id(), reply_msg->routing_id(), url, reply_msg, this, context, false); - int policy = net::OK; - if (context->cookie_policy()) { - policy = context->cookie_policy()->CanGetCookies( - url, first_party_for_cookies, callback); - if (policy == net::ERR_IO_PENDING) { - Send(new ViewMsg_SignalCookiePromptEvent()); - return; + // If this render view is associated with an automation channel, aka + // ChromeFrame then we need to retrieve cookies from the external host. + if (!AutomationResourceMessageFilter::GetCookiesForUrl(url, callback)) { + int policy = net::OK; + if (context->cookie_policy()) { + policy = context->cookie_policy()->CanGetCookies( + url, first_party_for_cookies, callback); + if (policy == net::ERR_IO_PENDING) { + Send(new ViewMsg_SignalCookiePromptEvent()); + return; + } } + callback->Run(policy); } - callback->Run(policy); } void ResourceMessageFilter::OnGetRawCookies( @@ -1627,6 +1638,7 @@ GetCookiesCompletion::GetCookiesCompletion(int render_process_id, render_process_id_(render_process_id), render_view_id_(render_view_id), raw_cookies_(raw_cookies) { + set_cookie_store(context_->cookie_store()); } void GetCookiesCompletion::RunWithParams(const Tuple1& params) { @@ -1634,7 +1646,7 @@ void GetCookiesCompletion::RunWithParams(const Tuple1& params) { int result = params.a; std::string cookies; if (result == net::OK) - cookies = context_->cookie_store()->GetCookies(url_); + cookies = cookie_store()->GetCookies(url_); ViewHostMsg_GetCookies::WriteReplyParams(reply_msg_, cookies); filter_->Send(reply_msg_); delete this; diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h index c112b77..2a4e6ce 100644 --- a/chrome/browser/renderer_host/resource_message_filter.h +++ b/chrome/browser/renderer_host/resource_message_filter.h @@ -27,6 +27,7 @@ #include "chrome/common/window_container_type.h" #include "gfx/native_widget_types.h" #include "ipc/ipc_channel_proxy.h" +#include "net/base/cookie_store.h" #include "third_party/WebKit/WebKit/chromium/public/WebCache.h" #include "third_party/WebKit/WebKit/chromium/public/WebPopupType.h" @@ -87,8 +88,7 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter, PluginService* plugin_service, printing::PrintJobManager* print_job_manager, Profile* profile, - RenderWidgetHelper* render_widget_helper, - URLRequestContextGetter* request_context); + RenderWidgetHelper* render_widget_helper); // IPC::ChannelProxy::MessageFilter methods: virtual void OnFilterAdded(IPC::Channel* channel); @@ -494,6 +494,14 @@ class GetCookiesCompletion : public net::CompletionCallback { return render_view_id_; } + void set_cookie_store(net::CookieStore* cookie_store) { + cookie_store_ = cookie_store; + } + + net::CookieStore* cookie_store() { + return cookie_store_.get(); + } + private: GURL url_; IPC::Message* reply_msg_; @@ -502,6 +510,7 @@ class GetCookiesCompletion : public net::CompletionCallback { int render_process_id_; int render_view_id_; bool raw_cookies_; + scoped_refptr cookie_store_; }; #endif // CHROME_BROWSER_RENDERER_HOST_RESOURCE_MESSAGE_FILTER_H_ diff --git a/chrome/browser/renderer_host/test/test_render_view_host.cc b/chrome/browser/renderer_host/test/test_render_view_host.cc index ac4efda..f5883ed 100644 --- a/chrome/browser/renderer_host/test/test_render_view_host.cc +++ b/chrome/browser/renderer_host/test/test_render_view_host.cc @@ -50,8 +50,7 @@ TestRenderViewHost::~TestRenderViewHost() { delete view(); } -bool TestRenderViewHost::CreateRenderView( - URLRequestContextGetter* request_context, const string16& frame_name) { +bool TestRenderViewHost::CreateRenderView(const string16& frame_name) { DCHECK(!render_view_created_); render_view_created_ = true; process()->ViewCreated(); diff --git a/chrome/browser/renderer_host/test/test_render_view_host.h b/chrome/browser/renderer_host/test/test_render_view_host.h index 82ff1c6..b4971bd 100644 --- a/chrome/browser/renderer_host/test/test_render_view_host.h +++ b/chrome/browser/renderer_host/test/test_render_view_host.h @@ -176,8 +176,7 @@ class TestRenderViewHost : public RenderViewHost { // RenderViewHost overrides -------------------------------------------------- - virtual bool CreateRenderView(URLRequestContextGetter* request_context, - const string16& frame_name); + virtual bool CreateRenderView(const string16& frame_name); virtual bool IsRenderViewLive() const; private: -- cgit v1.1