Extension SW: Add tests that serve web_accessible_resources from a SW.

Extension SW: Add tests that serve web_accessible_resources from a SW.

This CL also fixes an issue in DocumentThreadableLoader: DTL used to
only intercept http and https protocol requests. This CL expands it
to match content/ by using
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers().
Without this change fetch interception from extension SW would always
hit an ASSERT (in debug).

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.

BUG=545535, 567382
Committed: https://crrev.com/aea32c234f38db3f70454c86cbd3b40c833482e1
Cr-Commit-Position: refs/heads/master@{#367375}

[lazyboy, 2015-12-16 18:55:54]

Description was changed from

==========
Extension SW: Add a test that serves web_accessible_resources from a SW.

BUG=545535
==========

to

==========
Extension SW: Add a test that serves web_accessible_resources from a SW.

BUG=545535, 567382
==========

[lazyboy, 2015-12-16 18:56:12]

lazyboy@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Devlin, 2015-12-16 22:31:32]

This is a good start, but I'd also like to see us testing the ability to have an
iframe in a webpage to the extension's WAR.

[lazyboy, 2015-12-16 22:42:51]

On 2015/12/16 22:31:32, Devlin wrote:
> This is a good start, but I'd also like to see us testing the ability to have
an
> iframe in a webpage to the extension's WAR.

You mean iframe src="resource1.html" where resource1.html is listed in WAR?

[Devlin, 2015-12-16 22:47:30]

On 2015/12/16 22:42:51, lazyboy wrote:
> On 2015/12/16 22:31:32, Devlin wrote:
> > This is a good start, but I'd also like to see us testing the ability to
have
> an
> > iframe in a webpage to the extension's WAR.
> 
> You mean iframe src="resource1.html" where resource1.html is listed in WAR?

mostly - iframe src="chrome-extension://<id>/resource1.html"; it should be
accessible by a web page outside of the extension.

[lazyboy, 2015-12-17 01:37:29]

Description was changed from

==========
Extension SW: Add a test that serves web_accessible_resources from a SW.

BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
it is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
then it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

[lazyboy, 2015-12-17 01:37:57]

Description was changed from

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
it is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
then it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
then it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

[lazyboy, 2015-12-17 01:38:26]

Description was changed from

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
then it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

[lazyboy, 2015-12-17 01:39:03]

I've added another test to verify that scenario, PTAL. Thanks.

[Devlin, 2015-12-17 17:37:34]

https://chromiumcodereview.appspot.com/1532633003/diff/20001/chrome/browser/e...
File chrome/browser/extensions/service_worker_apitest.cc (right):

https://chromiumcodereview.appspot.com/1532633003/diff/20001/chrome/browser/e...
chrome/browser/extensions/service_worker_apitest.cc:580: // resource file. After
verifying that, we register a Service Worker that
Do we actually need a real resource file here?  I'm envisioning the use case of
an extension that can handle arbitrary urls at runtime by registering a service
worker to serve anything from a certain path.  Can we expand this test to see if
an url without a backing resource file is handled by the SW?

https://chromiumcodereview.appspot.com/1532633003/diff/20001/chrome/browser/e...
chrome/browser/extensions/service_worker_apitest.cc:585:
test_data_dir_.AppendASCII("service_worker/web_accessible_resources"),
it looks like maybe these should be two separate extensions.  This one can just
be a background page that registers a sw to handle incoming fetch events, and
the other can do all the fancy claim stuff.

WDYT?

[lazyboy, 2015-12-17 19:27:22]

https://chromiumcodereview.appspot.com/1532633003/diff/20001/chrome/browser/e...
File chrome/browser/extensions/service_worker_apitest.cc (right):

https://chromiumcodereview.appspot.com/1532633003/diff/20001/chrome/browser/e...
chrome/browser/extensions/service_worker_apitest.cc:580: // resource file. After
verifying that, we register a Service Worker that
On 2015/12/17 17:37:34, Devlin wrote:
> Do we actually need a real resource file here? 
No, we don't.
 I'm envisioning the use case of
