[Android] Send top-level data:// requests to the browser.

[Android] Send top-level data:// requests to the browser.

The Android WebView API informs the embedder of all top-level navigations,
including navigations to URLs with the data:// scheme.
To remain compatible with the WebView API (and the Android Browser
which is implemented using the WebView API) we need to be able to inform the
embedder of top-level navigations to data:// URLs too.
In order to do that we first need to send them over from the renderer to
the browser.

BUG=130006
TEST=none

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

[darin (slow to review), 2012-05-29 04:27:15]

Why do you need this change?  What does compat with the Android Browser
mean?
-Darin

On Mon, May 28, 2012 at 9:49 AM, <mkosiba@chromium.org> wrote:

> Reviewers: darin,
>
> Description:
> [Android] Send top-level data:// requests to the browser.
>
> For Android Browser compatibility we need to be able to intercept
> navigations to URLs with the data:// scheme. In order to do that
> we first need to send them over from the renderer to the browser.
>
> BUG=130006
> TEST=none
>
>
> Please review this at
https://chromiumcodereview.**appspot.com/10440068/<https://chromiumcodereview...
>
> SVN Base:
http://git.chromium.org/**chromium/src.git@master<http://git.chromium.org/chr...
>
> Affected files:
>  M webkit/glue/weburlloader_impl.**cc
>
>
> Index: webkit/glue/weburlloader_impl.**cc
> diff --git a/webkit/glue/weburlloader_**impl.cc
> b/webkit/glue/weburlloader_**impl.cc
> index c6bcab8732da198796f6acf6f7de4c**9bee2e20aa..**
> 5acf64a8f36a2cb486b29670ea40af**86829b223e 100644
> --- a/webkit/glue/weburlloader_**impl.cc
> +++ b/webkit/glue/weburlloader_**impl.cc
> @@ -689,6 +689,13 @@ bool WebURLLoaderImpl::Context::**CanHandleDataURL(const
> GURL& url) const {
>   // reasons as well as to support unit tests, which do not have an
> underlying
>   // ResourceLoaderBridge implementation.
>
> +#if defined(OS_ANDROID)
> +  // For compatibility reasons on Android we need to expose top-level
> data://
> +  // to the browser.
> +  if (request_.targetType() == WebURLRequest::**TargetIsMainFrame)
> +    return false;
> +#endif
> +
>   if (request_.targetType() != WebURLRequest::**TargetIsMainFrame &&
>       request_.targetType() != WebURLRequest::**TargetIsSubframe)
>     return true;
>
>
>

[mkosiba (inactive), 2012-05-29 09:33:36]

Hi Darin,
    After https://chromiumcodereview.appspot.com/10310124/ is checked in the
ShouldIgnoreNavigation callback will be fired for all top-level navigations
except for navigations with the data:// scheme.
    The WebViewClient shouldOverrideUrlLoading callback[1] (which will be
implemented using WebContentsDelegate::ShouldIgnoreNavigation) is currently
fired for navigations to data:// URLs. The Android Browser checks if there are
any apps in the system that are registered to handle a certain navigations using
that callback.
    We need this for 2 things
1) WebView API compatibility - the current implementation calls
shouldOverrideUrlLoading for navigations to data:// URLs, we would like to do
that too,
2) Android Browser compatibility - the Android Browser broadcasts an Android
Intent every time the user navigates between pages, it does that for data://
URLs too. Chrome on Android should have that behavior too, otherwise we might
break existing apps (even though it's very unlikely that there are apps there
that have registered themselves as handlers for data:// URLs)

I'll expand the issue description.

[1]
http://developer.android.com/reference/android/webkit/WebViewClient.html#shou...,
java.lang.String)

On 2012/05/29 04:27:15, darin wrote:
> Why do you need this change?  What does compat with the Android Browser
> mean?
> -Darin
> 
> On Mon, May 28, 2012 at 9:49 AM, <mailto:mkosiba@chromium.org> wrote:
> 
> > Reviewers: darin,
> >
> > Description:
> > [Android] Send top-level data:// requests to the browser.
> >
> > For Android Browser compatibility we need to be able to intercept
> > navigations to URLs with the data:// scheme. In order to do that
> > we first need to send them over from the renderer to the browser.
> >
> > BUG=130006
> > TEST=none
> >
> >
> > Please review this at
>
https://chromiumcodereview.**appspot.com/10440068/%3Chttps://chromiumcoderevi...>
> >
> > SVN Base:
>
http://git.chromium.org/**chromium/src.git%40master%3Chttp://git.chromium.org...>
> >
> > Affected files:
> >  M webkit/glue/weburlloader_impl.**cc
> >
> >
> > Index: webkit/glue/weburlloader_impl.**cc
> > diff --git a/webkit/glue/weburlloader_**impl.cc
> > b/webkit/glue/weburlloader_**impl.cc
> > index c6bcab8732da198796f6acf6f7de4c**9bee2e20aa..**
> > 5acf64a8f36a2cb486b29670ea40af**86829b223e 100644
> > --- a/webkit/glue/weburlloader_**impl.cc
> > +++ b/webkit/glue/weburlloader_**impl.cc
> > @@ -689,6 +689,13 @@ bool
WebURLLoaderImpl::Context::**CanHandleDataURL(const
> > GURL& url) const {
> >   // reasons as well as to support unit tests, which do not have an
> > underlying
> >   // ResourceLoaderBridge implementation.
> >
> > +#if defined(OS_ANDROID)
> > +  // For compatibility reasons on Android we need to expose top-level
> > data://
> > +  // to the browser.
> > +  if (request_.targetType() == WebURLRequest::**TargetIsMainFrame)
> > +    return false;
> > +#endif
> > +
> >   if (request_.targetType() != WebURLRequest::**TargetIsMainFrame &&
> >       request_.targetType() != WebURLRequest::**TargetIsSubframe)
> >     return true;
> >
> >
> >

[mkosiba (inactive), 2012-06-20 10:40:50]

Ping?

[darin (slow to review), 2012-06-20 16:52:48]

OK, LGTM

[commit-bot: I haz the power, 2012-06-20 16:54:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/10440068/1

[commit-bot: I haz the power, 2012-06-20 19:26:39]

Change committed as 143228