New issue
Advanced search Search tips

Issue 668714 link

Starred by 3 users

Issue metadata

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

Blocking:
issue 632361



Sign in to add a comment

Allow setting net/ flags for NavigationHandle

Project Member Reported by pasko@chromium.org, Nov 25 2016

Issue description

For NoStatePrefetch we want all redirects to be sent to the network stack with net::LOAD_PREFETCH.

In Pre-PlzNavigate as soon as WebContentsImpl::RenderFrameCreated() is called, the PrerenderMsg_SetIsPrerendering tells the renderer that the latter would need to set the flag during navigation.

With PlzNavigate the requests are made from the browser without a way to override net flags. The way to override it would be in a new NavigationThrottle's WillStartRequest() to do something like:

navigation_handle->SetNetFlags(new::LOAD_PREFETCH)
 

Comment 1 by pasko@chromium.org, Nov 25 2016

Blocking: 632361

Comment 2 by pasko@chromium.org, Nov 28 2016

Just occurred to me that URLRequest->priority() also needs to be overridden for NoStatePrefetch navigations. When the user-initiated navigation appears for the same URL/whatever, and the prefetch has not finished, we don't need to update the priority, but will probably want to cancel the low-priority navigation. My understanding is that it should be done with the NavigationThrottle.
Cc: -droger@chromium.org clamy@chromium.org
Owner: droger@chromium.org
Status: Started (was: Assigned)
As per discussion with clamy@, I'm going to try passing the load flags in  ChromeNavigationUIData.

Comment 6 by droger@chromium.org, Dec 16 2016

Status: Fixed (was: Started)
This is fixed.
The test still pass, but now without cheating.

Comment 7 by clamy@chromium.org, Dec 22 2016

The bug is marked as fixed, but when I run NoStatePrefetchBrowserTest.PrefetchLoadFlag & NoStatePrefetchBrowserTest.Prefetch301LoadFlags with PlzNavigate enabled, the load flags are still not set. Is this normal?

Comment 8 by pasko@chromium.org, Dec 22 2016

Status: Assigned (was: Fixed)
right, let's keep it open, as the browser-side-navigation.linux.browser_tests.filter mentions this bug

Comment 9 by pasko@chromium.org, Dec 22 2016

There is a proposed fix which droger@ did not send to me yet: https://codereview.chromium.org/2566163002/
Status: Fixed (was: Assigned)
Project Member

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

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

commit c2cdce4467fb89b77f62b3b042b5cdfd19739f13
Author: pasko <pasko@chromium.org>
Date: Mon Jan 23 13:18:41 2017

Prerender: Add thread checks to PrerenderResourceThrottle

These are small followup changes for: https://codereview.chromium.org/2602573002/

Adds DCHECK_CURRENTLY_ON() to the methods of the class as requested in the
previous codereview. Also fixes a typo.

BUG= 668714 

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

[modify] https://crrev.com/c2cdce4467fb89b77f62b3b042b5cdfd19739f13/chrome/browser/prerender/prerender_resource_throttle.cc
[modify] https://crrev.com/c2cdce4467fb89b77f62b3b042b5cdfd19739f13/chrome/browser/prerender/prerender_resource_throttle.h

Sign in to add a comment