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

Issue 875238 link

Starred by 1 user
Status: Fixed
Owner:
alexilin@chromium.org
Closed: Oct 10
Cc:
█ xunji...@chromium.org
csharrison@chromium.org
mmenke@chromium.org
pasko@chromium.org
ericorth@chromium.org
lizeb@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task

Blocking:
issue 699080



Sign in to add a comment

Remove deprecated chrome_browser_net::Predictor

Project Member Reported by alexilin@chromium.org, Aug 17 
The chrome_browser_net::Predictor is being replaced by predictors::LoadingPredictor which is enabled by default in M68 for all platforms but ChromeOS. It's default on ChromeOS since M69.

The net predictor is deprecated and can be removed starting M70. I see two options here:
* Mark the net predictor as deprecated in M70 to have a kill-switch for the LoadingPredictor a little longer and remove it in M71
* Remove the net predictor straight away in M70

I'd prefer the second option if there are no objections.

content/public/browser/resource_hints.h can also be removed since the LoadingPredictor will start using NetworkContext APIs directly and there is no other users of resource_hints.

Comment 1 by csharrison@chromium.org, Aug 17 

I don't have any objections to removing the predictor now. If we need to abort the experiment for the LoadingPredictor unexpectedly, we can always turn off predicting temporarily and it shouldn't break Chrome / websites.
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 5 

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

commit 273e20b678b8b0d9b8440893f8ec38a5fbe98aad
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Sep 05 19:47:57 2018

predictors: Remove chrome_browser_net::Predictor

This CL removes the deprecated chrome_browser_net::Predictor class. This code
isn't executed since M68 and was kept as a backup for the LoadingPredictor
experiment.

Bug:  875238 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I50e1fc48739bc73e23f396cfd60e61532812e16c
Reviewed-on: https://chromium-review.googlesource.com/1179884
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588974}
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/BUILD.gn
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/android/profiles/profile_manager_utils.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/android/warmup_manager.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/loader/predictor_resource_throttle.cc
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/loader/predictor_resource_throttle.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/net/OWNERS
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/net/predictor.cc
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/net/predictor.h
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/net/predictor_browsertest.cc
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/net/predictor_tab_helper.cc
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/net/predictor_tab_helper.h
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/net/predictor_unittest.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/net/referrer.cc
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/net/timed_cache.cc
[delete] https://crrev.com/39424745964259cf783eaced301de01ce1b76b2f/chrome/browser/net/timed_cache.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/net_benchmarking.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/net_benchmarking.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/predictors/loading_predictor_config.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/predictors/loading_predictor_config_unittest.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/profiles/off_the_record_profile_impl.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/profiles/off_the_record_profile_impl.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/profiles/profile.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/profiles/profile_impl.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/profiles/profile_impl_io_data.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/profiles/profile_impl_io_data.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/renderer_host/chrome_render_message_filter.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/renderer_host/chrome_render_message_filter.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/ui/app_list/test/fake_profile.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/ui/app_list/test/fake_profile.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/ui/omnibox/chrome_omnibox_client.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/ui/startup/startup_browser_creator.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/browser/ui/webui/about_ui.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/common/pref_names.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/common/pref_names.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/test/BUILD.gn
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/test/base/testing_profile.cc
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/chrome/test/base/testing_profile.h
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/testing/buildbot/filters/mac_window_server_killers.browser_tests.filter
[modify] https://crrev.com/273e20b678b8b0d9b8440893f8ec38a5fbe98aad/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Comment 3 by pasko@chromium.org, Sep 6 

\o/

Fixed?

Comment 4 by alexilin@chromium.org, Sep 6

Labels: -M-70 M-71
Status: Fixed (was: Started)
Fixed indeed.

Note that the patch was landed after the branch point so it goes to M71.

It's -12 kB of the binary size on Android, but I'm not sure it's worth merging into M70.

Comment 5 by alexilin@chromium.org, Sep 6

Status: Started (was: Fixed)
Actually, we still need to remove content/public/browser/resource_hints.h and there is one client on ChromeOS that uses it.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 5 

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

