[Android WebView] Implement initial settings and callback support for Service Workers.

[Android WebView] Implement initial settings and callback support for
Service Workers.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
Committed: https://crrev.com/47612e474a203be05f6c4f8f257902615e8406d2
Cr-Commit-Position: refs/heads/master@{#370998}

[timvolodine, 2016-01-05 13:39:39]

Description was changed from

==========
[WIP][Android] Implement initial ServiceWorker callbacks support in WebView.

NO SUBMIT! YET.

Intial implementation of AwServiceWorkerClient and related infrastructure.

BUG=566027
==========

to

==========
[WIP][Android] Implement initial ServiceWorker callbacks support in WebView.

NO SUBMIT! YET.

Intial implementation of AwServiceWorkerClient and related infrastructure.
In particular this patch also contains a test for the shouldInterceptRequest
callback.

BUG=566027
==========

[timvolodine, 2016-01-06 15:58:22]

Description was changed from

==========
[WIP][Android] Implement initial ServiceWorker callbacks support in WebView.

NO SUBMIT! YET.

Intial implementation of AwServiceWorkerClient and related infrastructure.
In particular this patch also contains a test for the shouldInterceptRequest
callback.

BUG=566027
==========

to

==========
[WIP][Android] Implement initial ServiceWorker callbacks support in WebView.

Intial implementation of AwServiceWorkerClient and related infrastructure.
In particular this patch also contains a test for the shouldInterceptRequest
callback.

BUG=566027
==========

[timvolodine, 2016-01-06 15:59:14]

timvolodine@chromium.org changed reviewers:
+ torne@chromium.org

[timvolodine, 2016-01-11 18:51:44]

timvolodine@chromium.org changed reviewers:
+ mnaganov@chromium.org

[timvolodine, 2016-01-11 18:51:45]

+mnaganov@: ptal, thanks!

[mnaganov (inactive), 2016-01-12 00:42:03]

The major comment that we have already discussed is that the instance of the
service worker client should be per browser context, not completely static.
Also, a couple of minor comments.

https://codereview.chromium.org/1544863002/diff/40001/android_webview/browser...
File android_webview/browser/net/aw_request_interceptor.cc (right):

https://codereview.chromium.org/1544863002/diff/40001/android_webview/browser...
android_webview/browser/net/aw_request_interceptor.cc:122: return
AwContentsIoThreadClient::GetServiceWorkerIoThreadClient();
Are you sure it's safe to assume that if a corresponding RenderFrame isn't
found, the request is from a shared worker? How about dedicated web workers for
example? I don't think they have a RenderFrame.

https://codereview.chromium.org/1544863002/diff/40001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
(right):

https://codereview.chromium.org/1544863002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:80:
assertEquals(fullSwUrl, helper.getAwWebResourceRequest().url);
What is ShouldInterceptRequest for a service worker supposed to intercept: both
loading of the sw script ('sw.js') and loads done by the script itself (e.g.
'sec-breach-nonexist.html' in this case), or just the latter?

In any case, you should also verify that loading of 'sec-breach-nonexist.html'
can also be intercepted.

[timvolodine, 2016-01-13 17:02:29]

Description was changed from

==========
[WIP][Android] Implement initial ServiceWorker callbacks support in WebView.

Intial implementation of AwServiceWorkerClient and related infrastructure.
In particular this patch also contains a test for the shouldInterceptRequest
callback.

BUG=566027
==========

to

==========
[WIP][Android] Implement initial ServiceWorker callbacks support in WebView.

Intial implementation of AwServiceWorkerClient, AwServiceWorkerSettings and
related infrastructure. In particular this patch also contains a test for
the shouldInterceptRequest callback.

BUG=566027
==========

[timvolodine, 2016-01-13 17:07:58]

Thanks for the comments!

Moved the service worker and related bits to browser context. Added
AwServiceWorkerSettings. Further replies below. PTAL.

https://codereview.chromium.org/1544863002/diff/40001/android_webview/browser...
File android_webview/browser/net/aw_request_interceptor.cc (right):

https://codereview.chromium.org/1544863002/diff/40001/android_webview/browser...
android_webview/browser/net/aw_request_interceptor.cc:122: return
AwContentsIoThreadClient::GetServiceWorkerIoThreadClient();
On 2016/01/12 00:42:03, mnaganov wrote:
> Are you sure it's safe to assume that if a corresponding RenderFrame isn't
> found, the request is from a shared worker? How about dedicated web workers
for
> example? I don't think they have a RenderFrame.

Just checked: regular web workers do have a valid render frame id so the
callbacks should go the usual way to the webview client. And we don't expose
shared workers in webview, so this approach seems safe enough at this point.

Currently there doesn't seem to be a (straightforward and reliable) way to check
if the request originated from a service worker. Looks like it would require
changes to IPC structs to have this info reliably. Could probably be done as a
separate patch if we really need it..

https://codereview.chromium.org/1544863002/diff/40001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
(right):

https://codereview.chromium.org/1544863002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:80:
assertEquals(fullSwUrl, helper.getAwWebResourceRequest().url);
On 2016/01/12 00:42:03, mnaganov wrote:
> What is ShouldInterceptRequest for a service worker supposed to intercept:
both
> loading of the sw script ('sw.js') and loads done by the script itself (e.g.
> 'sec-breach-nonexist.html' in this case), or just the latter?
> 
> In any case, you should also verify that loading of 'sec-breach-nonexist.html'
> can also be intercepted. 

In my view it should intercept all service worker related requests which are not
captured by the current callback mechanism.
This includes loading of 'sw.js' and fetches by the script (in both cases the
current callbacks are not called since there is no valid frame).

regarding second remark: i agree, tried adding another test in various ways but
it looks like service worker JavaScript does not get executed. Maybe we don't
support running SW in test environment, let me know if you have any thoughts
about this?

[mnaganov (inactive), 2016-01-13 17:55:27]

https://codereview.chromium.org/1544863002/diff/40001/android_webview/browser...
File android_webview/browser/net/aw_request_interceptor.cc (right):

https://codereview.chromium.org/1544863002/diff/40001/android_webview/browser...
android_webview/browser/net/aw_request_interceptor.cc:122: return
AwContentsIoThreadClient::GetServiceWorkerIoThreadClient();
On 2016/01/13 17:07:58, timvolodine wrote:
> On 2016/01/12 00:42:03, mnaganov wrote:
> > Are you sure it's safe to assume that if a corresponding RenderFrame isn't
> > found, the request is from a shared worker? How about dedicated web workers
> for
> > example? I don't think they have a RenderFrame.
> 
> Just checked: regular web workers do have a valid render frame id so the
> callbacks should go the usual way to the webview client. And we don't expose
> shared workers in webview, so this approach seems safe enough at this point.
> 
> Currently there doesn't seem to be a (straightforward and reliable) way to
check
> if the request originated from a service worker. Looks like it would require
> changes to IPC structs to have this info reliably. Could probably be done as a
> separate patch if we really need it..

OK, please then add a comment here explaining why we can assume that absence if
frame id means it's a request from SW.

https://codereview.chromium.org/1544863002/diff/40001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
(right):

https://codereview.chromium.org/1544863002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:80:
assertEquals(fullSwUrl, helper.getAwWebResourceRequest().url);
On 2016/01/13 17:07:58, timvolodine wrote:
> On 2016/01/12 00:42:03, mnaganov wrote:
> > What is ShouldInterceptRequest for a service worker supposed to intercept:
> both
> > loading of the sw script ('sw.js') and loads done by the script itself (e.g.
> > 'sec-breach-nonexist.html' in this case), or just the latter?
> > 
> > In any case, you should also verify that loading of
'sec-breach-nonexist.html'
> > can also be intercepted. 
> 
> In my view it should intercept all service worker related requests which are
not
> captured by the current callback mechanism.
> This includes loading of 'sw.js' and fetches by the script (in both cases the
> current callbacks are not called since there is no valid frame).
> 
> regarding second remark: i agree, tried adding another test in various ways
but
> it looks like service worker JavaScript does not get executed. Maybe we don't
> support running SW in test environment, let me know if you have any thoughts
> about this?

Perhaps JS is not enabled for service worker? JS execution is disabled by
default in WebView, and you need to enable it via AwSettings. As SW has it's own
context, it's possible that it might have JS disabled.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java:100:
mServiceWorkerClient = client;
Does it make sense to initialize everything else if 'client' is null?

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java:108:
private class ServiceWorkerIoThreadClientImpl extends AwContentsIoThreadClient {
I think it will be better to define this class outside of AwBrowserContext.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerClient.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerClient.java:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
nit: 2016

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerSettings.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerSettings.java:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
nit: 2016

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerSettings.java:10:
import android.util.Log;
I think we need to use org.chromium.base.Log these days.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
nit: 2016

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/TestAwServiceWorkerClient.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/TestAwServiceWorkerClient.java:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
nit: 2016

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/TestAwServiceWorkerClient.java:33:
ShouldInterceptRequestHelper mShouldInterceptRequestHelper;
private?

[timvolodine, 2016-01-14 19:39:52]

Description was changed from

==========
[WIP][Android] Implement initial ServiceWorker callbacks support in WebView.

Intial implementation of AwServiceWorkerClient, AwServiceWorkerSettings and
related infrastructure. In particular this patch also contains a test for
the shouldInterceptRequest callback.

BUG=566027
==========

to

==========
[Android] Implement initial ServiceWorker callbacks support in WebView.

Service Workers are not tied to AwContents as regular webviews are and hence
do not invoke callbacks provided in WebViewClient and WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it implements
the shouldInterceptRequest callback mechanism for service worker related
requests.
The callback is invoked when a service worker script is registered and also for
network fetches from within the service worker script.

This patch also contains a fix for TestWebServer to make it work with service
workers.

BUG=566027
==========

[timvolodine, 2016-01-14 19:42:19]

Description was changed from

==========
[Android] Implement initial ServiceWorker callbacks support in WebView.

Service Workers are not tied to AwContents as regular webviews are and hence
do not invoke callbacks provided in WebViewClient and WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it implements
the shouldInterceptRequest callback mechanism for service worker related
requests.
The callback is invoked when a service worker script is registered and also for
network fetches from within the service worker script.

This patch also contains a fix for TestWebServer to make it work with service
workers.

BUG=566027
==========

to

==========
[Android] Implement initial ServiceWorker callbacks support in WebView.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
==========

[timvolodine, 2016-01-14 19:43:56]

thanks again, replies below

https://codereview.chromium.org/1544863002/diff/40001/android_webview/browser...
File android_webview/browser/net/aw_request_interceptor.cc (right):

https://codereview.chromium.org/1544863002/diff/40001/android_webview/browser...
android_webview/browser/net/aw_request_interceptor.cc:122: return
AwContentsIoThreadClient::GetServiceWorkerIoThreadClient();
On 2016/01/13 17:55:27, mnaganov wrote:
> On 2016/01/13 17:07:58, timvolodine wrote:
> > On 2016/01/12 00:42:03, mnaganov wrote:
> > > Are you sure it's safe to assume that if a corresponding RenderFrame isn't
> > > found, the request is from a shared worker? How about dedicated web
workers
> > for
> > > example? I don't think they have a RenderFrame.
> > 
> > Just checked: regular web workers do have a valid render frame id so the
> > callbacks should go the usual way to the webview client. And we don't expose
> > shared workers in webview, so this approach seems safe enough at this point.
> > 
> > Currently there doesn't seem to be a (straightforward and reliable) way to
> check
> > if the request originated from a service worker. Looks like it would require
> > changes to IPC structs to have this info reliably. Could probably be done as
a
> > separate patch if we really need it..
> 
> OK, please then add a comment here explaining why we can assume that absence
if
> frame id means it's a request from SW.

Done.

https://codereview.chromium.org/1544863002/diff/40001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
(right):

https://codereview.chromium.org/1544863002/diff/40001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:80:
assertEquals(fullSwUrl, helper.getAwWebResourceRequest().url);
On 2016/01/13 17:55:27, mnaganov wrote:
> On 2016/01/13 17:07:58, timvolodine wrote:
> > On 2016/01/12 00:42:03, mnaganov wrote:
> > > What is ShouldInterceptRequest for a service worker supposed to intercept:
> > both
> > > loading of the sw script ('sw.js') and loads done by the script itself
(e.g.
> > > 'sec-breach-nonexist.html' in this case), or just the latter?
> > > 
> > > In any case, you should also verify that loading of
> 'sec-breach-nonexist.html'
> > > can also be intercepted. 
> > 
> > In my view it should intercept all service worker related requests which are
> not
> > captured by the current callback mechanism.
> > This includes loading of 'sw.js' and fetches by the script (in both cases
the
> > current callbacks are not called since there is no valid frame).
> > 
> > regarding second remark: i agree, tried adding another test in various ways
> but
> > it looks like service worker JavaScript does not get executed. Maybe we
don't
> > support running SW in test environment, let me know if you have any thoughts
> > about this?
> 
> Perhaps JS is not enabled for service worker? JS execution is disabled by
> default in WebView, and you need to enable it via AwSettings. As SW has it's
own
> context, it's possible that it might have JS disabled. 

Did some more debugging today regarding the service worker test. It turns out
the service worker registration itself failed because the TestWebServer did not
set the MIME type for .js files. I've rectified the situation now and added a
proper fetch interception test (yeey).

Will need review of the small change in TestWebServer now, hopefully it will not
impact other existing tests ;)

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java:100:
mServiceWorkerClient = client;
On 2016/01/13 17:55:27, mnaganov wrote:
> Does it make sense to initialize everything else if 'client' is null?

right, it doesn't. Fixed.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java:108:
private class ServiceWorkerIoThreadClientImpl extends AwContentsIoThreadClient {
On 2016/01/13 17:55:27, mnaganov wrote:
> I think it will be better to define this class outside of AwBrowserContext.

I've modeled this after how this is done in AwContents using two inner classes
to keep the members in one place and because these classes cannot exist without
a browser context. However if you feel strongly about this I can try and extract
the inner classes into two separate files with customized constructors..

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerClient.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerClient.java:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
On 2016/01/13 17:55:27, mnaganov wrote:
> nit: 2016

Done.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
File
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerSettings.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerSettings.java:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
On 2016/01/13 17:55:27, mnaganov wrote:
> nit: 2016

Done.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwServiceWorkerSettings.java:10:
import android.util.Log;
On 2016/01/13 17:55:27, mnaganov wrote:
> I think we need to use org.chromium.base.Log these days.

Done.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
On 2016/01/13 17:55:27, mnaganov wrote:
> nit: 2016

Done.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/TestAwServiceWorkerClient.java
(right):

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/TestAwServiceWorkerClient.java:1:
// Copyright 2015 The Chromium Authors. All rights reserved.
On 2016/01/13 17:55:27, mnaganov wrote:
> nit: 2016

Done.

https://codereview.chromium.org/1544863002/diff/60001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/TestAwServiceWorkerClient.java:33:
ShouldInterceptRequestHelper mShouldInterceptRequestHelper;
On 2016/01/13 17:55:27, mnaganov wrote:
> private?

Done.

[timvolodine, 2016-01-14 19:46:24]

timvolodine@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[timvolodine, 2016-01-14 19:46:25]

+phajdan.jr@chromium.org for TestWebServer.java

[mnaganov (inactive), 2016-01-14 21:13:39]

LGTM % comments

https://codereview.chromium.org/1544863002/diff/80001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java
(right):

https://codereview.chromium.org/1544863002/diff/80001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java:106:
AwContentsStatics.setServiceWorkerIoThreadClient(null, this);
If passing 'null' is considered to be a cleanup action, then we probably also
want to reset 'mServiceWorkerBackgroundThreadClient' and
'mServiceWorkerIoThreadClient' to release the objects.

https://codereview.chromium.org/1544863002/diff/80001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
(right):

https://codereview.chromium.org/1544863002/diff/80001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:62:
settings.setAppCacheEnabled(true);
Why do we really need all of these settings to be enabled? I don't see the test
using them. Are they required for SW?

https://codereview.chromium.org/1544863002/diff/80001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:79:
final String indexUrl = "/index.html";
nit: these indexUrl / indexHtml constants can be inlined to save some lines of
code

https://codereview.chromium.org/1544863002/diff/80001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:96:
TestAwServiceWorkerClient.ShouldInterceptRequestHelper helper =
I would suggest moving the instantiation of this helper before the call to
'loadUrlSync', storing the current call count into a variable called
'currentShouldInterceptRequestCount', and then call 'waitForCallback' this way:

helper.waitForCallback(currentShouldInterceptRequestCount, 2);

This would help code readability IMO.

[timvolodine, 2016-01-15 13:36:05]

https://codereview.chromium.org/1544863002/diff/80001/android_webview/java/sr...
File android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java
(right):

https://codereview.chromium.org/1544863002/diff/80001/android_webview/java/sr...
android_webview/java/src/org/chromium/android_webview/AwBrowserContext.java:106:
AwContentsStatics.setServiceWorkerIoThreadClient(null, this);
On 2016/01/14 21:13:39, mnaganov wrote:
> If passing 'null' is considered to be a cleanup action, then we probably also
> want to reset 'mServiceWorkerBackgroundThreadClient' and
> 'mServiceWorkerIoThreadClient' to release the objects.

yes. done.

https://codereview.chromium.org/1544863002/diff/80001/android_webview/javates...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java
(right):

https://codereview.chromium.org/1544863002/diff/80001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:62:
settings.setAppCacheEnabled(true);
On 2016/01/14 21:13:39, mnaganov wrote:
> Why do we really need all of these settings to be enabled? I don't see the
test
> using them. Are they required for SW?

oops this was for debugging, removed it now. Done.

https://codereview.chromium.org/1544863002/diff/80001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:79:
final String indexUrl = "/index.html";
On 2016/01/14 21:13:39, mnaganov wrote:
> nit: these indexUrl / indexHtml constants can be inlined to save some lines of
> code

Done.

https://codereview.chromium.org/1544863002/diff/80001/android_webview/javates...
android_webview/javatests/src/org/chromium/android_webview/test/AwServiceWorkerClientTest.java:96:
TestAwServiceWorkerClient.ShouldInterceptRequestHelper helper =
On 2016/01/14 21:13:39, mnaganov wrote:
> I would suggest moving the instantiation of this helper before the call to
> 'loadUrlSync', storing the current call count into a variable called
> 'currentShouldInterceptRequestCount', and then call 'waitForCallback' this
way:
> 
> helper.waitForCallback(currentShouldInterceptRequestCount, 2);
> 
> This would help code readability IMO.

Done.

[Paweł Hajdan Jr., 2016-01-15 13:39:22]

net/test LGTM

[timvolodine, 2016-01-21 20:37:46]

FYI
I've applied two (small) changes:

1. Introduced AwServiceWorkerManager to more naturally correspond with the
android/glue layer concepts and declutter AwBrowserContext. This also nicely
addresses one of the previous comments by Mikhail to put
ServiceWorkerIoThreadClientImpl into a separate file.

2. Removed get/setAcceptThirdPartyCookies from the settings, added a comment to
explain that we currently don't allow third party cookies from SW. At some point
we may decide to provide this setting if necessary, in which case it should
probably be supported through the CookieManager.

still TODO: rename AwServiceWorkerManager -> AwServiceWorkerController due to
android naming restrictions.

If I don't hear any objections I'll land this soon(ish) probably tomorrow.

[timvolodine, 2016-01-22 18:23:51]

Description was changed from

==========
[Android] Implement initial ServiceWorker callbacks support in WebView.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
==========

to

==========
[Android WebView] Implement initial ServiceWorker settings and callback support
in WebView.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
==========

[timvolodine, 2016-01-22 18:26:38]

Description was changed from

==========
[Android WebView] Implement initial ServiceWorker settings and callback support
in WebView.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
==========

to

==========
[Android WebView] Implement initial settings and callback support for
Service Workers.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
==========

[timvolodine, 2016-01-22 18:27:35]

The CQ bit was checked by timvolodine@chromium.org

[timvolodine, 2016-01-22 18:27:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
mnaganov@chromium.org, phajdan.jr@chromium.org
Link to the patchset: https://codereview.chromium.org/1544863002/#ps240001
(title: "rebase")

[commit-bot: I haz the power, 2016-01-22 18:27:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1544863002/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1544863002/240001

[commit-bot: I haz the power, 2016-01-22 18:35:38]

Description was changed from

==========
[Android WebView] Implement initial settings and callback support for
Service Workers.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
==========

to

==========
[Android WebView] Implement initial settings and callback support for
Service Workers.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
==========

[commit-bot: I haz the power, 2016-01-22 18:35:39]

Committed patchset #13 (id:240001)

[commit-bot: I haz the power, 2016-01-22 18:36:59]

Description was changed from

==========
[Android WebView] Implement initial settings and callback support for
Service Workers.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027
==========

to

==========
[Android WebView] Implement initial settings and callback support for
Service Workers.

Service Workers are not tied to AwContents as regular webviews are and
therefore do not invoke callbacks provided in WebViewClient and
WebChromeClient.

This patch provides initial implementation of AwServiceWorkerClient,
AwServiceWorkerSettings and related infrastructure. In particular it
implements the shouldInterceptRequest callback mechanism for service
worker related requests. The callback is invoked when a service worker
script is registered and also for network fetches from within the
service worker script.

This patch also contains a fix for TestWebServer to make it work with
service workers.

BUG=566027

Committed: https://crrev.com/47612e474a203be05f6c4f8f257902615e8406d2
Cr-Commit-Position: refs/heads/master@{#370998}
==========

[commit-bot: I haz the power, 2016-01-22 18:37:00]

Patchset 13 (id:??) landed as
https://crrev.com/47612e474a203be05f6c4f8f257902615e8406d2
Cr-Commit-Position: refs/heads/master@{#370998}