chromium/src
0
Fork 0
Code Activity

Revert "Add CHECKs to narrow down HttpCache::DoneWithEntry crash"

Browse Source 
This reverts commit e0254a14b8.

Reason for revert: No longer needed as we have identified the
CHECK failure: https://bugs.chromium.org/p/chromium/issues/detail?id=1348096#c11

Original change's description:
> Add CHECKs to narrow down HttpCache::DoneWithEntry crash
>
> Bug: 1348096
> Change-Id: I1c0679aee42637620b84e5dd784e30828fffd7f5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3788934
> Auto-Submit: Yutaka Hirano <yhirano@chromium.org>
> Reviewed-by: Adam Rice <ricea@chromium.org>
> Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
> Commit-Queue: Adam Rice <ricea@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1029144}

Bug: 1348096
Change-Id: Id92278915f2a6bcc0d96814ded196538db67af68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3818046
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Auto-Submit: Adam Rice <ricea@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1032964}
This commit is contained in:
Adam Rice
2022-08-09 11:36:36 +00:00
committed by Chromium LUCI CQ
parent a328bdd41e
commit 56cddb9124
3 changed files with 8 additions and 33 deletions

29
net/http/http_cache.cc

@ -925,11 +925,7 @@ int HttpCache::DoneWithResponseHeaders(ActiveEntry* entry,
void HttpCache::DoneWithEntry(ActiveEntry* entry,
Transaction* transaction,
bool entry_is_complete,
bool is_partial,
base::SafeRef<ActiveEntry> entry_ref) {
// TODO(crbug.com/1348096): Remove this once the investigation is done.
*entry_ref;
bool is_partial) {
bool is_mode_read_only = transaction->mode() == Transaction::READ;
if (!entry_is_complete && !is_mode_read_only && is_partial)
@ -948,9 +944,6 @@ void HttpCache::DoneWithEntry(ActiveEntry* entry,
return;
}
// TODO(crbug.com/1348096): Remove this once the investigation is done.
*entry_ref;
// Transaction is removed in the headers phase.
if (transaction == entry->headers_transaction) {
entry->headers_transaction = nullptr;
@ -965,9 +958,6 @@ void HttpCache::DoneWithEntry(ActiveEntry* entry,
return;
}
// TODO(crbug.com/1348096): Remove this once the investigation is done.
*entry_ref;
// Transaction is removed in the writing phase.
if (entry->writers && entry->writers->HasTransaction(transaction)) {
entry->writers->RemoveTransaction(transaction,
@ -975,23 +965,10 @@ void HttpCache::DoneWithEntry(ActiveEntry* entry,
return;
}
// TODO(crbug.com/1348096): Remove this once the investigation is done.
*entry_ref;
// Transaction is reading from the entry.
// TODO(crbug.com/1348096): Turn this CHECK into DCHECK once the
// investigation is done.
CHECK(!entry->writers);
DCHECK(!entry->writers);
auto readers_it = entry->readers.find(transaction);
// TODO(crbug.com/1348096): Turn this CHECK into DCHECK once the
// investigation is done.
CHECK(readers_it != entry->readers.end());
// TODO(crbug.com/1348096): Remove this once the investigation is done.
*entry_ref;
DCHECK(readers_it != entry->readers.end());
entry->readers.erase(readers_it);
ProcessQueuedTransactions(entry);
}

3
net/http/http_cache.h

@ -526,8 +526,7 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory {
void DoneWithEntry(ActiveEntry* entry,
Transaction* transaction,
bool entry_is_complete,
bool is_partial,
base::SafeRef<ActiveEntry> entry_ref);
bool is_partial);
// Invoked when writers wants to doom the entry and restart any queued and
// headers transactions.

9
net/http/http_cache_transaction.cc

@ -3485,8 +3485,7 @@ void HttpCache::Transaction::DoneWithEntry(bool entry_is_complete) {
if (!safe_entry_)
return;
cache_->DoneWithEntry(entry(), this, entry_is_complete, partial_ != nullptr,
safe_entry_.value());
cache_->DoneWithEntry(entry(), this, entry_is_complete, partial_ != nullptr);
safe_entry_ = absl::nullopt;
mode_ = NONE; // switch to 'pass through' mode
}
@ -3496,7 +3495,7 @@ void HttpCache::Transaction::DoneWithEntryForRestartWithCache() {
return;
cache_->DoneWithEntry(entry(), this, /*entry_is_complete=*/true,
partial_ != nullptr, safe_entry_.value());
partial_ != nullptr);
safe_entry_ = absl::nullopt;
new_entry_ = nullptr;
}
@ -3524,7 +3523,7 @@ int HttpCache::Transaction::OnCacheReadError(int result, bool restart) {
// or setting mode to NONE at this point by invoking the wrapper
// DoneWithEntry.
cache_->DoneWithEntry(entry(), this, true /* entry_is_complete */,
partial_ != nullptr, safe_entry_.value());
partial_ != nullptr);
safe_entry_ = absl::nullopt;
is_sparse_ = false;
// It's OK to use PartialData::RestoreHeaders here as |restart| is only set
@ -3566,7 +3565,7 @@ void HttpCache::Transaction::DoomPartialEntry(bool delete_object) {
}
cache_->DoneWithEntry(entry(), this, false /* entry_is_complete */,
partial_ != nullptr, safe_entry_.value());
partial_ != nullptr);
safe_entry_ = absl::nullopt;
is_sparse_ = false;
truncated_ = false;