app-shell: Introduce a ShellRendererMainDelegate.

app-shell: Introduce a ShellRendererMainDelegate.

It may be necessary to create custom filters/observers etc. on the renderer
side in athena (e.g., for virtual keyboard support). So allow athena to install
a delegate to be created in the renderer process. The delegate is notified
when the RenderThread is initialized, or when new RenderViews are created.

BUG=380215
R=jamescook@chromium.org, oshima@chromium.org

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

[oshima, 2014-06-17 16:18:38]

lgtm if james is ok with this.

Note: We may eventually need to run inside chrome (like ash) due to many
constraints, so please consider minimizing the dependency to app shell.

[James Cook, 2014-06-17 17:40:23]

I'm OK with the overall approach but see comments.

https://codereview.chromium.org/331323008/diff/1/apps/shell/app/shell_main_de...
File apps/shell/app/shell_main_delegate.h (right):

https://codereview.chromium.org/331323008/diff/1/apps/shell/app/shell_main_de...
apps/shell/app/shell_main_delegate.h:39: virtual ShellRendererMainDelegate*
CreateShellRendererMainDelegate();
Who owns this object?

https://codereview.chromium.org/331323008/diff/1/apps/shell/renderer/shell_re...
File apps/shell/renderer/shell_renderer_main_delegate.h (right):

https://codereview.chromium.org/331323008/diff/1/apps/shell/renderer/shell_re...
apps/shell/renderer/shell_renderer_main_delegate.h:23: virtual void
OnViewCreated(content::RenderView* view) = 0;
nit: comment similar to above on when this is called.

https://codereview.chromium.org/331323008/diff/1/athena/main/athena_main.cc
File athena/main/athena_main.cc (right):

https://codereview.chromium.org/331323008/diff/1/athena/main/athena_main.cc#n...
athena/main/athena_main.cc:72: };
DISALLOW_COPY_AND_ASSIGN?

https://codereview.chromium.org/331323008/diff/1/athena/main/athena_main.cc#n...
athena/main/athena_main.cc:88: return new AthenaRendererMainDelegate();
Doesn't this leak?

[sadrul, 2014-06-17 20:37:39]

https://codereview.chromium.org/331323008/diff/1/apps/shell/app/shell_main_de...
File apps/shell/app/shell_main_delegate.h (right):

https://codereview.chromium.org/331323008/diff/1/apps/shell/app/shell_main_de...
apps/shell/app/shell_main_delegate.h:39: virtual ShellRendererMainDelegate*
CreateShellRendererMainDelegate();
On 2014/06/17 17:40:22, James Cook wrote:
> Who owns this object?

Good catch. I have changed this to (1) return a scoped_ptr<>, (2) have the
ShellContentRendererClient own the delegate, and (3) document it here.

https://codereview.chromium.org/331323008/diff/1/apps/shell/renderer/shell_re...
File apps/shell/renderer/shell_renderer_main_delegate.h (right):

https://codereview.chromium.org/331323008/diff/1/apps/shell/renderer/shell_re...
apps/shell/renderer/shell_renderer_main_delegate.h:23: virtual void
OnViewCreated(content::RenderView* view) = 0;
On 2014/06/17 17:40:22, James Cook wrote:
> nit: comment similar to above on when this is called.

Done.

https://codereview.chromium.org/331323008/diff/1/athena/main/athena_main.cc
File athena/main/athena_main.cc (right):

https://codereview.chromium.org/331323008/diff/1/athena/main/athena_main.cc#n...
athena/main/athena_main.cc:72: };
On 2014/06/17 17:40:22, James Cook wrote:
> DISALLOW_COPY_AND_ASSIGN?

Done.

https://codereview.chromium.org/331323008/diff/1/athena/main/athena_main.cc#n...
athena/main/athena_main.cc:88: return new AthenaRendererMainDelegate();
On 2014/06/17 17:40:22, James Cook wrote:
> Doesn't this leak?

It indeed does. I have clarified the ownership of this (see previous comment)

[James Cook, 2014-06-17 21:37:28]

lgtm

[sadrul, 2014-06-18 00:24:17]

Committed patchset #3 manually as r277922 (presubmit successful).