[android_webview] Fake onReceivedError callback for intercepted navigations.

[android_webview] Fake onReceivedError callback for intercepted navigations.

In WebViewClassic a failed intercepted navigation would cause the
onReceivedError callback to be fired.

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

[mkosiba (inactive), 2013-03-11 18:36:46]

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:122:
private final String mOnReceivedErrorUnknownErrorDescriptionString;
this is ugly.. does anyone know if a Context obtained from a View can be used on
the IO thread?

[joth, 2013-03-11 18:51:27]

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:122:
private final String mOnReceivedErrorUnknownErrorDescriptionString;
On 2013/03/11 18:36:46, Martin Kosiba wrote:
> this is ugly.. does anyone know if a Context obtained from a View can be used
on
> the IO thread?

I think it depends what you're doing with said context. For this sort of thing
using the application context is probably OK too.
Other option is to pass the Context reference to postOnReceivedError and let
that fish out the string internally (already on the UI thread)

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:160: //
crbug.com/180950
Can you expand the comment a touch? The linked bug has a lot of stuff if there's
a one-line summary of the case we're handling that'd be great.
Add http:// to linkify the link.

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:275:
mOnReceivedErrorUnknownErrorDescriptionString = ErrorStrings.getString(
what is ErrorStrings ? I don't see it in the import list

[mkosiba (inactive), 2013-03-12 13:51:24]

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:160: //
crbug.com/180950
On 2013/03/11 18:51:28, joth wrote:
> Can you expand the comment a touch? The linked bug has a lot of stuff if
there's
> a one-line summary of the case we're handling that'd be great.
> Add http:// to linkify the link.

Done.

https://codereview.chromium.org/12620004/diff/4001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:275:
mOnReceivedErrorUnknownErrorDescriptionString = ErrorStrings.getString(
On 2013/03/11 18:51:28, joth wrote:
> what is ErrorStrings ? I don't see it in the import list

argh... nvm.. android.net.http.ErrorStrings is hidden :/ so we'll have to do
this in the glue layer :/

[benm (inactive), 2013-03-12 14:14:16]

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:164:
ErrorCodeConversionHelper.ERROR_UNKNOWN,
If I understood the bug correctly, should this be 404?

[joth, 2013-03-12 16:36:39]

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:151:
AwContents.this.mContentsClient.shouldInterceptRequest(url);
nit: this function (and class) is inconsistent in the usage of AwContents.this
prefixing

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:162: //
http://crbug.com/180950
nit: squidge link onto previous line

[mkosiba (inactive), 2013-03-13 16:09:07]

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:151:
AwContents.this.mContentsClient.shouldInterceptRequest(url);
On 2013/03/12 16:36:39, joth wrote:
> nit: this function (and class) is inconsistent in the usage of AwContents.this
> prefixing

Done.

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:162: //
http://crbug.com/180950
On 2013/03/12 16:36:39, joth wrote:
> nit: squidge link onto previous line

Done.

https://codereview.chromium.org/12620004/diff/8001/android_webview/java/src/o...
android_webview/java/src/org/chromium/android_webview/AwContents.java:164:
ErrorCodeConversionHelper.ERROR_UNKNOWN,
On 2013/03/12 14:14:16, benm wrote:
> If I understood the bug correctly, should this be 404?

yes, but a 404 is not a network layer error. FWIW this is the error
WebViewClassic uses.

[benm (inactive), 2013-03-13 16:19:44]

lgtm

https://codereview.chromium.org/12620004/diff/14001/android_webview/javatests...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
(right):

https://codereview.chromium.org/12620004/diff/14001/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java:330:
assertEquals(0, mWebServer.getRequestCount("/" +
CommonResources.ABOUT_FILENAME));
s/CommonResource.ABOUT_FILENAME/aboutpageUrl/ ?

[mkosiba (inactive), 2013-03-13 16:30:09]

https://codereview.chromium.org/12620004/diff/14001/android_webview/javatests...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
(right):

https://codereview.chromium.org/12620004/diff/14001/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java:330:
assertEquals(0, mWebServer.getRequestCount("/" +
CommonResources.ABOUT_FILENAME));
On 2013/03/13 16:19:44, benm wrote:
> s/CommonResource.ABOUT_FILENAME/aboutpageUrl/ ?

it's not a URL, it's the path part of the URL ('/about.html'). The TestWebServer
API is a bit silly about this, maybe we could auto-prepend the '/' or something.

[commit-bot: I haz the power, 2013-03-13 16:31:20]

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

[benm (inactive), 2013-03-13 16:34:44]

https://codereview.chromium.org/12620004/diff/14001/android_webview/javatests...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
(right):

https://codereview.chromium.org/12620004/diff/14001/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java:330:
assertEquals(0, mWebServer.getRequestCount("/" +
CommonResources.ABOUT_FILENAME));
I see. Just looked a bit asymmetrical - we get the page Url by asking the
webserver for it, then assert it was a magic constant later on. The other tests
(OK, the one below) uses the constants to build the URL that gets loaded, for
example.

Doesn't really bother me, just an observation.

[mkosiba (inactive), 2013-03-13 16:49:57]

https://codereview.chromium.org/12620004/diff/14001/android_webview/javatests...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java
(right):

https://codereview.chromium.org/12620004/diff/14001/android_webview/javatests...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldInterceptRequestTest.java:330:
assertEquals(0, mWebServer.getRequestCount("/" +
CommonResources.ABOUT_FILENAME));
On 2013/03/13 16:34:44, benm wrote:
> I see. Just looked a bit asymmetrical - we get the page Url by asking the
> webserver for it, then assert it was a magic constant later on. The other
tests
> (OK, the one below) uses the constants to build the URL that gets loaded, for
> example.
> 
> Doesn't really bother me, just an observation.

this line asserts that the number of requests for the page is 0. The assymetry
comes from the fact that the WebServer.getRequestCount method expects a URL
relative to the server's address while the aboutPageUrl string contains the full
URL ("http://localhost:8080/about.html").

This could be easily fixed by making the WebServer API use Uri instead of
String.

[commit-bot: I haz the power, 2013-03-13 18:58:53]

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

[commit-bot: I haz the power, 2013-03-13 20:24:56]

Retried try job too often on android_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2013-03-14 11:35:35]

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

[commit-bot: I haz the power, 2013-03-14 20:21:46]

Change committed as 188171