Commit f71656d5 authored by Dominic Farolino's avatar Dominic Farolino Committed by Chromium LUCI CQ

Revert "Reland "[Prefetch]: DCHECK if recursive prefetch token is set incorrectly""

This reverts commit 422bc672.

Reason for revert: We've collected enough info

Original change's description:
> Reland "[Prefetch]: DCHECK if recursive prefetch token is set incorrectly"
>
> This reverts commit 2638a6e6.
>
> Reason for revert: We thought we had collected enough crash data, however crbug.com/1122182 was not actually fixed yet, so the minidumps did not actually contain the information we needed. agrieve@ has informed me that the issue should be fixed now, and we'll confirm whether or not that is the case by:
>  1.) Reverting this change
>  2.) Collecting minidumps
>  3.) Examining the minidumps to confirm that the debugging information exists
>
> Original change's description:
> > Revert "[Prefetch]: DCHECK if recursive prefetch token is set incorrectly"
> >
> > This reverts commit b344652b.
> > Reason for revert: We have collected enough crashdumps now, and this is
> > causing crash metrics to be skewed as it's getting counted as a real
> > crash.
> >
> > Bug: 1132770
> > Change-Id: Ia2e7a92bea2ebeca01c62ca9af73d3fe014685f6
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2505985
> > Auto-Submit: Mugdha Lakhani <nator@chromium.org>
> > Commit-Queue: Charlie Harrison <csharrison@chromium.org>
> > Reviewed-by: Charlie Harrison <csharrison@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#822189}
>
> TBR=csharrison@chromium.org,nator@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> TBR: charrison@chromium.org
> Bug: 1132770
> Change-Id: I31ac96c2c5f4f0738fd2d8c4e60236b2bc0c35a3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2513109
> Reviewed-by: Charlie Harrison <csharrison@chromium.org>
> Reviewed-by: Mugdha Lakhani <nator@chromium.org>
> Reviewed-by: Dominic Farolino <dom@chromium.org>
> Commit-Queue: Dominic Farolino <dom@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#829091}

TBR=csharrison@chromium.org,dom@chromium.org,nator@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1132770
Change-Id: Ie8515e382ba8d4610aaeb3026909bec63a1e9405
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568110
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: default avatarDominic Farolino <dom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832416}
parent 6204cae0
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#include "content/browser/loader/prefetch_url_loader_service.h" #include "content/browser/loader/prefetch_url_loader_service.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/debug/dump_without_crashing.h"
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/time/default_tick_clock.h" #include "base/time/default_tick_clock.h"
#include "content/browser/loader/prefetch_url_loader.h" #include "content/browser/loader/prefetch_url_loader.h"
...@@ -29,14 +28,6 @@ ...@@ -29,14 +28,6 @@
#include "third_party/blink/public/common/renderer_preferences/renderer_preferences.h" #include "third_party/blink/public/common/renderer_preferences/renderer_preferences.h"
#include "third_party/blink/public/mojom/loader/resource_load_info.mojom-shared.h" #include "third_party/blink/public/mojom/loader/resource_load_info.mojom-shared.h"
namespace {
void DumpWithoutCrashing(const network::ResourceRequest& request) {
DEBUG_ALIAS_FOR_GURL(prefetch_buf, request.url);
DEBUG_ALIAS_FOR_GURL(initiator_buf, request.request_initiator->GetURL());
base::debug::DumpWithoutCrashing();
}
} // namespace
namespace content { namespace content {
struct PrefetchURLLoaderService::BindContext { struct PrefetchURLLoaderService::BindContext {
...@@ -171,18 +162,9 @@ void PrefetchURLLoaderService::CreateLoaderAndStart( ...@@ -171,18 +162,9 @@ void PrefetchURLLoaderService::CreateLoaderAndStart(
// Recursive prefetch from a cross-origin main resource prefetch. // Recursive prefetch from a cross-origin main resource prefetch.
if (resource_request.recursive_prefetch_token) { if (resource_request.recursive_prefetch_token) {
// A request's |recursive_prefetch_token| is only provided if the request is // TODO(crbug.com/1132770): Figure out why we're seeing this condition hold
// a recursive prefetch. This means it is expected that the current // true in the field.
// context's |cross_origin_factory| was already created.
if (!current_context.cross_origin_factory) { if (!current_context.cross_origin_factory) {
// This could happen due to a compromised renderer passing in a recursive
// prefetch token for a request that's not a recursive prefetch. Cancel
// the request.
DVLOG(1) << "Recursive prefetch token unexpectedly set.";
DumpWithoutCrashing(resource_request);
mojo::Remote<network::mojom::URLLoaderClient>(std::move(client))
->OnComplete(
network::URLLoaderCompletionStatus(net::ERR_INVALID_ARGUMENT));
return; return;
} }
...@@ -196,8 +178,6 @@ void PrefetchURLLoaderService::CreateLoaderAndStart( ...@@ -196,8 +178,6 @@ void PrefetchURLLoaderService::CreateLoaderAndStart(
// a request in a special way. We'll cancel the request. // a request in a special way. We'll cancel the request.
if (isolation_info_iterator == if (isolation_info_iterator ==
current_context.prefetch_isolation_infos.end()) { current_context.prefetch_isolation_infos.end()) {
DVLOG(1) << "Recursive prefetch request is missing prefetch isolation";
DumpWithoutCrashing(resource_request);
mojo::Remote<network::mojom::URLLoaderClient>(std::move(client)) mojo::Remote<network::mojom::URLLoaderClient>(std::move(client))
->OnComplete( ->OnComplete(
network::URLLoaderCompletionStatus(net::ERR_INVALID_ARGUMENT)); network::URLLoaderCompletionStatus(net::ERR_INVALID_ARGUMENT));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment