Issue metadata
Sign in to add a comment
|
PlzNavigate needs to use the SKIP_CACHE_VALIDATION for back/forward navigations |
||||||||||||||||||||||||
Issue descriptionThis 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
,
Mar 31 2017
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.
,
Mar 31 2017
Ah, the CL is https://codereview.chromium.org/2780353005/ :).
,
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
,
Mar 31 2017
I'll resolve this one as fixed, since it has an explicit test for it.
,
Nov 7 2017
,
Nov 7 2017
Apologies, applied the wrong component in bulk. |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by jam@chromium.org
, Mar 31 2017