Eliminate network fallback from ServiceWorkerContextRequestHandler.

Eliminate network fallback from ServiceWorkerContextRequestHandler.

ServiceWorkerContextRequestHandler should emit a network error instead
of falling back to network when it cannot handle a request. Otherwise, a
service worker can be spawned that did not load via our custom
ServiceWorkerWriteToCacheJob/ServiceWorkerReadFromCacheJob jobs,
resulting in a running worker whose ServiceWorkerVersion has not
been properly initialized. I suspect this can cause the bug 485900.

This patch:
- Changes network fallback for failure cases to an ERR_FAILED network
  error.
- As an exception, an installed worker loading an unstored script still
  results in network fallback. This should be deprecated and removed
  eventually, see https://github.com/w3c/ServiceWorker/issues/1021.
- Changes the behavior for a new worker loading an already stored script
  (i.e., calling importScripts() for the same script multiple times).
  Before this patch, we would fallback to network for this script. Now,
  we read the stored script. This is not yet codified in the spec but is
  expected to have almost no real-world impact and has support on
  https://github.com/w3c/ServiceWorker/issues/1041

BUG=485900, 678899
Review-Url: https://codereview.chromium.org/2602853002
Cr-Commit-Position: refs/heads/master@{#442590}
Committed: https://chromium.googlesource.com/chromium/src/+/815dc48c9d00b0142a2421de8f10b7cb9c5b34ab

[falken, 2016-12-28 10:03:43]

The CQ bit was checked by falken@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 10:03:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-28 10:46:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 10:46:59]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[falken, 2017-01-05 08:24:20]

The CQ bit was checked by falken@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 08:24:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 09:03:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-05 09:03:41]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[falken, 2017-01-06 05:53:40]

The CQ bit was checked by falken@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 05:53:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[falken, 2017-01-06 07:40:49]

Description was changed from

==========
WIP: ContextRequestHandler overhaul

BUG=
==========

to

==========
Eliminate network fallback from ServiceWorkerContextRequestHandler.

ServiceWorkerContextRequestHandler should emit a network error instead
of falling back to network when it cannot handle a request. Otherwise, a
service worker can be spawned did not load via our custom
ServiceWorkerWriteToCacheJob/ServiceWorkerReadFromCacheJob jobs,
resulting in a running worker whose ServiceWorkerVersion has not
been properly initialized. I suspect this can cause the bug 485900.

This patch:
- Changes network fallback for failure cases to an ERR_FAILED network
  error.
- As an exception, an installed worker loading an unstored script still
  results in network fallback. This should be deprecated and removed
  eventually, see https://github.com/w3c/ServiceWorker/issues/1021.
- Changes the behavior for a new worker loading an already stored script
  (i.e., calling importScripts() for the same script multiple times).
  Before this patch, we would fallback to network for this script. Now,
  we read the stored script. This is not yet codified in the spec but is
  expected to have almost no real-world impact and has support on
  https://github.com/w3c/ServiceWorker/issues/1041

BUG=485900,678899
==========

[falken, 2017-01-06 07:44:42]

The CQ bit was checked by falken@chromium.org to run a CQ dry run

[falken, 2017-01-06 07:44:48]

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

[falken, 2017-01-06 07:44:48]

horo: Can you review this?

