Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2823613002:
PlzNavigate: add dump to understand pdf navigation cancellation (Closed)
DescriptionPlzNavigate: add dump to understand pdf navigation cancellation
This CL adds a DumpWithoutCrashing to try to understand why navigations
to pdf are cancelled when PlzNavigate is enabled.
BUG=709771
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
Review-Url: https://codereview.chromium.org/2823613002
Cr-Commit-Position: refs/heads/master@{#465418}
Committed: https://chromium.googlesource.com/chromium/src/+/70fd878b6b7abc27c01a68f35b82bee5f51ba03e
Patch Set 1 #
Total comments: 2
Patch Set 2 : Addressed comments #Messages
Total messages: 18 (10 generated)
Description was changed from ========== PlzNavigate: add dump to understand pdf navigation cancellation This CL adds a DumpWithoutCrashing to try to understand why navigations to pdf are cancelled when PlzNavigate is enabled. BUG=709771 ========== to ========== PlzNavigate: add dump to understand pdf navigation cancellation This CL adds a DumpWithoutCrashing to try to understand why navigations to pdf are cancelled when PlzNavigate is enabled. BUG=709771 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
Description was changed from ========== PlzNavigate: add dump to understand pdf navigation cancellation This CL adds a DumpWithoutCrashing to try to understand why navigations to pdf are cancelled when PlzNavigate is enabled. BUG=709771 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== PlzNavigate: add dump to understand pdf navigation cancellation This CL adds a DumpWithoutCrashing to try to understand why navigations to pdf are cancelled when PlzNavigate is enabled. BUG=709771 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ==========
clamy@chromium.org changed reviewers: + nasko@chromium.org
@nasko: PTAL If you have a better idea as to how to get more info on the bug, I'm interested!
https://codereview.chromium.org/2823613002/diff/1/content/browser/frame_host/... File content/browser/frame_host/navigation_request.cc (right): https://codereview.chromium.org/2823613002/diff/1/content/browser/frame_host/... content/browser/frame_host/navigation_request.cc:615: if (common_params_.url.spec().find(".pdf") != std::string::npos) Shouldn't this be searching only at the end of the path component? Otherwise anything in the query string or the domain part can trip it, which I think is a bit too lax.
The CQ bit was checked by clamy@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2823613002/diff/1/content/browser/frame_host/... File content/browser/frame_host/navigation_request.cc (right): https://codereview.chromium.org/2823613002/diff/1/content/browser/frame_host/... content/browser/frame_host/navigation_request.cc:615: if (common_params_.url.spec().find(".pdf") != std::string::npos) On 2017/04/17 21:30:05, nasko wrote: > Shouldn't this be searching only at the end of the path component? Otherwise > anything in the query string or the domain part can trip it, which I think is a > bit too lax. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM
LGTM
The CQ bit was checked by nasko@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492557118640210,
"parent_rev": "6b7609d689c7b16d0479180635642a298d52b6b3", "commit_rev":
"70fd878b6b7abc27c01a68f35b82bee5f51ba03e"}
Message was sent while issue was closed.
Description was changed from ========== PlzNavigate: add dump to understand pdf navigation cancellation This CL adds a DumpWithoutCrashing to try to understand why navigations to pdf are cancelled when PlzNavigate is enabled. BUG=709771 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation ========== to ========== PlzNavigate: add dump to understand pdf navigation cancellation This CL adds a DumpWithoutCrashing to try to understand why navigations to pdf are cancelled when PlzNavigate is enabled. BUG=709771 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2823613002 Cr-Commit-Position: refs/heads/master@{#465418} Committed: https://chromium.googlesource.com/chromium/src/+/70fd878b6b7abc27c01a68f35b82... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/70fd878b6b7abc27c01a68f35b82...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2837573002/ by clamy@chromium.org. The reason for reverting is: Reverting since we got the data needed.. |
