Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(371)

Issue 2847183002: predictors: Introduce GlowplugPredictor. (Closed)

Created:
10 months ago by Benoit L
Modified:
9 months, 2 weeks ago
CC:
chromium-reviews, csharrison+watch_chromium.org, wifiprefetch-reviews_google.com, loading-reviews+metrics_chromium.org, speed-metrics-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

predictors: Introduce LoadingPredictor. This is the first step of the Glowplug refactoring. It extracts the external entry points of Glowplug, and starts to separate the resource prefetch specific parts from the rest. This CL mostly adds the top-level Glowplug classes, and there is no behavior change. This will make further refactoring easier. The Android side client code will be updated in a forthcoming CL. BUG=715525 Review-Url: https://codereview.chromium.org/2847183002 Cr-Commit-Position: refs/heads/master@{#471718} Committed: https://chromium.googlesource.com/chromium/src/+/d999f0f298065398ec9bbc156c177892d74d17f6

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : Rebase. #

Patch Set 4 : . #

Patch Set 5 : . #

Total comments: 17

Patch Set 6 : Address comments. #

Total comments: 11

Patch Set 7 : . #

Patch Set 8 : GlowplugPredictor -> LoadingPredictor #

Unified diffs Side-by-side diffs Delta from patch set Stats (+564 lines, -409 lines) Patch
M chrome/browser/BUILD.gn View 1 2 3 4 5 6 7 2 chunks +6 lines, -2 lines 0 comments Download
M chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer.cc View 1 2 3 4 5 6 7 2 chunks +9 lines, -6 lines 0 comments Download
M chrome/browser/page_load_metrics/observers/resource_prefetch_predictor_page_load_metrics_observer_unittest.cc View 1 2 3 4 5 6 7 3 chunks +5 lines, -3 lines 0 comments Download
M chrome/browser/precache/precache_manager_factory.cc View 1 2 3 4 5 6 7 4 chunks +7 lines, -5 lines 0 comments Download
A chrome/browser/predictors/loading_predictor.h View 1 2 3 4 5 6 7 1 chunk +56 lines, -0 lines 0 comments Download
A chrome/browser/predictors/loading_predictor.cc View 1 2 3 4 5 6 7 1 chunk +37 lines, -0 lines 0 comments Download
A chrome/browser/predictors/loading_predictor_config.h View 1 2 3 4 5 6 7 1 chunk +94 lines, -0 lines 0 comments Download
A chrome/browser/predictors/loading_predictor_config.cc View 1 2 3 4 5 6 7 1 chunk +83 lines, -0 lines 0 comments Download
A chrome/browser/predictors/loading_predictor_factory.h View 1 2 3 4 5 6 7 1 chunk +38 lines, -0 lines 0 comments Download
A chrome/browser/predictors/loading_predictor_factory.cc View 1 2 3 4 5 6 7 1 chunk +49 lines, -0 lines 0 comments Download
M chrome/browser/predictors/predictor_database.cc View 1 2 3 4 5 6 7 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_common.h View 1 2 3 4 5 6 7 4 chunks +11 lines, -78 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_common.cc View 1 2 3 4 5 6 7 5 chunks +40 lines, -103 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_common_unittest.cc View 1 2 3 4 5 6 7 10 chunks +26 lines, -27 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor.h View 1 2 3 4 5 6 7 7 chunks +14 lines, -15 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor.cc View 1 2 3 4 5 6 7 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_android.cc View 1 2 3 4 5 6 7 5 chunks +15 lines, -14 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc View 1 2 3 4 5 6 7 9 chunks +18 lines, -16 lines 0 comments Download
D chrome/browser/predictors/resource_prefetch_predictor_factory.h View 1 chunk +0 lines, -38 lines 0 comments Download
D chrome/browser/predictors/resource_prefetch_predictor_factory.cc View 1 chunk +0 lines, -52 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc View 1 2 3 4 5 6 7 4 chunks +16 lines, -12 lines 0 comments Download
M chrome/browser/predictors/resource_prefetch_predictor_unittest.cc View 1 2 3 4 5 6 7 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/predictors/resource_prefetcher_manager.h View 1 2 3 4 5 6 7 3 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/predictors/resource_prefetcher_manager.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -4 lines 0 comments Download
M chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/profiles/profile_io_data.cc View 1 2 3 4 5 6 7 2 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/ui/tab_helpers.cc View 1 2 3 4 5 6 7 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/predictors/predictors_handler.h View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/predictors/predictors_handler.cc View 1 2 3 4 5 6 7 4 chunks +12 lines, -8 lines 0 comments Download

Messages

Total messages: 47 (33 generated)
Benoit L
9 months, 2 weeks ago (2017-05-11 12:43:23 UTC) #13
alexilin
Thanks! https://codereview.chromium.org/2847183002/diff/80001/chrome/browser/predictors/glowplug_config.h File chrome/browser/predictors/glowplug_config.h (right): https://codereview.chromium.org/2847183002/diff/80001/chrome/browser/predictors/glowplug_config.h#newcode23 chrome/browser/predictors/glowplug_config.h:23: struct GlowplugPredictorConfig { I don't see clear benefits ...
9 months, 2 weeks ago (2017-05-11 16:53:12 UTC) #17
Benoit L
Thanks! All done. https://codereview.chromium.org/2847183002/diff/80001/chrome/browser/predictors/glowplug_config.h File chrome/browser/predictors/glowplug_config.h (right): https://codereview.chromium.org/2847183002/diff/80001/chrome/browser/predictors/glowplug_config.h#newcode23 chrome/browser/predictors/glowplug_config.h:23: struct GlowplugPredictorConfig { On 2017/05/11 16:53:12, ...
9 months, 2 weeks ago (2017-05-12 12:12:53 UTC) #19
alexilin
Thank you Benoit for doing this unpleasant job! lgtm with nits. https://codereview.chromium.org/2847183002/diff/80001/chrome/browser/predictors/glowplug_config.h File chrome/browser/predictors/glowplug_config.h (right): ...
9 months, 2 weeks ago (2017-05-12 14:26:46 UTC) #23
Benoit L
Thanks! For new reviewers: welcome! This is just a refactoring / renaming, hence the large ...
9 months, 2 weeks ago (2017-05-12 15:13:12 UTC) #25
msarda
//chrome/browser/profiles/ LGTM.
9 months, 2 weeks ago (2017-05-12 15:16:51 UTC) #28
Charlie Harrison
page_load_metrics LGTM but maybe you could make the comment in glowplug_predictor.h a bit more prominent ...
9 months, 2 weeks ago (2017-05-12 15:22:09 UTC) #29
Avi (use Gerrit)
On 2017/05/12 15:22:09, Charlie Harrison wrote: > page_load_metrics LGTM but maybe you could make the ...
9 months, 2 weeks ago (2017-05-12 15:31:22 UTC) #30
Charlie Harrison
On 2017/05/12 15:31:22, Avi (ping after 24h) wrote: > On 2017/05/12 15:22:09, Charlie Harrison wrote: ...
9 months, 2 weeks ago (2017-05-12 15:42:02 UTC) #31
Benoit L
Thanks for the comments, you're right. Renamed to LoadingPredictor, since from the high-level comment in ...
9 months, 2 weeks ago (2017-05-12 16:40:16 UTC) #37
Avi (use Gerrit)
lgtm for both the rename and tab_helpers
9 months, 2 weeks ago (2017-05-12 17:01:39 UTC) #38
Charlie Harrison
plm still lgtm
9 months, 2 weeks ago (2017-05-12 17:09:26 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2847183002/140001
9 months, 2 weeks ago (2017-05-15 08:29:57 UTC) #44
commit-bot: I haz the power
9 months, 2 weeks ago (2017-05-15 10:24:11 UTC) #47
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/d999f0f298065398ec9bbc156c17...

Powered by Google App Engine
This is Rietveld 408576698