New issue
Advanced search Search tips

Issue 683319 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 348655



Sign in to add a comment

Determine whether CHECK in FrameFetchContext is necessary to ensure referrer is set

Project Member Reported by csharrison@chromium.org, Jan 20 2017

Issue description

In FrameFetchContext::addAdditionalRequestHeaders, we have the following code:

  CHECK_EQ(SecurityPolicy::generateReferrer(request.getReferrerPolicy(),
                                                request.url(),
                                                request.httpReferrer())
                   .referrer,
               request.httpReferrer());


This accounts for ~3.5% of CPU time per resource request, mostly in KURL initialization.

Jochen, it looks like you added this code. Can you give some overview of why it is necessary, or the steps we should do to enable this to be a DCHECK instead of a CHECK? Feel free to WontFix if this is not possible.
 

Comment 1 by jochen@chromium.org, Jan 20 2017

DCHECK is fine, we also check in the network stack again
Blocking: 348655
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f2447403881449544e449c595cad4287b835971

commit 7f2447403881449544e449c595cad4287b835971
Author: csharrison <csharrison@chromium.org>
Date: Mon Jan 23 16:33:02 2017

Migrate expensive referrer CHECK to DCHECK in FrameFetchContext

Profiles on linux show this accounts for ~3.5% of the CPU cost of
requesting a resource. This check is important for security, but as
jochen@ notes in the linked bug, we do the check in the network stack
as well.

BUG= 683319 

Review-Url: https://codereview.chromium.org/2645043004
Cr-Commit-Position: refs/heads/master@{#445386}

[modify] https://crrev.com/7f2447403881449544e449c595cad4287b835971/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

Status: Fixed (was: Untriaged)
Many thanks, jochen@

Sign in to add a comment