New issue
Advanced search Search tips

Issue 905662 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Renderer process crashes while registering service worker in a page served via a signed exchange

Project Member Reported by horo@chromium.org, Nov 15

Issue description

Chrome Version: 72.0.3611.0 (Official Build) canary (64-bit)

What steps will reproduce the problem?
(1) Enable chrome://flags/#allow-sxg-certs-without-extension
(2) Go https://htxg-b1.appspot.com/ and click "hello_ec.sxg"
(3) Open DevTools and run navigator.serviceWorker.register("sw.js").

What is the expected result?
No crash

What happens instead?
Crashes (ebc450dd0df05734).
"Origins are not matching, or some cannot access service worker"

My patch "Skip other interceptors after SignedExchangeRequestHandler started handling the request" was wrong.
https://chromium-review.googlesource.com/c/chromium/src/+/1278433
We need to call ServiceWorkerProviderHost::SetDocumentUrl() and SetTopmostFrameUrl() even after SignedExchangeRequestHandler started handling the request.

Fortunately (?), AMP demo (https://g.co/webpackagepreview) doesn't crash.
The demo site is using a service worker.
But service worker registration fails because of the CSP header in the signed exchange.
> "amp-install-serviceworker.js:296 Refused to create a worker from 'https://ampbyexample.com/sw.js' because it violates the following Content Security Policy directive: "script-src blob: https://cdn.ampproject.org/rtv/ https://cdn.ampproject.org/v0.js https://cdn.ampproject.org/v0/ https://cdn.ampproject.org/viewer/". Note that 'worker-src' was not explicitly set, so 'script-src' is used as a fallback."
So the renderer process doesn't crash.

 
Cc: shimazu@chromium.org
There as thousands of crashes of "Origins are not matching, or some cannot access service worker".

https://crash.corp.google.com/browse?q=REGEXP_CONTAINS+%28expanded_custom_data.ChromeCrashProto.magic_signature_1.name%2C%27Origins+are+not+matching%2C+or+some+cannot+access+service+worker%27%29#-propertyselector,productname:1000,+productversion:100,-magicsignature:50,-magicsignature2:50,-stablesignature:50
But I think most of them are not related to signed exchange.

falken@, shimazu@
Do we have a tracking crbug for this?

No, this looks new to me. Thanks for catching this. Should we make another issue for the non-SXG related case?
> shimazu@
Yes. please.
Status: Started (was: Assigned)
Created another issue for service worker cases: issue 906524.
Status: Fixed (was: Started)
This issue was fixed by this change:


commit 9ecea5f760faf209a56fd8efc017028d323e7053
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Thu Nov 22 11:36:07 2018

Reset the state of SWProviderHost when SXGRequestHandler handles the response.

I introduced skip_other_interceptors flag in crrev.com/c/1278433 to skip the
service worker handling of the synthesized redirection of signed exchange.
But this was wrong. We need to call ServiceWorkerProviderHost::SetDocumentUrl()
and SetTopmostFrameUrl() even after SignedExchangeRequestHandler started
handling the response to reject the service worker API calls from the page in
the signed exchange correctly. And also we need to call
SetControllerRegistration(null) to reset the controller state.

Bug:   894755  , 907712 
Change-Id: Iace762636982429c23ec24d9af6a466468e4b9fb
Reviewed-on: https://chromium-review.googlesource.com/c/1347954
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610381}
[modify] https://crrev.com/9ecea5f760faf209a56fd8efc017028d323e7053/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/9ecea5f760faf209a56fd8efc017028d323e7053/content/browser/service_worker/service_worker_request_handler.cc
[modify] https://crrev.com/9ecea5f760faf209a56fd8efc017028d323e7053/content/browser/service_worker/service_worker_request_handler.h
[modify] https://crrev.com/9ecea5f760faf209a56fd8efc017028d323e7053/content/browser/service_worker/service_worker_request_handler_unittest.cc
[modify] https://crrev.com/9ecea5f760faf209a56fd8efc017028d323e7053/content/browser/web_package/signed_exchange_request_handler_browsertest.cc
[add] https://crrev.com/9ecea5f760faf209a56fd8efc017028d323e7053/content/test/data/sxg/no-respond-with-service-worker.html
[add] https://crrev.com/9ecea5f760faf209a56fd8efc017028d323e7053/content/test/data/sxg/no-respond-with-service-worker.js
[add] https://crrev.com/9ecea5f760faf209a56fd8efc017028d323e7053/content/test/data/sxg/no-respond-with-service-worker.js.mock-http-headers


Sign in to add a comment