Currently our service worker code runs the Request::Constructor() when creating a FetchEvent: https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#3818 The spec actually just says: "Let r be a new Request object associated with request." Using the constructor is problematic because it has all kinds of extra checking to place restrictions on content script. For example, it throws on URLs with userpass. We still need to be able to create these Request objects in order to intercept them, though.
This also effects Cache API when reading a Request out of the database: https://bugzilla.mozilla.org/show_bug.cgi?id=1201664 We should be able to intercept a URL with userpass, store it in the Cache, and then pull it back out of the Cache.
Comment 1 should reference this link instead of the bug URL again: https://dxr.mozilla.org/mozilla-central/source/dom/cache/TypeUtils.cpp?offset=0#488
Oh, actually that path is only hit for putting things into Cache. Deserializing from Cache does not run the Request::Constructor() again. Sorry for my confusion.
I'm happy to help somebody solve this! We should have another static method that creates a Response object given a set of arguments, and we can call that from Constructor.
I think getting the right restrictions on the Request might be a bit tricky. It's not clear to me what parts of the constructor we were relying on and its not really obvious from the spec.
Note, we also already have a lot of the code necessary to do this. The SWM can create an InternalRequest to represent the channel and then just do |new Request(global, internalRequest)|. This is what Cache currently does: https://dxr.mozilla.org/mozilla-central/source/dom/cache/TypeUtils.cpp#367 So really we just need a ToInternalRequestFromChannel(nsIChannel) method.
Doesn't sound like I'm a good mentor here.
This enables us to avoid a number of checks in Request::Constructor which don't make sense for exposing FetchEvent.request.
Attachment #8707661 - Flags: review?(bkelly)
Comment on attachment 8707661 [details] [diff] [review] Avoid using Request's constructor when creating FetchEvent.request Review of attachment 8707661 [details] [diff] [review]: ----------------------------------------------------------------- I'm slightly worried there are invariants in the fetch spec assumed for non-constructor Request objects that we may not capture here. But the fetch spec does not lay them out in a single place like it does for the constructor. I guess we must rely on our tests for now. r=me
Attachment #8707661 - Flags: review?(bkelly) → review+
You need to log in before you can comment on or make changes to this bug.