Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove NOTREACHED in HttpCache::Transaction #38326

Merged
merged 1 commit into from Feb 3, 2023

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Feb 2, 2023

net::HttpCache::Transaction::BeginCacheRead had a NOTREACHED() statement
which is reached if a range request is sent with "only-if-cached" as the
cache mode.

Remove the NOTREACHED() and permit the fallthrough behaviour of
returning ERR_CACHE_MISS to happen instead.

Add unit and web-platform tests for this case. The wpt fails because it
expects the fetch to succeed, whereas our implementation returns
ERR_CACHE_MISS.

BUG=1408233

Change-Id: I1023712b1a0d3642e6c980dd12dba81440d4fc13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4218212
Reviewed-by: Nidhi Jaju <nidhijaju@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100870}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

net::HttpCache::Transaction::BeginCacheRead had a NOTREACHED() statement
which is reached if a range request is sent with "only-if-cached" as the
cache mode.

Remove the NOTREACHED() and permit the fallthrough behaviour of
returning ERR_CACHE_MISS to happen instead.

Add unit and web-platform tests for this case. The wpt fails because it
expects the fetch to succeed, whereas our implementation returns
ERR_CACHE_MISS.

BUG=1408233

Change-Id: I1023712b1a0d3642e6c980dd12dba81440d4fc13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4218212
Reviewed-by: Nidhi Jaju <nidhijaju@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100870}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants