New issue
Advanced search Search tips

Issue 784018 link

Starred by 1 user
Status: Fixed
Owner:
horo@chromium.org
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Track cross origin CORS responses to "same-origin" requests from a service worker

Project Member Reported by jakearchibald@chromium.org, Nov 11 2017 
Can we get numbers for the following service worker responses:

* The request's mode is 'same-origin'.
* The response url's origin is different to the request url's origin.
* The response is accepted (as in, is not rejected by 3.2.3 https://fetch.spec.whatwg.org/#http-fetch)

We're considering rejecting cross-origin CORS responses to same-origin requests, and we want to get a feel for how many sites are currently doing this. My gut tells me this is happening extremely infrequently.

Related issue: https://github.com/whatwg/fetch/issues/629

Comment 1 by falken@chromium.org, Nov 12 2017

Owner: horo@chromium.org
Implementation note: We should do this using UseCounter instead of a manual UMA as we've done in the past.

horo: Would you consider taking this on?

Comment 2 by horo@chromium.org, Nov 13 2017

Status: Assigned (was: Untriaged)
Sure.

Comment 3 by horo@chromium.org, Nov 13 2017

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75a6ecc2e2051e10ce41c64ee9630a77dc91af49

commit 75a6ecc2e2051e10ce41c64ee9630a77dc91af49
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Wed Nov 15 10:41:31 2017

Add UseCounter for cross origin CORS responses to same-origin requests

Bug:  784018 
Change-Id: Id70c364e623b9dfe19d2a5e43f0035015fe9a599
Reviewed-on: https://chromium-review.googlesource.com/765623
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516662}
[add] https://crrev.com/75a6ecc2e2051e10ce41c64ee9630a77dc91af49/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.respond-to-same-origin-request-with-cross-origin-response.html
[add] https://crrev.com/75a6ecc2e2051e10ce41c64ee9630a77dc91af49/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.respond-to-same-origin-request-with-redirected-cross-origin-response.html
[modify] https://crrev.com/75a6ecc2e2051e10ce41c64ee9630a77dc91af49/third_party/WebKit/Source/modules/serviceworkers/FetchRespondWithObserver.cpp
[modify] https://crrev.com/75a6ecc2e2051e10ce41c64ee9630a77dc91af49/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/75a6ecc2e2051e10ce41c64ee9630a77dc91af49/tools/metrics/histograms/enums.xml
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 15 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/faf9a296f716358e3ac6ab3f5d2b0a304f99f517

commit faf9a296f716358e3ac6ab3f5d2b0a304f99f517
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Wed Nov 15 16:11:44 2017

Add UMA for CacheStorage installed scripts

This CL introduces these UMAs:
  ServiceWorker.CacheStorageInstalledScript.ScriptSize
  ServiceWorker.CacheStorageInstalledScript.Count
  ServiceWorker.CacheStorageInstalledScript.ScriptTotalSize

Bug:  784018 
Change-Id: Idf6c93b527edae077ce122bc825b7513cf7303fb
Reviewed-on: https://chromium-review.googlesource.com/771172
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516710}
[modify] https://crrev.com/faf9a296f716358e3ac6ab3f5d2b0a304f99f517/third_party/WebKit/Source/modules/cachestorage/Cache.cpp
[modify] https://crrev.com/faf9a296f716358e3ac6ab3f5d2b0a304f99f517/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp
[modify] https://crrev.com/faf9a296f716358e3ac6ab3f5d2b0a304f99f517/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.h
[modify] https://crrev.com/faf9a296f716358e3ac6ab3f5d2b0a304f99f517/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeProxy.cpp
[modify] https://crrev.com/faf9a296f716358e3ac6ab3f5d2b0a304f99f517/third_party/WebKit/Source/modules/serviceworkers/WaitUntilObserver.cpp
[modify] https://crrev.com/faf9a296f716358e3ac6ab3f5d2b0a304f99f517/tools/metrics/histograms/histograms.xml

Comment 6 by bke...@mozilla.com, Nov 15 2017 

Horo, will this accurately track worker script load usage back to the owning document's use count?  We are adding some telemetry probes for this case as well and this part turned out to be difficult for us.

I just want to make sure at least one of the browsers gets data on worker script loads as they are a main source of same-origin mode Request objects.

Thanks!

Comment 7 by horo@chromium.org, Nov 15 2017

Status: Fixed (was: Started)
Opps!
The change #5 is not related to this issue.
This is for  issue 768705 .

The cl #4 introduced RespondToSameOriginRequestWithCrossOriginResponse in https://www.chromestatus.com/metrics/feature/popularity.
We will able to see the percentages of Chrome page loads which is controlled by a service worker, which has responded to a same-origin mode FetchEvent with a CORS response.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 24 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e1f3c3724269c16527397f488530cd68003bf16

commit 2e1f3c3724269c16527397f488530cd68003bf16
Author: Yannic Bonenberger <contact@yannic-bonenberger.com>
Date: Wed Jan 24 17:47:48 2018

Do not allow CORS responses to "same-origin" requests

This matches the change in the Fetch spec:
https://github.com/whatwg/fetch/issues/629

This CL also removes the UseCounter for cross-origin CORS responses to
same-origin requests because it will be unreachable after hereafter.

Chrome status: https://www.chromestatus.com/feature/5694278818856960

Bug:  800234 ,  784018 
Change-Id: Id843a302fa5d0614de1c3ef1c0a39bcf92f7e3ef
Reviewed-on: https://chromium-review.googlesource.com/866849
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Yannic Bonenberger <contact@yannic-bonenberger.com>
Cr-Commit-Position: refs/heads/master@{#531594}
[delete] https://crrev.com/f6e20f64fb9d6bc4045aea5b24de4ab0b8bfb455/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-response-taint.https-expected.txt
[delete] https://crrev.com/f6e20f64fb9d6bc4045aea5b24de4ab0b8bfb455/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.respond-to-same-origin-request-with-cross-origin-response.html
[delete] https://crrev.com/f6e20f64fb9d6bc4045aea5b24de4ab0b8bfb455/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.respond-to-same-origin-request-with-redirected-cross-origin-response.html
[modify] https://crrev.com/2e1f3c3724269c16527397f488530cd68003bf16/third_party/WebKit/Source/modules/payments/PaymentHandlerUtils.cpp
[modify] https://crrev.com/2e1f3c3724269c16527397f488530cd68003bf16/third_party/WebKit/Source/modules/serviceworkers/FetchRespondWithObserver.cpp
[modify] https://crrev.com/2e1f3c3724269c16527397f488530cd68003bf16/third_party/WebKit/common/service_worker/service_worker_error_type.mojom
[modify] https://crrev.com/2e1f3c3724269c16527397f488530cd68003bf16/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/2e1f3c3724269c16527397f488530cd68003bf16/tools/metrics/histograms/enums.xml
►

Sign in to add a comment