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

Issue 2847183002: predictors: Introduce GlowplugPredictor. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 4 weeks ago by Benoit L
Modified:
2 months, 1 week 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
Trybot results:  win_clang   linux_chromium_asan_rel_ng   win_clang   win_chromium_x64_rel_ng   mac_chromium_rel_ng   win_chromium_compile_dbg_ng   win_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-simulator-xcode-clang   ios-simulator   ios-device-xcode-clang   linux_chromium_tsan_rel_ng   linux_chromium_rel_ng   ios-device   linux_chromium_chromeos_rel_ng   linux_chromium_compile_dbg_ng   linux_chromium_asan_rel_ng   linux_chromium_chromeos_ozone_rel_ng   chromeos_daisy_chromium_compile_only_ng   chromium_presubmit   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   cast_shell_android   linux_android_rel_ng   android_cronet   android_n5x_swarming_rel   android_clang_dbg_recipe   android_compile_dbg   android_arm64_dbg_recipe   win_chromium_x64_rel_ng   linux_chromium_rel_ng   win_chromium_x64_rel_ng   win_clang   win_chromium_rel_ng   win_chromium_compile_dbg_ng   mac_chromium_rel_ng   mac_chromium_compile_dbg_ng   ios-simulator-xcode-clang   ios-device-xcode-clang   ios-device   ios-simulator   linux_chromium_tsan_rel_ng   linux_chromium_compile_dbg_ng   linux_chromium_rel_ng   linux_chromium_chromeos_rel_ng   linux_chromium_chromeos_ozone_rel_ng   linux_chromium_asan_rel_ng   chromium_presubmit   chromeos_daisy_chromium_compile_only_ng   chromeos_amd64-generic_chromium_compile_only_ng   cast_shell_linux   linux_android_rel_ng   cast_shell_android   android_n5x_swarming_rel   android_cronet   android_compile_dbg   android_clang_dbg_recipe   android_arm64_dbg_recipe 
Commit queue not available (can’t edit this change).

Messages

Total messages: 47 (33 generated)
Benoit L
2 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 ...
2 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, ...
2 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): ...
2 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 ...
2 months, 2 weeks ago (2017-05-12 15:13:12 UTC) #25
msarda
//chrome/browser/profiles/ LGTM.
2 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 ...
2 months, 2 weeks ago (2017-05-12 15:22:09 UTC) #29
Avi (ping after 24h)
On 2017/05/12 15:22:09, Charlie Harrison wrote: > page_load_metrics LGTM but maybe you could make the ...
2 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: ...
2 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 ...
2 months, 2 weeks ago (2017-05-12 16:40:16 UTC) #37
Avi (ping after 24h)
lgtm for both the rename and tab_helpers
2 months, 2 weeks ago (2017-05-12 17:01:39 UTC) #38
Charlie Harrison
plm still lgtm
2 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
2 months, 1 week ago (2017-05-15 08:29:57 UTC) #44
commit-bot: I haz the power
2 months, 1 week 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973