Add support for interstitial pages on android.

Add support for interstitial pages on android.

Required fixes:
1.) Notify webcontents when attaching/detaching the layer associated
    with the interstitial's RVH.
2.) Expose RVH for interstitials on Android for passing the gestures.

BUG=159066
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=166559

[Ted C, 2012-11-06 21:59:22]

@jam - interstitial api changes
@sievers - Android-y bits

[no sievers, 2012-11-06 22:42:56]

http://codereview.chromium.org/11366118/diff/1/content/browser/web_contents/w...
File content/browser/web_contents/web_contents_view_android.cc (right):

http://codereview.chromium.org/11366118/diff/1/content/browser/web_contents/w...
content/browser/web_contents/web_contents_view_android.cc:50: if (rwhv)
Can this ever be null? Should it match the logic in content_view_core_impl.cc
maybe?

[jam, 2012-11-07 01:19:28]

https://codereview.chromium.org/11366118/diff/1/content/public/browser/inters...
File content/public/browser/interstitial_page.h (right):

https://codereview.chromium.org/11366118/diff/1/content/public/browser/inters...
content/public/browser/interstitial_page.h:89: virtual RenderViewHost*
GetRenderViewHost() const = 0;
since this is only called from inside content, can you put this on
InterstitialPageImpl and just cast internally to get to it from
InterstitialPage? generally content api is used when crossing the content/chrome
boundary

[Ted C, 2012-11-07 01:34:30]

New patch upload

http://codereview.chromium.org/11366118/diff/1/content/browser/web_contents/w...
File content/browser/web_contents/web_contents_view_android.cc (right):

http://codereview.chromium.org/11366118/diff/1/content/browser/web_contents/w...
content/browser/web_contents/web_contents_view_android.cc:50: if (rwhv)
On 2012/11/06 22:42:56, Daniel Sievers wrote:
> Can this ever be null? Should it match the logic in content_view_core_impl.cc
> maybe?

Based on the number of checks in content_view_core_impl, I suspect it can be.  I
cleaned this up a bit to follow more closely what we do in cvc_impl though.

http://codereview.chromium.org/11366118/diff/1/content/public/browser/interst...
File content/public/browser/interstitial_page.h (right):

http://codereview.chromium.org/11366118/diff/1/content/public/browser/interst...
content/public/browser/interstitial_page.h:89: virtual RenderViewHost*
GetRenderViewHost() const = 0;
On 2012/11/07 01:19:28, John Abd-El-Malek wrote:
> since this is only called from inside content, can you put this on
> InterstitialPageImpl and just cast internally to get to it from
> InterstitialPage? generally content api is used when crossing the
content/chrome
> boundary

Done.

[no sievers, 2012-11-07 02:42:45]

lgtm 

On 2012/11/07 01:34:30, Ted C wrote:
> New patch upload
> 
>
http://codereview.chromium.org/11366118/diff/1/content/browser/web_contents/w...
> File content/browser/web_contents/web_contents_view_android.cc (right):
> 
>
http://codereview.chromium.org/11366118/diff/1/content/browser/web_contents/w...
> content/browser/web_contents/web_contents_view_android.cc:50: if (rwhv)
> On 2012/11/06 22:42:56, Daniel Sievers wrote:
> > Can this ever be null? Should it match the logic in
content_view_core_impl.cc
> > maybe?
> 
> Based on the number of checks in content_view_core_impl, I suspect it can be. 
I
> cleaned this up a bit to follow more closely what we do in cvc_impl though.
> 
>
http://codereview.chromium.org/11366118/diff/1/content/public/browser/interst...
> File content/public/browser/interstitial_page.h (right):
> 
>
http://codereview.chromium.org/11366118/diff/1/content/public/browser/interst...
> content/public/browser/interstitial_page.h:89: virtual RenderViewHost*
> GetRenderViewHost() const = 0;
> On 2012/11/07 01:19:28, John Abd-El-Malek wrote:
> > since this is only called from inside content, can you put this on
> > InterstitialPageImpl and just cast internally to get to it from
> > InterstitialPage? generally content api is used when crossing the
> content/chrome
> > boundary
> 
> Done.

[no sievers, 2012-11-07 02:46:57]

https://codereview.chromium.org/11366118/diff/5001/content/browser/web_conten...
File content/browser/web_contents/interstitial_page_impl.h (right):

https://codereview.chromium.org/11366118/diff/5001/content/browser/web_conten...
content/browser/web_contents/interstitial_page_impl.h:63: // Android shares a
single view for all tabs, so we need to expose the
maybe say 'platform window' instead of 'view' here, so it's not to be confused
with RenderWidgetHostView.

[jam, 2012-11-07 17:15:42]

lgtm

https://codereview.chromium.org/11366118/diff/5001/content/browser/web_conten...
File content/browser/web_contents/interstitial_page_impl.h (right):

https://codereview.chromium.org/11366118/diff/5001/content/browser/web_conten...
content/browser/web_contents/interstitial_page_impl.h:63: // Android shares a
single view for all tabs, so we need to expose the
On 2012/11/07 02:46:57, Daniel Sievers wrote:
> maybe say 'platform window' instead of 'view' here, so it's not to be confused
> with RenderWidgetHostView.

+1, that confused me :)

nit: can you move this to line 82, so that the platform specific functions are
at the end of the public section?

[Ted C, 2012-11-07 18:55:21]

http://codereview.chromium.org/11366118/diff/5001/content/browser/web_content...
File content/browser/web_contents/interstitial_page_impl.h (right):

http://codereview.chromium.org/11366118/diff/5001/content/browser/web_content...
content/browser/web_contents/interstitial_page_impl.h:63: // Android shares a
single view for all tabs, so we need to expose the
On 2012/11/07 17:15:42, John Abd-El-Malek wrote:
> On 2012/11/07 02:46:57, Daniel Sievers wrote:
> > maybe say 'platform window' instead of 'view' here, so it's not to be
confused
> > with RenderWidgetHostView.
> 
> +1, that confused me :)
> 
> nit: can you move this to line 82, so that the platform specific functions are
> at the end of the public section?

Done.

http://codereview.chromium.org/11366118/diff/5002/content/browser/web_content...
File content/browser/web_contents/web_contents_view_android.cc (right):

http://codereview.chromium.org/11366118/diff/5002/content/browser/web_content...
content/browser/web_contents/web_contents_view_android.cc:53:
rwhv->SetContentViewCore(content_view_core_);
I ended up needing to switch this back to the previous version as we want to set
the content view core on both renderwidgethostviews and not just one depending
on whether the interstitial was showing.

[commit-bot: I haz the power, 2012-11-07 21:58:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedchoc@chromium.org/11366118/5002

[commit-bot: I haz the power, 2012-11-08 00:33:11]

Change committed as 166559