Make sure all responses have a URL list #322
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In #146 I tried to let service worker responses that were not synthetic to keep their url list. However, the way I fixed that regressed url lists for a number of responses: HTTP cache responses, about URL responses, etc. This fix instead tries to keep the old setup and only overwrites the url list of a response if it’s empty. Fixes #312.
|
Would appreciate review from @Manishearth and @wanderview. |
|
LGTM |
|
Implementing this PR doesn't cause additional failures in Servo. It doesn't cause any additional test passes, either, which is curious; we probably want more tests for this. |
Manishearth
added a commit
to Manishearth/servo
that referenced
this pull request
Jun 9, 2016
Manishearth
added a commit
to Manishearth/servo
that referenced
this pull request
Jun 9, 2016
|
LGTM. @Manishearth, to my knowledge neither chrome nor firefox implement this "if url list is empty" check yet. For us this is: https://bugzilla.mozilla.org/show_bug.cgi?id=1222008 Edit: So that is why no one has written a WPT test yet. |
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #146 I tried to let service worker responses that were not synthetic
to keep their url list. However, the way I fixed that regressed url
lists for a number of responses: HTTP cache responses, about URL
responses, etc.
This fix instead tries to keep the old setup and only overwrites the
url list of a response if it’s empty.
Fixes #312.