PerformanceResourceTiming.workerStart should not be restricted by Timing-Allow-Origin

RESOLVED FIXED in Firefox 58

Status

()

Core
DOM: Service Workers
RESOLVED FIXED
11 days ago
9 days ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

unspecified
mozilla58
Points:
---

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

11 days ago
When I implemented workerStart in bug 1191943 I accidentally copied code that restricts it to same-origin or Timing-Allow-Origin.  The spec does not say to do this:

https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-workerstart

In particular, this prevents the workerStart from showing up for typical CDN hosted image requests.
(Assignee)

Comment 1

11 days ago
Created attachment 8926659 [details] [diff] [review]
P1 Allow PerformanceResourceTiming.workerStart to be accessed on no-cors resources without Timing-Allow-Origin. r=baku

This fixes the problem.  I can write a WPT test for this.  I verified it makes firefox match chrome behavior manually.
(Assignee)

Comment 2

11 days ago
Comment on attachment 8926659 [details] [diff] [review]
P1 Allow PerformanceResourceTiming.workerStart to be accessed on no-cors resources without Timing-Allow-Origin. r=baku

Andrea, this removes the same-origin or Timing-Allow-Origin header restriction I accidentally used when first implementing workerStart.  Per the spec here we shouldn't use it:

https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-workerstart

Chrome also matches this behavior.
Attachment #8926659 - Flags: review?(amarchesini)
(Assignee)

Comment 3

11 days ago
I'll still add a WPT test before landing this.
Attachment #8926659 - Flags: review?(amarchesini) → review+
(Assignee)

Comment 4

10 days ago
Created attachment 8927058 [details] [diff] [review]
P2 Add a WPT test to verify we report workerStart on cross-origin no-cors requests, like <img>. r=baku

Andrea, this patch adds a cross-origin <img> load to the resource-timing.https.html test.  We should see workerStart here, but per the spec responseStart should not be shown.

Without the P1 patch we fail this test due to the missing workerStart.  With the P1 patch we pass this test.
Attachment #8927058 - Flags: review?(amarchesini)
Attachment #8927058 - Flags: review?(amarchesini) → review+

Comment 5

10 days ago
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f496f8ba892
P1 Allow PerformanceResourceTiming.workerStart to be accessed on no-cors resources without Timing-Allow-Origin. r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/56b28c7bc989
P2 Add a WPT test to verify we report workerStart on cross-origin no-cors requests, like <img>. r=baku
(Assignee)

Comment 6

10 days ago
Backed out the WPT in P2 because I forgot to update the manifest:

https://hg.mozilla.org/integration/mozilla-inbound/rev/9c74e8d1e1c3c31fe672058c4d6e09dd07f512ad
(Assignee)

Comment 7

10 days ago
Created attachment 8927244 [details] [diff] [review]
P2 Add a WPT test to verify we report workerStart on cross-origin no-cors requests, like <img>. r=baku
Attachment #8927058 - Attachment is obsolete: true
Attachment #8927244 - Flags: review+

Comment 8

10 days ago
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4762a6f87238
P2 Add a WPT test to verify we report workerStart on cross-origin no-cors requests, like <img>. r=baku

Comment 9

9 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2f496f8ba892
https://hg.mozilla.org/mozilla-central/rev/4762a6f87238
Status: ASSIGNED → RESOLVED
Last Resolved: 9 days ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.