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

Status: RESOLVED FIXED

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

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Respect skip serviceworker flag

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

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

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

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?

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(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.

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(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.

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ad380221ba6

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7ad380221ba6