Do not reuse opaque Resource from a service worker for non no-cors requests

Do not reuse opaque Resource from a service worker for non no-cors requests

RespondWithObserver::responseWasFulfilled() rejects
FetchResponseData::OpaqueType for requests that are not
WebURLRequest::FetchRequestModeNoCORS, but this check is not in MemoryCache.

BUG=625575
Committed: https://crrev.com/77317690ae5f0d6e60ea0b2693085ed5b9b2df09
Cr-Commit-Position: refs/heads/master@{#410052}

[hiroshige, 2016-07-28 03:56:21]

Description was changed from

==========
MemoryCache: Reload Resource when FetchRequestMode doesn't match

BUG=
==========

to

==========
Do not reuse opaque Resource from a service worker for non no-cors requests

BUG=625575
==========

[hiroshige, 2016-07-28 03:56:21]

hiroshige@chromium.org changed reviewers:
+ horo@chromium.org, mkwst@chromium.org, tyoshino@chromium.org

[hiroshige, 2016-07-28 03:59:34]

Description was changed from

==========
Do not reuse opaque Resource from a service worker for non no-cors requests

BUG=625575
==========

to

==========
Do not reuse opaque Resource from a service worker for non no-cors requests

RespondWithObserver::responseWasFulfilled() rejects
FetchResponseData::OpaqueType for requests that are not
WebURLRequest::FetchRequestModeNoCORS, but this check is not in MemoryCache.

BUG=625575
==========

[hiroshige, 2016-07-28 04:01:54]

PTAL.

This corresponds to
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/servic....

If we should also merge another condition in
RespondWithObserver::responseWasFulfilled(), or you have better idea for keeping
RespondWithObserver and MemoryCache consistent, please let me know.

[horo, 2016-07-28 07:29:00]

https://codereview.chromium.org/2177283006/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/2177283006/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:698: if
(existingResource->response().serviceWorkerResponseType() ==
WebServiceWorkerResponseTypeOpaque && request.fetchRequestMode() !=
WebURLRequest::FetchRequestModeNoCORS)
I think we should also check
existingResource->response().wasFetchedViaServiceWorker().

[Mike West, 2016-07-28 09:03:27]

Can you add a test for this behavior?

[hiroshige, 2016-08-05 07:53:14]

Added layout tests. PTAL.

https://codereview.chromium.org/2177283006/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/2177283006/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:698: if
(existingResource->response().serviceWorkerResponseType() ==
WebServiceWorkerResponseTypeOpaque && request.fetchRequestMode() !=
WebURLRequest::FetchRequestModeNoCORS)
On 2016/07/28 07:29:00, horo wrote:
> I think we should also check
> existingResource->response().wasFetchedViaServiceWorker().

Done.

[Mike West, 2016-08-05 10:52:25]

LGTM

[horo, 2016-08-05 12:07:00]

lgtm

[hiroshige, 2016-08-05 12:20:18]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2016-08-05 12:20:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-05 14:06:21]

Description was changed from

==========
Do not reuse opaque Resource from a service worker for non no-cors requests

RespondWithObserver::responseWasFulfilled() rejects
FetchResponseData::OpaqueType for requests that are not
WebURLRequest::FetchRequestModeNoCORS, but this check is not in MemoryCache.

BUG=625575
==========

to

==========
Do not reuse opaque Resource from a service worker for non no-cors requests

RespondWithObserver::responseWasFulfilled() rejects
FetchResponseData::OpaqueType for requests that are not
WebURLRequest::FetchRequestModeNoCORS, but this check is not in MemoryCache.

BUG=625575
==========

[commit-bot: I haz the power, 2016-08-05 14:06:23]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-05 14:07:55]

Description was changed from

==========
Do not reuse opaque Resource from a service worker for non no-cors requests

RespondWithObserver::responseWasFulfilled() rejects
FetchResponseData::OpaqueType for requests that are not
WebURLRequest::FetchRequestModeNoCORS, but this check is not in MemoryCache.

BUG=625575
==========

to

==========
Do not reuse opaque Resource from a service worker for non no-cors requests

RespondWithObserver::responseWasFulfilled() rejects
FetchResponseData::OpaqueType for requests that are not
WebURLRequest::FetchRequestModeNoCORS, but this check is not in MemoryCache.

BUG=625575

Committed: https://crrev.com/77317690ae5f0d6e60ea0b2693085ed5b9b2df09
Cr-Commit-Position: refs/heads/master@{#410052}
==========

[commit-bot: I haz the power, 2016-08-05 14:07:56]

Patchset 4 (id:??) landed as
https://crrev.com/77317690ae5f0d6e60ea0b2693085ed5b9b2df09
Cr-Commit-Position: refs/heads/master@{#410052}

[tyoshino (SeeGerritForStatus), 2016-08-09 06:42:58]

lgtm