Make the resource_prefetch_predictor work with PlzNavigate |
||||
Issue description
The resource_prefetch_predictor tracks navigation through {child_id, frame_id}. This is not the correct way to do it.
In addition, there is logic to track whether the same tab has navigated somewhere else, to discard abandonned navigations. This is a bit brittle, and should be tracked in a different way.
The stable ID to use is frame_tree_node_id.
,
Dec 5 2016
Hi, It is in progress, ahemery@ is handling it, there is a preliminary draft CL adding plumbing in review (https://codereview.chromium.org/2545943003/), and the actual support is not far away.
,
Dec 6 2016
Great, thanks. If this issue can be fixed this week, that'd be awesome. Our goal is to go to canary this month, so we're trying to fix any remaining blockers.
,
Dec 6 2016
For canary, this is probably not a blocker, as the predictor is currently only enabled through command-line flags.
,
Dec 6 2016
ah, yes you're right in that case it's not blocking :)
,
Dec 21 2016
,
Dec 21 2016
Working on the final CL for this bug. You can find it at https://codereview.chromium.org/2587443002. To solve the problem we have used the session ids linked to tabs. We added tests that cover potential problematic cases such as cross process navigation and navigating to new windows. If no further problem arises, this should be landed very soon.
,
Dec 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628 commit 62729f8a7e996d8f6b0451e8bcfcd3a124fd2628 Author: ahemery <ahemery@chromium.org> Date: Fri Dec 23 10:32:47 2016 predictors: Make speculative_prefetch_predictor work with PlzNavigate Summary -------- We are now tracking navigations for the ResourcePrefetchPredictor using the tab ids instead of render process and host. Added browser tests to test edge cases that could break the tab_id usage. Details -------- - WebContents is now used to extract the session_id with help from SessionTabHelper::IdForTab() instead of render_frame_host_id + render_process_id - Reworked unit_tests to use this session_id instead - Reworked browser_tests to test for the following cases : Cross Domain Navigation inducing cross process behavior and Navigating to new tabs/window/popup. BUG= 650246 Review-Url: https://codereview.chromium.org/2587443002 Cr-Commit-Position: refs/heads/master@{#440615} [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/net/resource_prefetch_predictor_observer.cc [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/predictors/resource_prefetch_common.cc [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/predictors/resource_prefetch_common.h [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/predictors/resource_prefetch_predictor.cc [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/predictors/resource_prefetch_predictor_test_util.cc [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/predictors/resource_prefetch_predictor_test_util.h [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/chrome/browser/predictors/resource_prefetcher_unittest.cc [modify] https://crrev.com/62729f8a7e996d8f6b0451e8bcfcd3a124fd2628/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter
,
Dec 26 2016
is this fixed now?
,
Dec 28 2016
Fix has been committed. Marking this as fixed. |
||||
►
Sign in to add a comment |
||||
Comment 1 by jam@chromium.org
, Dec 5 2016