BrowserPlugin: Interstitial Pages work

BrowserPlugin: Interstitial Pages work

This CL fixes resize, focus and cursor issues for interstitial pages inside
Browser Plugin.

Interstitial page RenderViewHosts are not a part of the main page's WebContents. Thus, BrowserPluginGuest's (a WebContentsObserver) OnMessageReceived method will not catch IPCs coming from interstitials. However, guest interstitials have RenderWidgetHostViewGuests.

By hanlding focus, resize, cursors and the like through RenderWidgetHostViewGuest, we ensure that interstitial pages get the same treatment inside a BrowserPlugin as RenderViewHosts that belong to the guest WebContents.

BUG=273089
Committed: https://crrev.com/9e3d9ad06062b656f0366f7828ad870b5430f999
Cr-Commit-Position: refs/heads/master@{#294281}

[Fady Samuel, 2014-09-08 22:19:39]

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

[Fady Samuel, 2014-09-08 22:19:39]

Ignore the CompositingHelper change, that's a remnant from another CL that's in
the CQ. Otherwise, this makes interstitial pages work! :D

[lazyboy, 2014-09-09 07:04:45]

Requesting clarifications on the CL and probably add a bit more CL description.

https://chromiumcodereview.appspot.com/549323002/diff/1/content/browser/brows...
File content/browser/browser_plugin/browser_plugin_guest.h (left):

https://chromiumcodereview.appspot.com/549323002/diff/1/content/browser/brows...
content/browser/browser_plugin/browser_plugin_guest.h:309: void OnDragStopped();
Nice catch.

https://chromiumcodereview.appspot.com/549323002/diff/1/content/browser/frame...
File content/browser/frame_host/render_widget_host_view_guest.cc (right):

https://chromiumcodereview.appspot.com/549323002/diff/1/content/browser/frame...
content/browser/frame_host/render_widget_host_view_guest.cc:109: if (guest_)
Can you explain a bit why this change and HasFocus() change are necessary?
Probably add a bit of description in your CL too.

https://chromiumcodereview.appspot.com/549323002/diff/1/content/browser/frame...
content/browser/frame_host/render_widget_host_view_guest.cc:282:
guest_->SendMessageToEmbedder(
Same for this one, probably add a comment right here as well; i.e. why
ViewMsg_SetCursort won't be received but RWHView::UpdateCursort would get
called.

[Fady Samuel, 2014-09-09 17:14:49]

PTAL

https://codereview.chromium.org/549323002/diff/1/content/browser/browser_plug...
File content/browser/browser_plugin/browser_plugin_guest.h (left):

https://codereview.chromium.org/549323002/diff/1/content/browser/browser_plug...
content/browser/browser_plugin/browser_plugin_guest.h:309: void OnDragStopped();
On 2014/09/09 07:04:45, lazyboy wrote:
> Nice catch.

Acknowledged.

https://codereview.chromium.org/549323002/diff/1/content/browser/frame_host/r...
File content/browser/frame_host/render_widget_host_view_guest.cc (right):

https://codereview.chromium.org/549323002/diff/1/content/browser/frame_host/r...
content/browser/frame_host/render_widget_host_view_guest.cc:109: if (guest_)
On 2014/09/09 07:04:45, lazyboy wrote:
> Can you explain a bit why this change and HasFocus() change are necessary?
> Probably add a bit of description in your CL too.

Done. InterstitialPage calls Focus directly:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

https://codereview.chromium.org/549323002/diff/1/content/browser/frame_host/r...
content/browser/frame_host/render_widget_host_view_guest.cc:282:
guest_->SendMessageToEmbedder(
On 2014/09/09 07:04:45, lazyboy wrote:
> Same for this one, probably add a comment right here as well; i.e. why
> ViewMsg_SetCursort won't be received but RWHView::UpdateCursort would get
> called.

Done.

[lazyboy, 2014-09-09 18:32:24]

lgtm

[Fady Samuel, 2014-09-09 18:43:56]

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

[Fady Samuel, 2014-09-09 18:43:57]

+creis@ for web_contents_view_guest, and render_widget_host_view_guest.

[Charlie Reis, 2014-09-10 01:00:05]

On 2014/09/09 18:43:57, Fady Samuel wrote:
> +creis@ for web_contents_view_guest, and render_widget_host_view_guest.

Hmm, I also reviewed part of this in https://codereview.chromium.org/554733003/.
 Was that a subset pulled from this one to land first?  If so, I'll wait to
review the rest of this until we finish that one.

(Side note: it can be helpful to give reviewers some context for cases like
this.  Sorry for the delayed / out-of-order review, since I'm just catching up
on mail after today's training.)

[Fady Samuel, 2014-09-10 15:36:49]

On 2014/09/10 01:00:05, Charlie Reis wrote:
> On 2014/09/09 18:43:57, Fady Samuel wrote:
> > +creis@ for web_contents_view_guest, and render_widget_host_view_guest.
> 
> Hmm, I also reviewed part of this in
https://codereview.chromium.org/554733003/.
>  Was that a subset pulled from this one to land first?  If so, I'll wait to
> review the rest of this until we finish that one.
> 
> (Side note: it can be helpful to give reviewers some context for cases like
> this.  Sorry for the delayed / out-of-order review, since I'm just catching up
> on mail after today's training.)

Apologies, I should have mentioned that this patch builds on top of the previous
patch.

[Fady Samuel, 2014-09-10 15:37:47]

PTAL, I've rebased the patch and cleaned up the comments. Hopefully this is more
readable and the code/comments are self-evident.

[Charlie Reis, 2014-09-10 19:55:31]

Rubber stamp LGTM.  Will there be a test for interstitials inside <webview>
tags?  (Not sure if it makes sense in this CL, which is more about focus, etc.)

[Fady Samuel, 2014-09-10 20:29:34]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-09-10 21:02:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/549323002/60001

[commit-bot: I haz the power, 2014-09-10 21:21:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-10 21:21:26]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/54241)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/59178)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Fady Samuel, 2014-09-10 21:33:56]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-09-10 21:35:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/549323002/80001

[Fady Samuel, 2014-09-10 23:11:37]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-09-10 23:18:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/549323002/100001

[commit-bot: I haz the power, 2014-09-11 01:05:25]

Committed patchset #6 (id:100001) as 28626ecb6bfec0e17bf24354720987010e69d0ec

[commit-bot: I haz the power, 2014-09-11 01:07:50]

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