commit eb0de79d63b83a2a9c0c810e7bf651fced3c29ed
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Fri Oct 05 17:35:08 2018

chromeos: Migrate AuthPrewarmer off the deprecated preconnect API

content::PreconnectUrl() is about to be removed and AuthPrewarmer is the last
caller. This CL makes AuthPrewarmer to use NetworkContext::PreconnectSockets()
for prewarming a connection to the signin server.

Bug:  875238 
Change-Id: I0865d6d9566b13f71ee0ac59e8a583bc7c401099
Reviewed-on: https://chromium-review.googlesource.com/c/1261717
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597185}
[modify] https://crrev.com/eb0de79d63b83a2a9c0c810e7bf651fced3c29ed/chrome/browser/chromeos/login/auth/auth_prewarmer.cc
[modify] https://crrev.com/eb0de79d63b83a2a9c0c810e7bf651fced3c29ed/chrome/browser/chromeos/login/auth/auth_prewarmer.h
[modify] https://crrev.com/eb0de79d63b83a2a9c0c810e7bf651fced3c29ed/chrome/browser/chromeos/login/helper.cc
[modify] https://crrev.com/eb0de79d63b83a2a9c0c810e7bf651fced3c29ed/chrome/browser/chromeos/login/helper.h
[modify] https://crrev.com/eb0de79d63b83a2a9c0c810e7bf651fced3c29ed/chrome/browser/chromeos/login/ui/login_display_host_common.h
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 10 

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

commit 0d09089dd5f807460548d989a4cb552c2b12f848
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Oct 10 16:00:26 2018

Remove preconnect code from content/

All preconnect code was moved into chrome/browser/predictors and the code in
content/ doesn't have clients. So it can be safely removed.

Bug:  875238 
Change-Id: I24531ab832b2d4b49c5e8586f96ad66645501e5e
Reviewed-on: https://chromium-review.googlesource.com/c/1268343
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598349}
[modify] https://crrev.com/0d09089dd5f807460548d989a4cb552c2b12f848/net/dns/BUILD.gn
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 10 

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

commit f431bbab054d629edfd5b28782f81e5aa8f008f5
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Oct 10 16:01:49 2018

Remove features::kNetworkPrediction

Code for the feature has been already removed, so this base::Feature doesn't
control anything.

Bug:  875238 
Change-Id: I057bddd0895ef2e48ac98876ab61383631674536
Reviewed-on: https://chromium-review.googlesource.com/c/1271117
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598350}
[modify] https://crrev.com/f431bbab054d629edfd5b28782f81e5aa8f008f5/chrome/android/javatests/src/org/chromium/chrome/browser/feed/FeedNewTabPageTest.java
[modify] https://crrev.com/f431bbab054d629edfd5b28782f81e5aa8f008f5/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/NewTabPageTest.java
[modify] https://crrev.com/f431bbab054d629edfd5b28782f81e5aa8f008f5/chrome/android/javatests/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerViewTest.java
[modify] https://crrev.com/f431bbab054d629edfd5b28782f81e5aa8f008f5/chrome/android/javatests/src/org/chromium/chrome/browser/suggestions/TileGridLayoutTest.java
[modify] https://crrev.com/f431bbab054d629edfd5b28782f81e5aa8f008f5/chrome/browser/net/network_request_metrics_browsertest.cc
[modify] https://crrev.com/f431bbab054d629edfd5b28782f81e5aa8f008f5/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/f431bbab054d629edfd5b28782f81e5aa8f008f5/chrome/common/chrome_features.cc
[modify] https://crrev.com/f431bbab054d629edfd5b28782f81e5aa8f008f5/chrome/common/chrome_features.h

Comment 9 by alexilin@chromium.org, Oct 10

Cc: mmenke@chromium.org
Status: Fixed (was: Started)
It seems mmenke@ beat me in a race for removing resource_hints.h: https://crrev.com/c/1269557. Because of this I lost almost all changes in https://crrev.com/c/1268343 after rebase. ¯\_(ツ)_/¯
►

Sign in to add a comment