app_shell: Add ShellContentRendererClient

app_shell: Add ShellContentRendererClient

This is a step toward getting app_shell to initialize a renderer with extension support.

* Add apps/shell/renderer so we can separate out the render process code
* Do a little bit of cleanup in the Permissions system, which is the next thing to tackle.

BUG=335632
TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248548

[James Cook, 2014-01-29 22:43:14]

yoz, PTAL

[Yoyo Zhou, 2014-01-30 00:49:53]

LGTM for this iteration, although you said permissions providing was coming

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_content...
File apps/shell/shell_content_renderer_client.cc (right):

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_content...
apps/shell/shell_content_renderer_client.cc:38: // TODO(jamescook): Create
ContentSettingsObserver for allowScript?
This doesn't look extension-related but it might still be needed eventually. I
don't know.

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_content...
apps/shell/shell_content_renderer_client.cc:39: // TODO(jamescook): Create
ExtensionFrameHelper?
Looks like this is for Pepper web plugins?
https://codereview.chromium.org/105553005

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_main_de...
File apps/shell/shell_main_delegate.cc (right):

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_main_de...
apps/shell/shell_main_delegate.cc:84: if (process_type.empty())
nit: combine this with the below?

[James Cook, 2014-01-30 01:10:13]

yoz, please take another look. I added the Chrome permissions provider like we
talked about.

With this addition I can now call chrome.runtime.getManifest() and get a result.
Woo hoo!

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_content...
File apps/shell/shell_content_renderer_client.cc (right):

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_content...
apps/shell/shell_content_renderer_client.cc:38: // TODO(jamescook): Create
ContentSettingsObserver for allowScript?
On 2014/01/30 00:49:54, Yoyo Zhou wrote:
> This doesn't look extension-related but it might still be needed eventually. I
> don't know.

Removed TODO. We'll solve that problem if we need it.

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_content...
apps/shell/shell_content_renderer_client.cc:39: // TODO(jamescook): Create
ExtensionFrameHelper?
On 2014/01/30 00:49:54, Yoyo Zhou wrote:
> Looks like this is for Pepper web plugins?
> https://codereview.chromium.org/105553005

Hrm, so we might need it eventually if we want Pepper Flash?  I left it in.

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_main_de...
File apps/shell/shell_main_delegate.cc (right):

https://codereview.chromium.org/149163002/diff/80001/apps/shell/shell_main_de...
apps/shell/shell_main_delegate.cc:84: if (process_type.empty())
On 2014/01/30 00:49:54, Yoyo Zhou wrote:
> nit: combine this with the below?

Done.

[James Cook, 2014-01-30 18:07:31]

joi, can I get OWNERS for the apps/DEPS change?  The src/apps module already
depends on src/content; we're just starting to pick up some renderer stuff.

[Jói, 2014-01-31 10:58:09]

I don't know what the plans are for //apps, but I wonder if it would be better
to have a separate directory for renderer code (//apps/shell/renderer) similar
to what is done in the content shell, and add this to a DEPS file in that
directory?

As-is, there is nothing enforcing that you don't include renderer headers when
running in the browser process, which could lead to "interesting" things down
the line.

Cheers,
Jói

[James Cook, 2014-01-31 20:22:44]

joi@, please take another look

Good point about browser vs. renderer includes. //apps is all browser-process
stuff, but //apps/shell is going to need separate browser, renderer and common
code, just like content_shell.

I've added apps/shell/renderer in this CL. If it's OK with you I'll create
apps/shell/browser and apps/shell/common in a subsequent CL -- that will need a
bunch of file moves and DEPS changes and I'd like to keep this CL focused on
ShellContentRendererClient.

[Jói, 2014-02-03 10:39:12]

LGTM

[Jói, 2014-02-03 10:42:21]

https://codereview.chromium.org/149163002/diff/170001/apps/shell/renderer/DEPS
File apps/shell/renderer/DEPS (right):

https://codereview.chromium.org/149163002/diff/170001/apps/shell/renderer/DEP...
apps/shell/renderer/DEPS:1: include_rules = [
Just noticed: You need to add -content/public/browser.

[James Cook, 2014-02-03 17:31:36]

The CQ bit was checked by jamescook@chromium.org

[commit-bot: I haz the power, 2014-02-03 17:32:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamescook@chromium.org/149163002/270001

[commit-bot: I haz the power, 2014-02-03 19:32:33]

Change committed as 248548

[commit-bot: I haz the power, 2014-02-03 19:32:37]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-03 19:32:43]

CQ bit was unchecked on CL. Ignoring.