ChromeFrame currently overrides the request context for intercepting network ...

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
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=54867

[amit, 2010-08-04 00:30:01]

LGTM! Thanks for getting rid of UrlRequestContext override for Automation and
related muck.

http://codereview.chromium.org/3036038/diff/1/5
File chrome/browser/automation/automation_resource_message_filter.cc (right):

http://codereview.chromium.org/3036038/diff/1/5#newcode35
chrome/browser/automation/automation_resource_message_filter.cc:35: //
CookieStore specialization to have automation specific
Please, some background on why we need this workaround. :)

http://codereview.chromium.org/3036038/diff/1/5#newcode326
chrome/browser/automation/automation_resource_message_filter.cc:326:
AutomationDetails(tab_handle, filter, false);
why not add cookie_store arg to the AutomationDetails ctor?

http://codereview.chromium.org/3036038/diff/1/5#newcode401
chrome/browser/automation/automation_resource_message_filter.cc:401:
filtered_render_views_.Get().find(RendererId(
nit: declaring a RendererId renderer_key(...) would make this readable.

http://codereview.chromium.org/3036038/diff/1/19
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/3036038/diff/1/19#newcode567
chrome/browser/renderer_host/resource_message_filter.cc:567: if
(!AutomationResourceMessageFilter::SetCookiesForUrl(url, cookie,
Please comment on what's happening here. It feels odd for ResourceMessageFilter
to consult with AutomationResourceMessageFilter but it's much better than the
earlier UrlRequestContext workaround.

[ananta, 2010-08-04 01:25:17]

http://codereview.chromium.org/3036038/diff/1/5
File chrome/browser/automation/automation_resource_message_filter.cc (right):

http://codereview.chromium.org/3036038/diff/1/5#newcode35
chrome/browser/automation/automation_resource_message_filter.cc:35: //
CookieStore specialization to have automation specific
On 2010/08/04 00:30:01, amit wrote:
> Please, some background on why we need this workaround. :)

Done.

http://codereview.chromium.org/3036038/diff/1/5#newcode326
chrome/browser/automation/automation_resource_message_filter.cc:326:
AutomationDetails(tab_handle, filter, false);
On 2010/08/04 00:30:01, amit wrote:
> why not add cookie_store arg to the AutomationDetails ctor?

I thought about that. However we have a case where the automation filter could
change for pending views which basically needs an accessor to set the store.
Leaving it as is for consistency.

http://codereview.chromium.org/3036038/diff/1/5#newcode401
chrome/browser/automation/automation_resource_message_filter.cc:401:
filtered_render_views_.Get().find(RendererId(
On 2010/08/04 00:30:01, amit wrote:
> nit: declaring a RendererId renderer_key(...) would make this readable.

Done.

http://codereview.chromium.org/3036038/diff/1/19
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/3036038/diff/1/19#newcode567
chrome/browser/renderer_host/resource_message_filter.cc:567: if
(!AutomationResourceMessageFilter::SetCookiesForUrl(url, cookie,
On 2010/08/04 00:30:01, amit wrote:
> Please comment on what's happening here. It feels odd for
ResourceMessageFilter
> to consult with AutomationResourceMessageFilter but it's much better than the
> earlier UrlRequestContext workaround.
> 

Done.

[willchan no longer on Chromium, 2011-03-23 02:21:21]

http://codereview.chromium.org/3036038/diff/2003/chrome/browser/renderer_host...
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/3036038/diff/2003/chrome/browser/renderer_host...
chrome/browser/renderer_host/resource_message_filter.cc:565: context);
I was doing a refactor of this code (now moved to content/). I don't understand
how this is not being leaked when CF intercepts it. I don't see the
AutomationResourceMessageFilter running or deleting this callback. How does this
work?

[ananta, 2011-03-23 03:39:29]

http://codereview.chromium.org/3036038/diff/2003/chrome/browser/renderer_host...
File chrome/browser/renderer_host/resource_message_filter.cc (right):

http://codereview.chromium.org/3036038/diff/2003/chrome/browser/renderer_host...
chrome/browser/renderer_host/resource_message_filter.cc:565: context);
On 2011/03/23 02:21:22, willchan wrote:
> I was doing a refactor of this code (now moved to content/). I don't
understand
> how this is not being leaked when CF intercepts it. I don't see the
> AutomationResourceMessageFilter running or deleting this callback. How does
this
> work?

Thanks for catching this. It is definitely being leaked. The callback can be
deleted after sending the IPC over to chrome frame.