1263469 - FetchEvent.request.cache is not set correctly for non-fetch channels

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

There is a mapping between our load flags and fetch RequestCache values.  We convert from RequestCache to load flags when setting an explicit RequestCache value.  We don't, however, translate the other way creating a Request from an nsIChannel.  So non-fetch channels never produce any RequestCache values besides "default".

I think we should fix this.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Set FetchEvent.request.cache value correctly for non-fetch channels. r=mayhemer

This maintains the current behavior for explicitly set fetch cache modes, but will otherwise guess the cache mode from the load flags.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

I will add a check for this in one of our tests that trigger a window refresh since we should get "no-cache" for those requests.  Should get to that tomorrow.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Attachment: bug1263469_p2_test.patch

Add a check for evt.request.cache to our current refresh test.  This causes the test to time out without the P1 and pass with the P1.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=22cc9d859c17

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

I will add a test for EventSource as well since its required to bypass the http cache.

Comment 6

[(no longer active)]

Comment on attachment 8739988 [details] [diff] [review]
bug1263469_p2_test.patch

Review of attachment 8739988 [details] [diff] [review]:
-----------------------------------------------------------------

This is good, but it would be nice if you can add a web platform test for this.  Either in addition or instead of this one.  r=me.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

We discussed this at the service worker face-to-face and decided we should implement this:

  https://github.com/slightlyoff/ServiceWorker/issues/875

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P3 Test FetchEvent.request.cache value on reload in wpt test. r=ehsan

WPT version of test validating frame reload cache value.  I'll look at writing a wpt EventSource test now.  We don't test EventSource at all there.

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P4 Update test_eventsource_intercept.html to validate FetchEvent.request.cache. r=ehsan

Update our EventSource mochitest to check for cache.

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P5 Add a wpt test case for EventSource. r=ehsan

After much pain, this adds a wpt test case for EventSource.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b42011ab8f71

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5ea46c9e9e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/a908bea60e67
https://hg.mozilla.org/integration/mozilla-inbound/rev/afd297527888
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8392b3322a9
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d66f5a53c09

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c5ea46c9e9e9
https://hg.mozilla.org/mozilla-central/rev/a908bea60e67
https://hg.mozilla.org/mozilla-central/rev/afd297527888
https://hg.mozilla.org/mozilla-central/rev/a8392b3322a9
https://hg.mozilla.org/mozilla-central/rev/6d66f5a53c09