[commit-bot: I haz the power, 2017-01-06 07:44:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[falken, 2017-01-06 08:31:40]

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_browsertest.cc (left):

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_browsertest.cc:2563:
ASSERT_EQ(SERVICE_WORKER_OK, status);
Forgot to mention something. I didn't understand why this test is needs to
activate? Is it OK to remove this?

As written the test would fail because of new DCHECKs in SWContextRequestHandler
(it expects an installed worker to load stored script if it's the main script).
So we need to call InstallHelper here or do some more work like that.

[commit-bot: I haz the power, 2017-01-06 09:42:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 09:42:51]

Dry run: This issue passed the CQ dry run.

[horo, 2017-01-10 07:09:27]

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_browsertest.cc (left):

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_browsertest.cc:2563:
ASSERT_EQ(SERVICE_WORKER_OK, status);
On 2017/01/06 08:31:40, falken wrote:
> Forgot to mention something. I didn't understand why this test is needs to
> activate? Is it OK to remove this?
> 
> As written the test would fail because of new DCHECKs in
SWContextRequestHandler
> (it expects an installed worker to load stored script if it's the main
script).
> So we need to call InstallHelper here or do some more work like that.

It is OK to remove this block.

This test intended to check that the worker can start correctly with the cached
metadata which was previously stored to the storage.

In the previous code, if the worker is not activated yet, the script is not
loaded from the storage. It is because ShouldReadFromScriptCache() returns false
and MaybeCreateJobImpl() returns null with CeateJobStatus::IGNORE_UNKNOWN. So we
needed this code block to activate the worker and make the script be loaded from
the storage.

But in the new code, MaybeCreateJobImpl() returns a
ServiceWorkerReadFromCacheJob with
CreateJobStatus::READ_JOB_FOR_DUPLICATE_SCRIPT_IMPORT. So the script is loaded
from the storage without this block.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_context_request_handler.cc
(right):

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler.cc:41: }

} // namespace

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler.cc:103: //
TODO(michaeln): Get the desired behavior clarified in the spec,
Remove or update this TODO comment.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler.cc:123: if
(!job) {
Early return.

if (job)
  return;

...

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler.cc:235: if
(resource_type_ == RESOURCE_TYPE_SERVICE_WORKER)
if (is_main_script)

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
File
content/browser/service_worker/service_worker_context_request_handler_unittest.cc
(right):

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler_unittest.cc:40:
class MockHttpProtocolHandler
class MockHttpProtocolHandler
    : public net::URLRequestJobFactory::ProtocolHandler {
 public:
  MockHttpProtocolHandler(ResourceContext* resource_context);
  net::URLRequestJob* MaybeCreateJob(
      net::URLRequest* request,
      net::NetworkDelegate* network_delegate) const override;

 private:
  ResourceContext* resource_context_;
};

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler_unittest.cc:130:
ServiceWorkerRequestHandler::InitializeHandler(
Calling ServiceWorkerRequestHandler::InitializeHandler() in MaybeCreateJob()
looks strange. I think you should call
ServiceWorkerRequestHandler::InitializeHandler() soon after creating a
net::URLRequest.

MockHttpProtocolHandler::MockHttpProtocolHandler(
    ResourceContext* resource_context)
    : resource_context_(resource_context) {}

net::URLRequestJob* MockHttpProtocolHandler::MaybeCreateJob(
    net::URLRequest* request,
    net::NetworkDelegate* network_delegate) const {
  ServiceWorkerRequestHandler* handler =
      ServiceWorkerRequestHandler::GetHandler(request);
  return handler->MaybeCreateJob(request, network_delegate, resource_context_);
}

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler_unittest.cc:277:

You can call ServiceWorkerRequestHandler::InitializeHandler() here.

  ServiceWorkerRequestHandler::InitializeHandler(
      request.get(), helper_->context_wrapper(), &blob_storage_context_,
      helper_->mock_render_process_id(), provider_host_->provider_id(),
      true /* skip_service_worker */, FETCH_REQUEST_MODE_NO_CORS,
      FETCH_CREDENTIALS_MODE_OMIT, FetchRedirectMode::FOLLOW_MODE,
      RESOURCE_TYPE_SERVICE_WORKER, REQUEST_CONTEXT_TYPE_SERVICE_WORKER,
      REQUEST_CONTEXT_FRAME_TYPE_NONE, nullptr);

https://codereview.chromium.org/2602853002/diff/100001/net/log/net_log_event_...
File net/log/net_log_event_type_list.h (right):

https://codereview.chromium.org/2602853002/diff/100001/net/log/net_log_event_...
net/log/net_log_event_type_list.h:2092: // This event is emitted when a request
for a service worker script
strange wrapping

https://codereview.chromium.org/2602853002/diff/100001/net/log/net_log_event_...
net/log/net_log_event_type_list.h:2096: //}
// }

https://codereview.chromium.org/2602853002/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/serviceworker/importScripts.html
(right):

https://codereview.chromium.org/2602853002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/importScripts.html:24:
return navigator.serviceWorker.register('resources/duplicate-import-worker.js',
{scope: scope});
Wrap at 80 columns

https://codereview.chromium.org/2602853002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/importScripts.html:35:
return service_worker_unregister_and_register(t,
'resources/import-scripts-worker.js', scope)
ditto

[falken, 2017-01-10 08:17:46]

Thanks. PTAL.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_browsertest.cc (left):

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_browsertest.cc:2563:
ASSERT_EQ(SERVICE_WORKER_OK, status);
On 2017/01/10 07:09:27, horo wrote:
> On 2017/01/06 08:31:40, falken wrote:
> > Forgot to mention something. I didn't understand why this test is needs to
> > activate? Is it OK to remove this?
> > 
> > As written the test would fail because of new DCHECKs in
> SWContextRequestHandler
> > (it expects an installed worker to load stored script if it's the main
> script).
> > So we need to call InstallHelper here or do some more work like that.
> 
> It is OK to remove this block.
> 
> This test intended to check that the worker can start correctly with the
cached
> metadata which was previously stored to the storage.
> 
> In the previous code, if the worker is not activated yet, the script is not
> loaded from the storage. It is because ShouldReadFromScriptCache() returns
false
> and MaybeCreateJobImpl() returns null with CeateJobStatus::IGNORE_UNKNOWN. So
we
> needed this code block to activate the worker and make the script be loaded
from
> the storage.
> 
> But in the new code, MaybeCreateJobImpl() returns a
> ServiceWorkerReadFromCacheJob with
> CreateJobStatus::READ_JOB_FOR_DUPLICATE_SCRIPT_IMPORT. So the script is loaded
> from the storage without this block.

Thanks, makes sense.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_context_request_handler.cc
(right):

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler.cc:41: }
On 2017/01/10 07:09:27, horo wrote:
> 
> } // namespace

