Fetch API: Set skip serviceworker flag for fetch() invoked in ServiceWorkers

RESOLVED FIXED in mozilla38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nsm, Assigned: nsm)

Tracking

33 Branch
mozilla38
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Created attachment 8545526 [details] [diff] [review]
Respect skip serviceworker flag
Attachment #8545526 - Flags: review?(bkelly)
Assignee: nobody → nsm.nikhil
Status: NEW → ASSIGNED
Comment on attachment 8545526 [details] [diff] [review]
Respect skip serviceworker flag

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

One question, but otherwise looks good to me.  Flag Andrea for DOM peer sign off.

::: dom/fetch/FetchDriver.cpp
@@ +373,5 @@
> +  // While the spec also gates on the client being a ServiceWorker, we can't
> +  // infer that here. Instead we rely on callers to set the flag correctly.
> +  if (mRequest->SkipServiceWorker()) {
> +    nsCOMPtr<nsIHttpChannelInternal> internalChan = do_QueryInterface(httpChan);
> +    internalChan->ForceNoIntercept();

Do you need to do this for redirect channels as well?

::: dom/fetch/InternalRequest.h
@@ +135,5 @@
>      mReferrer.Assign(aReferrer);
>    }
>  
>    bool
> +  SkipServiceWorker()

const
Attachment #8545526 - Flags: review?(bkelly)
Attachment #8545526 - Flags: review?(amarchesini)
Attachment #8545526 - Flags: review+
Also, it seems we need to set the skip service worker if a ServiceWorker is consulted and declines to override the network response.  Is that something Josh's code will need to do?
Attachment #8545526 - Flags: review?(amarchesini) → review+
(In reply to Ben Kelly [:bkelly] from comment #2)
> Comment on attachment 8545526 [details] [diff] [review]
> Respect skip serviceworker flag
> 
> Review of attachment 8545526 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> One question, but otherwise looks good to me.  Flag Andrea for DOM peer sign
> off.
> 
> ::: dom/fetch/FetchDriver.cpp
> @@ +373,5 @@
> > +  // While the spec also gates on the client being a ServiceWorker, we can't
> > +  // infer that here. Instead we rely on callers to set the flag correctly.
> > +  if (mRequest->SkipServiceWorker()) {
> > +    nsCOMPtr<nsIHttpChannelInternal> internalChan = do_QueryInterface(httpChan);
> > +    internalChan->ForceNoIntercept();
> 
> Do you need to do this for redirect channels as well?

I've filed a followup to test it once we can actually test this.
(In reply to Ben Kelly [:bkelly] from comment #3)
> Also, it seems we need to set the skip service worker if a ServiceWorker is
> consulted and declines to override the network response.  Is that something
> Josh's code will need to do?

The interception code will have to make sure it sets SkipServiceWorker() on the request then.
https://hg.mozilla.org/mozilla-central/rev/7ad380221ba6
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.