1126820 - persist Request context attribute in ServiceWorker Cache

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

The ServiceWorker Cache should persist the RequestContext value after Request.context is implemented in bug 1119037.

Comment 1

[(no longer active)]

I think Request.context does the wrong thing for now (see bug 1121157 comment 1).  Setting the dependency so that we figure that out first.

Comment 2

[(no longer active)]

Turns out that I need to fix this for bug 1147699, otherwise the IPDL serialization of RequestContext fails, and the tests crash.

Comment 3

[(no longer active)]

Attachment: Persist the Request.context attribute in DOM Cache

Ben, the test here is based on bug 1147699.  Please see <https://github.com/ehsan/gecko-dev/tree/explicit> if you want to take a look at all of those patches on top of each other.

Comment 4

[(no longer active)]

Attachment: Persist the Request.context attribute in DOM Cache

Took out a debug statement which was accidentally left in.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8584282 [details] [diff] [review]
Persist the Request.context attribute in DOM Cache

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

Starting to feel like maybe the enum asserts should move to a separate header or something, but we can do that later.

Note, the default context for a synthesized Request may change.  I was talking to Anne and he suggested maybe making it empty string by default and having fetch() set it to "fetch".

Comment 6

[(no longer active)]

(In reply to Ben Kelly [:bkelly] from comment #5)
> Starting to feel like maybe the enum asserts should move to a separate
> header or something, but we can do that later.

Let me know if you want me to do that in a separate bug.

> Note, the default context for a synthesized Request may change.  I was
> talking to Anne and he suggested maybe making it empty string by default and
> having fetch() set it to "fetch".

Good to know, but still, as long as we can't synthesize Request objects with context values of our choosing from script, the test strategy will remain the same.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8584282 [details] [diff] [review]
Persist the Request.context attribute in DOM Cache

Hold for clarification from Anne.

Comment 8

[(no longer active)]

(In reply to Ben Kelly [:bkelly] from comment #7)
> Comment on attachment 8584282 [details] [diff] [review]
> Persist the Request.context attribute in DOM Cache
> 
> Hold for clarification from Anne.

How would that change anything about this patch?

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

Anne is now telling me Cache should always sanitize the Request and therefore always produce a context of "fetch".

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

See this spec issue:  https://github.com/slightlyoff/ServiceWorker/issues/664

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8584282 [details] [diff] [review]
Persist the Request.context attribute in DOM Cache

r=me for now.  We can always take it back out if the spec ends up changing.

Comment 12

[(no longer active)]

I somehow managed to accidentally fold this patch into another one, so ended up landing this as part of https://hg.mozilla.org/integration/mozilla-inbound/rev/e7e532d69293.