Don't hold the installed app in BrowserProcessRenderHost, since that's in content layer now. Ext...

Don't hold the installed app in BrowserProcessRenderHost, since that's in content layer now.  ExtensionService keeps a map from renderer child ids to installed extensions instead.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=82797

[Charlie Reis, 2011-04-20 23:45:50]

Is this ready for review?  I noticed the patch sets kept changing as I started,
so I'm holding off for the time being.

Charlie

[Charlie Reis, 2011-04-21 00:25:12]

There's also a compile error in browser_render_process_host.cc.

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/automation/au...
File chrome/browser/automation/automation_util.cc (right):

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/automation/au...
chrome/browser/automation/automation_util.cc:110: const Extension* installed_app
= NULL;
Don't need this line.

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_service.h (right):

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_service.h:161: void
SetInstalledAppForRenderer(const Extension* app, int renderer_child_id);
Let's swap these arguments so it's more like a traditional put(key, value) call.

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile.h (right):

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/profiles/prof...
chrome/browser/profiles/profile.h:336: // Returns the request context
appropriate for the given app. If the renderer
"for the given app" -> "for the given renderer process"?

I'm wondering if we should just rename this GetRequestContextForRenderProcess. 
The fact that it bases the decision on whether the process has an app isn't
critical.

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/renderer_host...
File chrome/browser/renderer_host/chrome_render_message_filter.cc (right):

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/renderer_host...
chrome/browser/renderer_host/chrome_render_message_filter.cc:186:
request_context_->GetURLRequestContext());
This is ok to fix, though it seems unnecessary for this CL.

[jam, 2011-04-21 06:37:01]

hey, looks like I had missed one of the files in the upload hence the compile
error, it's updated now.  i had sent it for you before running the tests to get
the review in parallel with fixing tests etc

On 2011/04/20 23:45:50, creis wrote:
> Is this ready for review?  I noticed the patch sets kept changing as I
started,
> so I'm holding off for the time being.
> 
> Charlie

[jam, 2011-04-21 06:52:19]

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/automation/au...
File chrome/browser/automation/automation_util.cc (right):

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/automation/au...
chrome/browser/automation/automation_util.cc:110: const Extension* installed_app
= NULL;
On 2011/04/21 00:25:12, creis wrote:
> Don't need this line.

Done.

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_service.h (right):

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_service.h:161: void
SetInstalledAppForRenderer(const Extension* app, int renderer_child_id);
On 2011/04/21 00:25:12, creis wrote:
> Let's swap these arguments so it's more like a traditional put(key, value)
call.

Done.

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile.h (right):

http://codereview.chromium.org/6880089/diff/1038/chrome/browser/profiles/prof...
chrome/browser/profiles/profile.h:336: // Returns the request context
appropriate for the given app. If the renderer
On 2011/04/21 00:25:12, creis wrote:
> "for the given app" -> "for the given renderer process"?

Done

> 
> I'm wondering if we should just rename this GetRequestContextForRenderProcess.

> The fact that it bases the decision on whether the process has an app isn't
> critical.

sure, that sounds like a more appropriate name now.  Done.

[Charlie Reis, 2011-04-21 16:17:28]

LGTM, once it passes the try bots.  I think the switch from getting a
RequestContext for an installed app to getting one for a process sounds fine,
since each process is guaranteed to only have at most one installed app.