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

Issue 707282 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Out until 24 Jan
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification

Blocking:
issue 705559



Sign in to add a comment

PlzNavigate needs to use the SKIP_CACHE_VALIDATION for back/forward navigations

Project Member Reported by csharrison@chromium.org, Mar 31 2017

Issue description

This is likely causing the performance regression for back/forward navigations with PlzNavigate enabled. This enhancement causes Chrome to use stale main frames if possible for these types of navigations.

The relevant place to add this header is in UpdateLoadFlagsWithCacheFlags in navigation_request.cc

 

Comment 1 by jam@chromium.org, Mar 31 2017

Thank you for tracking this!

Comment 2 by nasko@chromium.org, Mar 31 2017

Cc: clamy@chromium.org creis@chromium.org
Thanks for spotting this csharrison@!

I found where the problem is. For history navigations, the old path has two cases for back/forward navigation load flags: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp?rcl=e7aca0670b59b9062005a9b5a96cfb2d29743682&l=250. However, the PlzNavigate is missing an else statement: https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_request.cc?rcl=e7aca0670b59b9062005a9b5a96cfb2d29743682&l=87.

I have a CL with a fix and a test for this.

Comment 3 by nasko@chromium.org, Mar 31 2017

Ah, the CL is https://codereview.chromium.org/2780353005/ :).
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 31 2017

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

commit 7ee0fa238b36d1f1a12a240f2caf025559727ec4
Author: nasko <nasko@chromium.org>
Date: Fri Mar 31 21:54:44 2017

Fix history navigations in PlzNavigate to use the proper load flags.

There is a difference in flags between the regular navigation path and
PlzNavigate. This CL fixes the new path to have the same logic as the
existing one.

BUG= 707282 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/7ee0fa238b36d1f1a12a240f2caf025559727ec4/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/7ee0fa238b36d1f1a12a240f2caf025559727ec4/content/browser/frame_host/navigation_request.cc
[add] https://crrev.com/7ee0fa238b36d1f1a12a240f2caf025559727ec4/content/test/data/navigation_controller/page_with_no_cache_header.html
[add] https://crrev.com/7ee0fa238b36d1f1a12a240f2caf025559727ec4/content/test/data/navigation_controller/page_with_no_cache_header.html.mock-http-headers

Comment 5 by nasko@chromium.org, Mar 31 2017

Status: Fixed (was: Started)
I'll resolve this one as fixed, since it has an explicit test for it.

Comment 6 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 7 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Sign in to add a comment