1209095 - FetchEvent.respondWith() should accept opaqueredirect Response whenever FetchEvent.request.redirect is "manual"

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

See:

  https://github.com/whatwg/fetch/issues/127

Currently we block opaqueredirect in these cases unless the request is a navigation.

Comment 1

[Josh Matthews [:jdm]]

Attachment: Accept opaqueredirection fetch results if the request redirection type is manual

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8699260 [details] [diff] [review]
Accept opaqueredirection fetch results if the request redirection type is manual

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

r=me with comments addressed.  Thanks!

::: dom/workers/ServiceWorkerEvents.cpp
@@ +575,5 @@
>        mRequestURL, modeString);
>      return;
>    }
>  
> +  if (mRequestRedirectMode != RequestRedirect::Manual && response->Type() == ResponseType::Opaqueredirect) {

nit: line length

::: testing/web-platform/mozilla/tests/service-workers/service-worker/resources/fetch-event-redirect-iframe.html
@@ +4,5 @@
>    var data = evt.data;
>    fetch(new Request(data.url, data.request_init)).then(function(response) {
>      if (data.request_init.mode === 'no-cors' && data.redirect_dest != 'same-origin') {
> +      var expected_type = data.expected_type || 'opaque';
> +      if (response.type === expected_type) {

I believe all the test cases have an expected_type, so you shouldn't need the || clause getting the expected_type.

Also, I think we should only permit this to pass if the type is opaque or opaqueredirect.

So maybe change this to something like:

  if (response.type === data.expected_type &&
      (response.type === 'opaque' || response.type === 'opaqueredirect')) {
    // success
  }

Comment 3

[Josh Matthews [:jdm]]

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

Comment 4

[Josh Matthews [:jdm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b068d21a21087b21042d106a9fab94cab3f3bcf0
Bug 1209095 - Accept opaqueredirection fetch results if the request redirection type is manual. r=bkelly

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b068d21a2108