Move guest view container message routing out of content/.

Move guest view container message routing out of content/.

GuestViews used to receive messages through BrowserPlugin, this CL makes
GuestView message routing possible without BrowserPlugin. This is a
prerequisite for making <webview> pluginless.

BUG=330264
Test=None, internal change only.

Committed: https://crrev.com/7a4f1ceb18ffe6777d6a8b10b2db06e957187eba
Cr-Commit-Position: refs/heads/master@{#332430}

[lazyboy, 2015-06-01 05:56:40]

lazyboy@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Fady Samuel, 2015-06-01 11:32:25]

Generally good, but there seems to be unnecessary code here?

https://codereview.chromium.org/1161483004/diff/120001/chrome/renderer/chrome...
File chrome/renderer/chrome_guest_view_container_observer.cc (right):

https://codereview.chromium.org/1161483004/diff/120001/chrome/renderer/chrome...
chrome/renderer/chrome_guest_view_container_observer.cc:5: #include
"chrome/renderer/chrome_guest_view_container_observer.h"
Why do we need this class at all? ExtensiosnGuestViewContainerObserver (really
should be ExtensionsGuestviewContainerDispatcher) does the same thing.

https://codereview.chromium.org/1161483004/diff/120001/chrome/renderer/chrome...
chrome/renderer/chrome_guest_view_container_observer.cc:10:
ChromeGuestViewContainerObserver::ChromeGuestViewContainerObserver() {
This is more of a GuestViewContaineDispatcher.

https://codereview.chromium.org/1161483004/diff/120001/components/guest_view/...
File components/guest_view/renderer/guest_view_container_observer.cc (right):

https://codereview.chromium.org/1161483004/diff/120001/components/guest_view/...
components/guest_view/renderer/guest_view_container_observer.cc:12:
GuestViewContainerObserver::GuestViewContainerObserver() {
The name of this class doesn't match what it's doing. This class filters and
forwards messages to the appropriate guest_view_container.

https://codereview.chromium.org/1161483004/diff/120001/components/guest_view/...
components/guest_view/renderer/guest_view_container_observer.cc:23: int
element_instance_id = -1;
nit: kInstanceIDNone;

https://codereview.chromium.org/1161483004/diff/120001/components/guest_view/...
File components/guest_view/renderer/guest_view_container_observer.h (right):

https://codereview.chromium.org/1161483004/diff/120001/components/guest_view/...
components/guest_view/renderer/guest_view_container_observer.h:15: // Observer
used to route messages to GuestViewContainer.
This is a dispatcher for GuestViewContainer, not an observer of
GuestViewContainers.

https://codereview.chromium.org/1161483004/diff/120001/content/public/rendere...
File content/public/renderer/content_renderer_client.h (right):

https://codereview.chromium.org/1161483004/diff/120001/content/public/rendere...
content/public/renderer/content_renderer_client.h:12: #include "base/bind.h"
nit: Is this necessary?

https://codereview.chromium.org/1161483004/diff/120001/extensions/renderer/gu...
File extensions/renderer/guest_view/extensions_guest_view_container_observer.cc
(right):

https://codereview.chromium.org/1161483004/diff/120001/extensions/renderer/gu...
extensions/renderer/guest_view/extensions_guest_view_container_observer.cc:21:
(IPC_MESSAGE_CLASS(message) == ExtensionsGuestViewMsgStart);
You only need to add this. Call the base class:

return GuestViewContaienrObserver(message) ||
  (IPC_MESSAGE_CLASS(message) == ExtensionsGuestViewMsgStart);

[lazyboy, 2015-06-01 16:00:07]

https://chromiumcodereview.appspot.com/1161483004/diff/120001/chrome/renderer...
File chrome/renderer/chrome_guest_view_container_observer.cc (right):

https://chromiumcodereview.appspot.com/1161483004/diff/120001/chrome/renderer...
chrome/renderer/chrome_guest_view_container_observer.cc:5: #include
"chrome/renderer/chrome_guest_view_container_observer.h"
On 2015/06/01 at 11:32:25, Fady Samuel wrote:
> Why do we need this class at all? ExtensiosnGuestViewContainerObserver (really
should be ExtensionsGuestviewContainerDispatcher) does the same thing.

See comment in other file.

https://chromiumcodereview.appspot.com/1161483004/diff/120001/chrome/renderer...
chrome/renderer/chrome_guest_view_container_observer.cc:10:
ChromeGuestViewContainerObserver::ChromeGuestViewContainerObserver() {
On 2015/06/01 at 11:32:25, Fady Samuel wrote:
> This is more of a GuestViewContaineDispatcher.

Done.

https://chromiumcodereview.appspot.com/1161483004/diff/120001/components/gues...
File components/guest_view/renderer/guest_view_container_observer.cc (right):

https://chromiumcodereview.appspot.com/1161483004/diff/120001/components/gues...
components/guest_view/renderer/guest_view_container_observer.cc:12:
GuestViewContainerObserver::GuestViewContainerObserver() {
On 2015/06/01 at 11:32:25, Fady Samuel wrote:
> The name of this class doesn't match what it's doing. This class filters and
forwards messages to the appropriate guest_view_container.

Done.

https://chromiumcodereview.appspot.com/1161483004/diff/120001/components/gues...
components/guest_view/renderer/guest_view_container_observer.cc:23: int
element_instance_id = -1;
On 2015/06/01 at 11:32:25, Fady Samuel wrote:
> nit: kInstanceIDNone;

Done.

https://chromiumcodereview.appspot.com/1161483004/diff/120001/components/gues...
File components/guest_view/renderer/guest_view_container_observer.h (right):

https://chromiumcodereview.appspot.com/1161483004/diff/120001/components/gues...
components/guest_view/renderer/guest_view_container_observer.h:15: // Observer
used to route messages to GuestViewContainer.
On 2015/06/01 at 11:32:25, Fady Samuel wrote:
> This is a dispatcher for GuestViewContainer, not an observer of
GuestViewContainers.

Done.

https://chromiumcodereview.appspot.com/1161483004/diff/120001/content/public/...
File content/public/renderer/content_renderer_client.h (right):

https://chromiumcodereview.appspot.com/1161483004/diff/120001/content/public/...
content/public/renderer/content_renderer_client.h:12: #include "base/bind.h"
On 2015/06/01 at 11:32:25, Fady Samuel wrote:
> nit: Is this necessary?

Yes.
(I've removed ipc_message.h below since we don't use IPC::
anymore, base::Bind was implicitly being used from there).

https://chromiumcodereview.appspot.com/1161483004/diff/120001/extensions/rend...
File extensions/renderer/guest_view/extensions_guest_view_container_observer.cc
(right):

https://chromiumcodereview.appspot.com/1161483004/diff/120001/extensions/rend...
extensions/renderer/guest_view/extensions_guest_view_container_observer.cc:21:
(IPC_MESSAGE_CLASS(message) == ExtensionsGuestViewMsgStart);
On 2015/06/01 at 11:32:25, Fady Samuel wrote:
> You only need to add this. Call the base class:
> 
> return GuestViewContaienrObserver(message) ||
>   (IPC_MESSAGE_CLASS(message) == ExtensionsGuestViewMsgStart);

If we disable extensions, then we shouldn't handle
ExtensionsGuestViewMsg* messages, that's why there's a separate
class in chrome/* with #if (ENABLE_EXTENSIONS) to guard this
bit. Let me know if I'm missing something.

[lazyboy, 2015-06-01 21:57:15]

Removed ChromeGuestViewContainerDispatcher + synced to HEAD.

[Fady Samuel, 2015-06-01 22:41:12]

Awesome! LGTM!

[lazyboy, 2015-06-01 22:48:34]

lazyboy@chromium.org changed reviewers:
+ jochen@chromium.org, kalman@chromium.org

[lazyboy, 2015-06-01 22:48:35]

+jochen for components/*, content/public/renderer/* and chrome/renderer/*,
specifically:
chrome/renderer/chrome_content_renderer_client.h
chrome/renderer/chrome_content_renderer_client.cc
components/guest_view.gypi
content/public/renderer/browser_plugin_delegate.h
content/public/renderer/browser_plugin_delegate.cc
content/public/renderer/content_renderer_client.h
content/public/renderer/content_renderer_client.cc

+kalman for extensions/*, specifically:
extensions/extensions.gypi
extensions/shell/renderer/shell_content_renderer_client.h
extensions/shell/renderer/shell_content_renderer_client.cc

[not at google - send to devlin, 2015-06-01 22:53:08]

rubberstamp lgtm

[jochen (gone - plz use gerrit), 2015-06-02 16:10:37]

lgtm

[lazyboy, 2015-06-02 17:41:04]

The CQ bit was checked by lazyboy@chromium.org

[commit-bot: I haz the power, 2015-06-02 17:42:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1161483004/160001

[commit-bot: I haz the power, 2015-06-02 17:47:35]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2015-06-02 17:49:26]

Patchset 9 (id:??) landed as
https://crrev.com/7a4f1ceb18ffe6777d6a8b10b2db06e957187eba
Cr-Commit-Position: refs/heads/master@{#332430}