Done.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler.cc:103: //
TODO(michaeln): Get the desired behavior clarified in the spec,
On 2017/01/10 07:09:27, horo wrote:
> Remove or update this TODO comment.

I think it's still unresolved. I'll change michaeln -> falken.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler.cc:123: if
(!job) {
On 2017/01/10 07:09:27, horo wrote:
> Early return.
> 
> if (job)
>   return;
> 
> ...

Done.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler.cc:235: if
(resource_type_ == RESOURCE_TYPE_SERVICE_WORKER)
On 2017/01/10 07:09:27, horo wrote:
> if (is_main_script)

Done.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
File
content/browser/service_worker/service_worker_context_request_handler_unittest.cc
(right):

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler_unittest.cc:40:
class MockHttpProtocolHandler
On 2017/01/10 07:09:27, horo wrote:
> class MockHttpProtocolHandler
>     : public net::URLRequestJobFactory::ProtocolHandler {
>  public:
>   MockHttpProtocolHandler(ResourceContext* resource_context);
>   net::URLRequestJob* MaybeCreateJob(
>       net::URLRequest* request,
>       net::NetworkDelegate* network_delegate) const override;
> 
>  private:
>   ResourceContext* resource_context_;
> };

Done.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler_unittest.cc:130:
ServiceWorkerRequestHandler::InitializeHandler(
On 2017/01/10 07:09:27, horo wrote:
> Calling ServiceWorkerRequestHandler::InitializeHandler() in MaybeCreateJob()
> looks strange. I think you should call
> ServiceWorkerRequestHandler::InitializeHandler() soon after creating a
> net::URLRequest.
> 
> MockHttpProtocolHandler::MockHttpProtocolHandler(
>     ResourceContext* resource_context)
>     : resource_context_(resource_context) {}
> 
> net::URLRequestJob* MockHttpProtocolHandler::MaybeCreateJob(
>     net::URLRequest* request,
>     net::NetworkDelegate* network_delegate) const {
>   ServiceWorkerRequestHandler* handler =
>       ServiceWorkerRequestHandler::GetHandler(request);
>   return handler->MaybeCreateJob(request, network_delegate,
resource_context_);
> }

Good idea, done.

https://codereview.chromium.org/2602853002/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler_unittest.cc:277:

On 2017/01/10 07:09:27, horo wrote:
> You can call ServiceWorkerRequestHandler::InitializeHandler() here.
> 
>   ServiceWorkerRequestHandler::InitializeHandler(
>       request.get(), helper_->context_wrapper(), &blob_storage_context_,
>       helper_->mock_render_process_id(), provider_host_->provider_id(),
>       true /* skip_service_worker */, FETCH_REQUEST_MODE_NO_CORS,
>       FETCH_CREDENTIALS_MODE_OMIT, FetchRedirectMode::FOLLOW_MODE,
>       RESOURCE_TYPE_SERVICE_WORKER, REQUEST_CONTEXT_TYPE_SERVICE_WORKER,
>       REQUEST_CONTEXT_FRAME_TYPE_NONE, nullptr);

Done.

https://codereview.chromium.org/2602853002/diff/100001/net/log/net_log_event_...
File net/log/net_log_event_type_list.h (right):

https://codereview.chromium.org/2602853002/diff/100001/net/log/net_log_event_...
net/log/net_log_event_type_list.h:2092: // This event is emitted when a request
for a service worker script
On 2017/01/10 07:09:27, horo wrote:
> strange wrapping

Done.

https://codereview.chromium.org/2602853002/diff/100001/net/log/net_log_event_...
net/log/net_log_event_type_list.h:2096: //}
On 2017/01/10 07:09:27, horo wrote:
> // }

Done.

https://codereview.chromium.org/2602853002/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/serviceworker/importScripts.html
(right):

https://codereview.chromium.org/2602853002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/importScripts.html:24:
return navigator.serviceWorker.register('resources/duplicate-import-worker.js',
{scope: scope});
On 2017/01/10 07:09:27, horo wrote:
> Wrap at 80 columns

Done.

https://codereview.chromium.org/2602853002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/serviceworker/importScripts.html:35:
return service_worker_unregister_and_register(t,
'resources/import-scripts-worker.js', scope)
On 2017/01/10 07:09:27, horo wrote:
> ditto

Done.

[horo, 2017-01-10 08:37:37]

lgtm with a nit.

https://codereview.chromium.org/2602853002/diff/160001/content/browser/servic...
File
content/browser/service_worker/service_worker_context_request_handler_unittest.cc
(right):

https://codereview.chromium.org/2602853002/diff/160001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler_unittest.cc:39:
class ServiceWorkerContextRequestHandlerTest;
nit: remove this

[falken, 2017-01-10 08:44:33]

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

[falken, 2017-01-10 08:44:34]

asanka: can you review net/
horo: thanks

https://codereview.chromium.org/2602853002/diff/160001/content/browser/servic...
File
content/browser/service_worker/service_worker_context_request_handler_unittest.cc
(right):

https://codereview.chromium.org/2602853002/diff/160001/content/browser/servic...
content/browser/service_worker/service_worker_context_request_handler_unittest.cc:39:
class ServiceWorkerContextRequestHandlerTest;
On 2017/01/10 08:37:37, horo wrote:
> nit: remove this

Done.

[falken, 2017-01-10 08:44:49]

The CQ bit was checked by falken@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 08:44:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 10:42:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 10:42:56]

Dry run: This issue passed the CQ dry run.

[asanka, 2017-01-10 15:13:29]

/net/ lgtm

[falken, 2017-01-10 15:15:09]

The CQ bit was checked by falken@chromium.org

[falken, 2017-01-10 15:15:10]

The patchset sent to the CQ was uploaded after l-g-t-m from horo@chromium.org
Link to the patchset: https://codereview.chromium.org/2602853002/#ps180001
(title: "rm class")

[commit-bot: I haz the power, 2017-01-10 15:15:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[falken, 2017-01-10 15:15:42]

The CQ bit was unchecked by falken@chromium.org

[falken, 2017-01-10 15:15:50]

Description was changed from

==========
Eliminate network fallback from ServiceWorkerContextRequestHandler.

ServiceWorkerContextRequestHandler should emit a network error instead
of falling back to network when it cannot handle a request. Otherwise, a
service worker can be spawned did not load via our custom
ServiceWorkerWriteToCacheJob/ServiceWorkerReadFromCacheJob jobs,
resulting in a running worker whose ServiceWorkerVersion has not
been properly initialized. I suspect this can cause the bug 485900.

This patch:
- Changes network fallback for failure cases to an ERR_FAILED network
  error.
- As an exception, an installed worker loading an unstored script still
  results in network fallback. This should be deprecated and removed
  eventually, see https://github.com/w3c/ServiceWorker/issues/1021.
- Changes the behavior for a new worker loading an already stored script
  (i.e., calling importScripts() for the same script multiple times).
  Before this patch, we would fallback to network for this script. Now,
  we read the stored script. This is not yet codified in the spec but is
  expected to have almost no real-world impact and has support on
  https://github.com/w3c/ServiceWorker/issues/1041

BUG=485900,678899
==========

to

==========
Eliminate network fallback from ServiceWorkerContextRequestHandler.

ServiceWorkerContextRequestHandler should emit a network error instead
of falling back to network when it cannot handle a request. Otherwise, a
service worker can be spawned that did not load via our custom
ServiceWorkerWriteToCacheJob/ServiceWorkerReadFromCacheJob jobs,
resulting in a running worker whose ServiceWorkerVersion has not
been properly initialized. I suspect this can cause the bug 485900.

This patch:
- Changes network fallback for failure cases to an ERR_FAILED network
  error.
- As an exception, an installed worker loading an unstored script still
  results in network fallback. This should be deprecated and removed
  eventually, see https://github.com/w3c/ServiceWorker/issues/1021.
- Changes the behavior for a new worker loading an already stored script
  (i.e., calling importScripts() for the same script multiple times).
  Before this patch, we would fallback to network for this script. Now,
  we read the stored script. This is not yet codified in the spec but is
  expected to have almost no real-world impact and has support on
  https://github.com/w3c/ServiceWorker/issues/1041

BUG=485900,678899
==========

[falken, 2017-01-10 15:16:04]

The CQ bit was checked by falken@chromium.org

[commit-bot: I haz the power, 2017-01-10 15:16:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 15:19:40]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1484061364439780,
"parent_rev": "1e97203ae5c8df638f08776582e4978c1b3f7e69", "commit_rev":
"815dc48c9d00b0142a2421de8f10b7cb9c5b34ab"}

[commit-bot: I haz the power, 2017-01-10 15:20:06]

Description was changed from

==========
Eliminate network fallback from ServiceWorkerContextRequestHandler.

ServiceWorkerContextRequestHandler should emit a network error instead
of falling back to network when it cannot handle a request. Otherwise, a
service worker can be spawned that did not load via our custom
ServiceWorkerWriteToCacheJob/ServiceWorkerReadFromCacheJob jobs,
resulting in a running worker whose ServiceWorkerVersion has not
been properly initialized. I suspect this can cause the bug 485900.

This patch:
- Changes network fallback for failure cases to an ERR_FAILED network
  error.
- As an exception, an installed worker loading an unstored script still
  results in network fallback. This should be deprecated and removed
  eventually, see https://github.com/w3c/ServiceWorker/issues/1021.
- Changes the behavior for a new worker loading an already stored script
  (i.e., calling importScripts() for the same script multiple times).
  Before this patch, we would fallback to network for this script. Now,
  we read the stored script. This is not yet codified in the spec but is
  expected to have almost no real-world impact and has support on
  https://github.com/w3c/ServiceWorker/issues/1041

BUG=485900,678899
==========

to

==========
Eliminate network fallback from ServiceWorkerContextRequestHandler.

ServiceWorkerContextRequestHandler should emit a network error instead
of falling back to network when it cannot handle a request. Otherwise, a
service worker can be spawned that did not load via our custom
ServiceWorkerWriteToCacheJob/ServiceWorkerReadFromCacheJob jobs,
resulting in a running worker whose ServiceWorkerVersion has not
been properly initialized. I suspect this can cause the bug 485900.

This patch:
- Changes network fallback for failure cases to an ERR_FAILED network
  error.
- As an exception, an installed worker loading an unstored script still
  results in network fallback. This should be deprecated and removed
  eventually, see https://github.com/w3c/ServiceWorker/issues/1021.
- Changes the behavior for a new worker loading an already stored script
  (i.e., calling importScripts() for the same script multiple times).
  Before this patch, we would fallback to network for this script. Now,
  we read the stored script. This is not yet codified in the spec but is
  expected to have almost no real-world impact and has support on
  https://github.com/w3c/ServiceWorker/issues/1041

BUG=485900,678899

Review-Url: https://codereview.chromium.org/2602853002
Cr-Commit-Position: refs/heads/master@{#442590}
Committed:
https://chromium.googlesource.com/chromium/src/+/815dc48c9d00b0142a2421de8f10...
==========

[commit-bot: I haz the power, 2017-01-10 15:20:08]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/815dc48c9d00b0142a2421de8f10...