Auto merge of #24401 - gterzian:ensure_done_chan_is_none_when_revalidating, r=jdm

Fetch: ensure consistency between response, done-chan, and cache state.

This ensures that when a cached response is not used because it requires revalidation, the done chan is set back to `None`(since the cache might have set it to `Some` when constructing the response requiring revalidation).

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #24399 (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24401)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-10-09 10:53:17 -04:00 committed by GitHub
commit b34fc13eee
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -1092,20 +1092,6 @@ fn http_network_or_cache_fetch(
response_from_cache.needs_validation,
),
};
if cached_response.is_none() {
// Ensure the done chan is not set if we're not using the cached response,
// as the cache might have set it to Some if it constructed a pending response.
*done_chan = None;
// Update the cache state, incrementing the pending store count,
// or starting the count.
if let HttpCacheEntryState::PendingStore(i) = *state {
let new = i + 1;
*state = HttpCacheEntryState::PendingStore(new);
} else {
*state = HttpCacheEntryState::PendingStore(1);
}
}
if needs_revalidation {
revalidating_flag = true;
// Substep 5
@ -1124,6 +1110,20 @@ fn http_network_or_cache_fetch(
// Substep 6
response = cached_response;
}
if response.is_none() {
// Ensure the done chan is not set if we're not using the cached response,
// as the cache might have set it to Some if it constructed a pending response.
*done_chan = None;
// Update the cache state, incrementing the pending store count,
// or starting the count.
if let HttpCacheEntryState::PendingStore(i) = *state {
let new = i + 1;
*state = HttpCacheEntryState::PendingStore(new);
} else {
*state = HttpCacheEntryState::PendingStore(1);
}
}
}
}
// Notify the next thread waiting in line, if there is any.
@ -1223,6 +1223,9 @@ fn http_network_or_cache_fetch(
.map_or(false, |s| s.0 == StatusCode::NOT_MODIFIED)
{
if let Ok(mut http_cache) = context.state.http_cache.write() {
// Ensure done_chan is None,
// since the network response will be replaced by the revalidated stored one.
*done_chan = None;
response = http_cache.refresh(&http_request, forward_response.clone(), done_chan);
wait_for_cached_response(done_chan, &mut response);
}