NOTREACHED in chrome_pdf::DocumentLoader::DidOpen |
|||||||
Issue description
Hit this in the Albatross DCHECK-on canary builds.
void DocumentLoader::DidOpen(int32_t result) {
if (result != PP_OK) {
NOTREACHED(); <<< HERE
client_->OnDocumentFailed();
return;
}
if (!ResponseStatusSuccess(loader_)) {
client_->OnDocumentFailed();
return;
}
is_multipart_ = false;
current_chunk_size_ = 0;
current_chunk_read_ = 0;
pp::Var headers_var = loader_.GetResponseInfo().GetHeaders();
std::string headers;
if (headers_var.is_string())
headers = headers_var.AsString();
std::string boundary = GetMultiPartBoundary(headers);
if (!boundary.empty()) {
// Leave position untouched for now, when we read the data we'll get it.
is_multipart_ = true;
multipart_boundary_ = boundary;
} else {
// Need to make sure that the server returned a byte-range, since it's
// possible for a server to just ignore our byte-range request and just
// return the entire document even if it supports byte-range requests.
// i.e. sniff response to
// http://www.act.org/compass/sample/pdf/geometry.pdf
current_pos_ = 0;
uint32_t start_pos;
uint32_t end_pos;
if (GetByteRange(headers, &start_pos, &end_pos)) {
current_pos_ = start_pos;
if (end_pos && end_pos > start_pos)
current_chunk_size_ = end_pos - start_pos + 1;
} else {
partial_document_ = false;
}
}
ReadMore();
}
,
Sep 1 2017
For the moment I cannot, as this crash wasn't uploaded, and I broke the "upload now" button.
,
Sep 5 2017
I'm not sure what Albatross is? What did you do in order to trigger the dcheck? It seems like your document failed to load? Was the navigation cancelled? How do we repro this issue?
,
Sep 5 2017
Albatross https://bugs.chromium.org/p/chromium/issues/detail?id=596231 - we'll be shipping official builds to the Canary channel with DCHECKs enabled. I can't say what happened here, and I can't give you a repro. I'll upload the crash at least.
,
Sep 5 2017
See crash/259e4497e90ca979.
,
Sep 5 2017
rharrison@ can you see if you can figure out how this gets triggered? If we can't determine how to repro, I'm not sure what we can do here.
,
Sep 5 2017
I'd recommend removing the DCHECK, just like in r496448. Having a NOTREACHED() and then handling it is a bad pattern. Probably because it's old code.
,
Sep 6 2017
,
Sep 8 2017
,
Sep 8 2017
I agree that we should remove the NOTREACHED. The branch that this on is a) handled gracefully, and b) is a standard error path for if loading a resource @ a URL fails.
,
Sep 11 2017
,
Sep 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/24428257e9aa3f0effbed3011ecb8a71bbe40b39 commit 24428257e9aa3f0effbed3011ecb8a71bbe40b39 Author: Ryan Harrison <rharrison@chromium.org> Date: Mon Sep 11 20:54:39 2017 Remove NOTREACHED() on branch that is handled BUG= chromium:761380 Change-Id: I0f4bb7d9fe64964fc0e5104d0e21e7cc7f44291c Reviewed-on: https://chromium-review.googlesource.com/657567 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Ryan Harrison <rharrison@chromium.org> Cr-Commit-Position: refs/heads/master@{#501034} [modify] https://crrev.com/24428257e9aa3f0effbed3011ecb8a71bbe40b39/pdf/document_loader.cc |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by w...@chromium.org
, Sep 1 2017