> an extension that can handle arbitrary urls at runtime by registering a
service
> worker to serve anything from a certain path.  Can we expand this test to see
if
> an url without a backing resource file is handled by the SW?
Added a step that uses url from WAR manifest that is not present in
the extension resources.

https://chromiumcodereview.appspot.com/1532633003/diff/20001/chrome/browser/e...
chrome/browser/extensions/service_worker_apitest.cc:585:
test_data_dir_.AppendASCII("service_worker/web_accessible_resources"),
On 2015/12/17 17:37:34, Devlin wrote:
> it looks like maybe these should be two separate extensions.  This one can
just
> be a background page that registers a sw to handle incoming fetch events, and
> the other can do all the fancy claim stuff.
> 
> WDYT?

Done.

[lazyboy, 2015-12-18 02:19:34]

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

[lazyboy, 2015-12-18 02:19:35]

https://chromiumcodereview.appspot.com/1532633003/diff/80001/chrome/test/data...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/page.js
(right):

https://chromiumcodereview.appspot.com/1532633003/diff/80001/chrome/test/data...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/page.js:51:
fetch(getTestURL()).then(function(response) {
@falken, this fetch() is causing an assertion failure in
DocumentThreadableLoader:
ASSERT(!m_fallbackRequestForServiceWorker ..)

Searching in crbug revealed crbug.com/559001, where it says:
"the renderer thinks it's controlled by a SW, but it really isn't"

Can you help?

Roughly what the test is trying to do is:
1. load an extension
2. fetch a resource from the extension via Fetch API
3. observe that #2 is served correctly from the extension
4. Install SW and wait for it to call claim()
5. fetch the same resource and observe that the SW serves the resource now.

[falken, 2015-12-18 06:53:55]

falken@chromium.org changed reviewers:
+ horo@chromium.org

[falken, 2015-12-18 06:53:56]

This will probably need some careful debugging, DocumentThreadableLoader is not
a fun class.

The assert you're hitting is different from crbug.com/559001:

Yours: !m_fallbackRequestForServiceWorker.isNull() || m_requestContext ==
WebURLRequest::RequestContextSharedWorker
559001: !m_fallbackRequestForServiceWorker ||
securityOrigin()->canRequest(m_fallbackRequestForServiceWorker->url())

It looks like in your case, the renderer got back a response from the service
worker but it was surprised because it thought it was *not* controlled.

I think what's happening is DTL is created when the page is uncontrolled, then
claim() occurs but no one told DTL that it's now being controlled.

But if it's that simple, I would expect we'd have discovered this bug earlier.
So I would try to figure out how claim() works in the non-failing case, and then
try to figure out why it's causing trouble here.

[lazyboy, 2015-12-19 01:13:16]

On 2015/12/18 06:53:56, falken wrote:
> This will probably need some careful debugging, DocumentThreadableLoader is
not
> a fun class.
> 
> The assert you're hitting is different from crbug.com/559001:
> 
> Yours: !m_fallbackRequestForServiceWorker.isNull() || m_requestContext ==
> WebURLRequest::RequestContextSharedWorker
> 559001: !m_fallbackRequestForServiceWorker ||
> securityOrigin()->canRequest(m_fallbackRequestForServiceWorker->url())
> 
> It looks like in your case, the renderer got back a response from the service
> worker but it was surprised because it thought it was *not* controlled.
> 
> I think what's happening is DTL is created when the page is uncontrolled, then
> claim() occurs but no one told DTL that it's now being controlled.

Hmmm, if I understand correctly it DTL *thinks* it is being controlled, i.e.
document.fetcher().isControlledByServiceWorker() returns true in this test.

I've ran a test webpage, registered a SW from there and used fetch() API to
compare, one difference that I noticed was interesting during the construction
of
DocumentThreadableLoader:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
    // We assume that ServiceWorker is skipped for sync requests and non-HTTP
    // familiy requests by content/ code.
    if (m_async && !request.skipServiceWorker() &&
request.url().protocolIsInHTTPFamily() &&
document.fetcher()->isControlledByServiceWorker()) {

The "request.url().protocolIsInHTTPFamily()" part, which is false for
chrome-extension:// URLs and the test in this CL. This bit is true for
the test webpage I mention above.
This difference is responsible for isNull() m_fallbackRequestForServiceWorker in
DTL
(which is causing the ASSERT failure).

Also, FetchManager allows chrome-extension:// URLs, by looking at
SchemeRegistry::shouldTreatURLSchemeAsSupportingFetchAPI().

*naivly speaking*, should we have expanded the check in DTL construction to
allow
schemes from shouldTreatURLSchemeAsSupportingFetchAPI() instead of just
http/https?

Note that JS fetch() call using ServiceWorker from an extension seems
to always hit this issue. Here's a simpler sample extension that will repro:
https://github.com/lazyboy/chromium/tree/master/tests/extensions/service_work...


> 
> But if it's that simple, I would expect we'd have discovered this bug earlier.
> So I would try to figure out how claim() works in the non-failing case, and
then
> try to figure out why it's causing trouble here.

[falken, 2015-12-21 05:14:56]

On 2015/12/19 01:13:16, lazyboy wrote:
> On 2015/12/18 06:53:56, falken wrote:
> > This will probably need some careful debugging, DocumentThreadableLoader is
> not
> > a fun class.
> > 
> > The assert you're hitting is different from crbug.com/559001:
> > 
> > Yours: !m_fallbackRequestForServiceWorker.isNull() || m_requestContext ==
> > WebURLRequest::RequestContextSharedWorker
> > 559001: !m_fallbackRequestForServiceWorker ||
> > securityOrigin()->canRequest(m_fallbackRequestForServiceWorker->url())
> > 
> > It looks like in your case, the renderer got back a response from the
service
> > worker but it was surprised because it thought it was *not* controlled.
> > 
> > I think what's happening is DTL is created when the page is uncontrolled,
then
> > claim() occurs but no one told DTL that it's now being controlled.
> 
> Hmmm, if I understand correctly it DTL *thinks* it is being controlled, i.e.
> document.fetcher().isControlledByServiceWorker() returns true in this test.
> 
> I've ran a test webpage, registered a SW from there and used fetch() API to
> compare, one difference that I noticed was interesting during the construction
> of
> DocumentThreadableLoader:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
>     // We assume that ServiceWorker is skipped for sync requests and non-HTTP
>     // familiy requests by content/ code.
>     if (m_async && !request.skipServiceWorker() &&
> request.url().protocolIsInHTTPFamily() &&
> document.fetcher()->isControlledByServiceWorker()) {
> 
> The "request.url().protocolIsInHTTPFamily()" part, which is false for
> chrome-extension:// URLs and the test in this CL. This bit is true for
> the test webpage I mention above.
> This difference is responsible for isNull() m_fallbackRequestForServiceWorker
in
> DTL
> (which is causing the ASSERT failure).
> 
> Also, FetchManager allows chrome-extension:// URLs, by looking at
> SchemeRegistry::shouldTreatURLSchemeAsSupportingFetchAPI().
> 
> *naivly speaking*, should we have expanded the check in DTL construction to
> allow
> schemes from shouldTreatURLSchemeAsSupportingFetchAPI() instead of just
> http/https?

Nice debugging! I think that check is indeed wrong.

I dug in a bit and think the solution is actually to remove the
request.url().protocolIsInHTTPFamily() check entirely.
shouldTreatURLSchemeAsSupportingFetchAPI() is close but not exactly the same as
"can a SW intercept this request" which is what this check tries to do. It turns
out the check was originally:

  m_async && !request.skipServiceWorker() &&
m_document.fetcher()->isControlledByServiceWorker()

And it got changed in [1] to:

  m_async && !request.skipServiceWorker() &&
request.url().protocolIsInHTTPFamily() && request.httpMethod() != "OPTIONS" &&
m_document.fetcher()->isControlledByServiceWorker()

With a FIXME to remove the OPTIONS part when a bug was fixed. Later the OPTIONS
part was removed[2] but HTTP stayed, I think just as an oversight.

[1] https://codereview.chromium.org/726543008/
[2] https://codereview.chromium.org/743653002/

> 
> Note that JS fetch() call using ServiceWorker from an extension seems
> to always hit this issue. Here's a simpler sample extension that will repro:
>
https://github.com/lazyboy/chromium/tree/master/tests/extensions/service_work...
> 
> 
> > 
> > But if it's that simple, I would expect we'd have discovered this bug
earlier.
> > So I would try to figure out how claim() works in the non-failing case, and
> then
> > try to figure out why it's causing trouble here.

[lazyboy, 2015-12-21 05:51:15]

Description was changed from

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.
This CL also removes a check from DocumentThreadableLoader's constructor,
which seems to have stayed around unintentionally at r185836.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

[lazyboy, 2015-12-21 05:53:09]

Description was changed from

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.
This CL also removes a check from DocumentThreadableLoader's constructor,
which seems to have stayed around unintentionally at r185836.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.
This CL also removes a http family protocol check from
DocumentThreadableLoader's constructor, which seems to have stayed
around unintentionally at r185836.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

[lazyboy, 2015-12-21 05:57:16]

Great, thanks for the help.

https://chromiumcodereview.appspot.com/1532633003/diff/120001/third_party/Web...
File third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp (right):

https://chromiumcodereview.appspot.com/1532633003/diff/120001/third_party/Web...
third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp:196: // We
assume that ServiceWorker is skipped for sync requests by content/
I've removed the http family protocol check and updated the CL description.
If you prefer me to make this change separately, let me know.

[falken, 2015-12-21 06:31:39]

falken@chromium.org changed reviewers:
+ yhirano@chromium.org

[falken, 2015-12-21 06:31:39]

Sorry, it looks like I jumped to conclusions about HTTP being an oversight. The
CL description[1] mentions data:// URLs also being problematic, not just
OPTIONS.

So we really do need to replicate content's logic about "can request be handled
by SW?" in Blink after all.

The way to do that is replace the HTTP family check with
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers. That's ultimately
what ServiceWorkerRequestHandler::InitializeHandler uses on the content/ side
when deciding whether to intercept a URL request.

+yhirano@ if you have input, horo@ would be best but is ooo.

[1]
https://codereview.chromium.org/726543008/diff/1/Source/core/loader/DocumentT...

[yhirano, 2015-12-21 08:04:26]

Sorry for the naive question, should we allow a service worker to trap requests
*to* extension URLs? Aren't they already offline-capable?

Regarding the scheme checking, it corresponds to the spec at
https://fetch.spec.whatwg.org/#basic-fetch.

[Devlin, 2015-12-22 00:25:09]

On 2015/12/21 08:04:26, yhirano wrote:
> Sorry for the naive question, should we allow a service worker to trap
requests
> *to* extension URLs? Aren't they already offline-capable?
> 
> Regarding the scheme checking, it corresponds to the spec at
> https://fetch.spec.whatwg.org/#basic-fetch.

Lots of reasons for this.  The biggest is "why not".  It's an awesome web
technology, and we want to be able to use it, rather than build something else
that mimics it.  Last thing we want is an arms race between extensions and OWP,
or more of the "this is the extension way, this is the OWP way".  There are a
lot of fun use cases for being able to handle fetch events, since right now,
every url needs to be backed by a file on disk; sws make it so that extensions
can handle arbitrary urls.  And then, of course, there's all the awesome SW apis
that are coming our way. :)

As a rule of thumb, the extension philosophy has always tried to be "if it works
on the web, it should 'just work'".

[lazyboy, 2015-12-22 02:12:34]

On 2015/12/21 06:31:39, falken wrote:
> Sorry, it looks like I jumped to conclusions about HTTP being an oversight.
The
> CL description[1] mentions data:// URLs also being problematic, not just
> OPTIONS.
> 
> So we really do need to replicate content's logic about "can request be
handled
> by SW?" in Blink after all.
> 
> The way to do that is replace the HTTP family check with
> SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers. That's
ultimately
> what ServiceWorkerRequestHandler::InitializeHandler uses on the content/ side
> when deciding whether to intercept a URL request.
I've updated the CL with shouldTreatURLSchemeAsAllowingServiceWorkers().
> 
> +yhirano@ if you have input, horo@ would be best but is ooo.
> 
> [1]
>
https://codereview.chromium.org/726543008/diff/1/Source/core/loader/DocumentT...

[falken, 2015-12-22 14:14:43]

The change to DocumentThreadableLoader lgtm (as a non-owner), be sure to update
CL description first though.

[lazyboy, 2015-12-22 17:44:10]

Description was changed from

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.
This CL also removes a http family protocol check from
DocumentThreadableLoader's constructor, which seems to have stayed
around unintentionally at r185836.

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

This CL also fixes an issue in DocumentThreadableLoader: DTL used to
only intercept http and https protocol requests. This CL expands it
to match content/ by using
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers().

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

[lazyboy, 2015-12-22 17:54:10]

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

[lazyboy, 2015-12-22 17:54:11]

+japhet for core/loader/DocumentThreadableLoader.cpp OWNER.

@falken, I've updated the CL description.

[Nate Chapin, 2015-12-30 18:32:30]

lgtm

[Devlin, 2016-01-04 17:16:50]

extensions lgtm

https://codereview.chromium.org/1532633003/diff/140001/chrome/test/data/exten...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/page.js
(right):

https://codereview.chromium.org/1532633003/diff/140001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/page.js:33:
var url = chrome.runtime.getURL('/data_for_extension');
needed?

https://codereview.chromium.org/1532633003/diff/140001/chrome/test/data/exten...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/sw.js
(right):

https://codereview.chromium.org/1532633003/diff/140001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/sw.js:6:
if (/data_for_extension$/.test(e.request.url)) {
I think this would be a bit more readable as
if (URL(e.request.url).pathname == '/data_for_extension')

https://codereview.chromium.org/1532633003/diff/140001/chrome/test/data/exten...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/iframe_src/background.js
(right):

https://codereview.chromium.org/1532633003/diff/140001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/iframe_src/background.js:14:
return "true";
nit: prefer single quotes

https://codereview.chromium.org/1532633003/diff/140001/chrome/test/data/exten...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/iframe_src/sw.js
(right):

https://codereview.chromium.org/1532633003/diff/140001/chrome/test/data/exten...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/iframe_src/sw.js:19:
if (/iframe.html$/.test(e.request.url)) {
again, prefer using URL api.

[lazyboy, 2016-01-04 18:46:17]

Description was changed from

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

This CL also fixes an issue in DocumentThreadableLoader: DTL used to
only intercept http and https protocol requests. This CL expands it
to match content/ by using
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers().

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

This CL also fixes an issue in DocumentThreadableLoader: DTL used to
only intercept http and https protocol requests. This CL expands it
to match content/ by using
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers().
Without this change fetch interception from extension SW would always
hit an ASSERT (in debug).

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

[lazyboy, 2016-01-04 19:10:26]

https://chromiumcodereview.appspot.com/1532633003/diff/140001/chrome/test/dat...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/page.js
(right):

https://chromiumcodereview.appspot.com/1532633003/diff/140001/chrome/test/dat...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/page.js:33:
var url = chrome.runtime.getURL('/data_for_extension');
On 2016/01/04 17:16:50, Devlin wrote:
> needed?

Removed.

https://chromiumcodereview.appspot.com/1532633003/diff/140001/chrome/test/dat...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/sw.js
(right):

https://chromiumcodereview.appspot.com/1532633003/diff/140001/chrome/test/dat...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/fetch/sw.js:6:
if (/data_for_extension$/.test(e.request.url)) {
On 2016/01/04 17:16:50, Devlin wrote:
> I think this would be a bit more readable as
> if (URL(e.request.url).pathname == '/data_for_extension')

Done.

https://chromiumcodereview.appspot.com/1532633003/diff/140001/chrome/test/dat...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/iframe_src/background.js
(right):

https://chromiumcodereview.appspot.com/1532633003/diff/140001/chrome/test/dat...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/iframe_src/background.js:14:
return "true";
On 2016/01/04 17:16:50, Devlin wrote:
> nit: prefer single quotes

Done.

https://chromiumcodereview.appspot.com/1532633003/diff/140001/chrome/test/dat...
File
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/iframe_src/sw.js
(right):

https://chromiumcodereview.appspot.com/1532633003/diff/140001/chrome/test/dat...
chrome/test/data/extensions/api_test/service_worker/web_accessible_resources/iframe_src/sw.js:19:
if (/iframe.html$/.test(e.request.url)) {
On 2016/01/04 17:16:50, Devlin wrote:
> again, prefer using URL api.

Done.

[lazyboy, 2016-01-04 20:17:43]

The CQ bit was checked by lazyboy@chromium.org

[lazyboy, 2016-01-04 20:17:43]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
rdevlin.cronin@chromium.org, japhet@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/1532633003/#ps180001 (title: "remove
unused var")

[commit-bot: I haz the power, 2016-01-04 20:18:50]

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

[commit-bot: I haz the power, 2016-01-04 21:37:13]

Description was changed from

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

This CL also fixes an issue in DocumentThreadableLoader: DTL used to
only intercept http and https protocol requests. This CL expands it
to match content/ by using
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers().
Without this change fetch interception from extension SW would always
hit an ASSERT (in debug).

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

This CL also fixes an issue in DocumentThreadableLoader: DTL used to
only intercept http and https protocol requests. This CL expands it
to match content/ by using
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers().
Without this change fetch interception from extension SW would always
hit an ASSERT (in debug).

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

[commit-bot: I haz the power, 2016-01-04 21:37:14]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-01-04 21:39:08]

Description was changed from

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

This CL also fixes an issue in DocumentThreadableLoader: DTL used to
only intercept http and https protocol requests. This CL expands it
to match content/ by using
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers().
Without this change fetch interception from extension SW would always
hit an ASSERT (in debug).

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.


BUG=545535, 567382
==========

to

==========
Extension SW: Add tests that serve web_accessible_resources from a SW.

This CL also fixes an issue in DocumentThreadableLoader: DTL used to
only intercept http and https protocol requests. This CL expands it
to match content/ by using
SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers().
Without this change fetch interception from extension SW would always
hit an ASSERT (in debug).

ServiceWorkerTest.WebAccessibleResourcesFetch:
Tests that a Fetch request is served correctly by a SW when
SW is controlling an extension.

ServiceWorkerTest.WebAccessibleResourcesIframeSrc:
From a page outside an extension, an iframe is loaded with a .src of an
extension URL that is listed in the extension's web_accessible_resources.
The test verifies that if a Service Worker is controlling the extension,
it will also control the iframe's requested extension resource.

BUG=545535, 567382

Committed: https://crrev.com/aea32c234f38db3f70454c86cbd3b40c833482e1
Cr-Commit-Position: refs/heads/master@{#367375}
==========

[commit-bot: I haz the power, 2016-01-04 21:39:09]

Patchset 10 (id:??) landed as
https://crrev.com/aea32c234f38db3f70454c86cbd3b40c833482e1
Cr-Commit-Position: refs/heads/master@{#367375}