New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 690632 link

Starred by 1 user
Status: Started
Owner:
█ y...@yoav.ws
Last visit > 30 days ago
Cc:
kinuko@chromium.org
csharrison@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Remove unneeded type matching condition from requestResource

Project Member Reported by y...@yoav.ws, Feb 9 2017 
Chrome Version: M58
OS: N/A

requestResource() currently has a type matching condition which is not needed assuming determineRevalidationPolicy is not bypassing type checks from speculative preloads.
We need to remove that condition, and make sure the type check happens in determineRevalidationPolicy().
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 10 2017 

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

commit 25b17c1a107cbc956487b5a6bb4698b986f6f035
Author: yoav <yoav@yoav.ws>
Date: Fri Feb 10 05:05:08 2017

Remove type mismatch condition in requestResource

This CL removes the type matching condition in ResourceFetcher::requestResource()
which shouldn't be hit and replaces it with a CHECK to make sure it is actually never
hit.

BUG=690632

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

[modify] https://crrev.com/25b17c1a107cbc956487b5a6bb4698b986f6f035/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

Comment 2 by y...@yoav.ws, Mar 1 2017

Status: Fixed (was: Started)

Comment 3 by csharrison@chromium.org, Mar 1 2017 

Do you mind keeping it open until the CHECK is removed?

Comment 4 by y...@yoav.ws, Mar 1 2017

Status: Started (was: Fixed)
sure

Comment 5 by csharrison@chromium.org, Mar 1 2017 

I just checked our crash dashboard for any and there are none so far, but we should wait to be sure.

Comment 6 by y...@yoav.ws, Mar 1 2017 

When can we safely turn the CHECK to DCHECK? When 58 hits stable?

Comment 7 by csharrison@chromium.org, Mar 1 2017 

I don't think there is a hard fast rule. Let's at least wait until M58 reaches beta for a little bit.
►

Sign in to add a comment