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

Issue 809583 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 821027
issue 838763
issue 845648



Sign in to add a comment

The new Preconnect Predictor should use the network service.

Project Member Reported by jcivelli@chromium.org, Feb 6 2018

Issue description

The new preconnect predictor should use the network service.
For instance, it should not rely use the ResourceDispatcherHost (unsupported by the network service) but the WebContentsObserver interface and the NavigationHandle, which will have to be augmented with any capacity required and not yet provided.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 12 2018

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

commit 6ec63231b0cd9d6a38c82afc5317e9e947d4e1f7
Author: Chong Zhang <chongz@chromium.org>
Date: Mon Mar 12 17:51:54 2018

NetworkService: Update bug and comments for failing Predictor tests

TBR=kinuko@chromium.org

Bug:  809583 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I0050d598ad4531280085e397403e9597c54977f2
Reviewed-on: https://chromium-review.googlesource.com/959226
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542535}
[modify] https://crrev.com/6ec63231b0cd9d6a38c82afc5317e9e947d4e1f7/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Cc: xunji...@chromium.org
Blockedon: 821027
Components: Internals>Services>Network
Ok. thanks. Currently Network Service doesn't have an API for predictor to use. I am adding blocked-on bug to reflect that.
Status: Started (was: Untriaged)

Comment 5 by dxie@chromium.org, May 15 2018

Labels: -Pri-3 Proj-Servicification-Canary OS-All Pri-1

Comment 6 by dougt@chromium.org, May 15 2018

Cc: dougt@chromium.org

Comment 7 by dxie@chromium.org, May 18 2018

Labels: -OS-All OS-Windows OS-Linux OS-Mac OS-Chrome OS-Android
Blockedon: 845648
Blockedon: 850066
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 8 2018

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

commit e9fcce491e0fd41f86b1a27583af87bc91586684
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Fri Jun 08 11:33:17 2018

predictor: Switch to WebContentsObserver to observe resource loads.

This CL is needed to make the preconnect predictor work with the Network
Service. The CL removes the LoadingPredictorObserver class that has been
notified by the ChromeResourceDispatcherHostDelegate and introduces instead the
LoadingPredictorTabHelper which implements the WebContentsObserver to notify
the predictor about resource loads.

All uses of the net::URLRequest and predictors::URLRequestSummary was replaced
by using the predictors::NavigationID or the content::mojom::ResourceLoadInfo.

The WebContentsObserver::ResourceLoadComplete() should work with and
without the Network Service, so we don't have to keep the old path.

Bug:  809583 
Change-Id: I05c0aa97cfab1d773285633e6c5c3ef6a91367ca
Reviewed-on: https://chromium-review.googlesource.com/1039526
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565609}
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/BUILD.gn
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[delete] https://crrev.com/af150f277f03b99c7e1a49e6ea4807f1185fa74d/chrome/browser/net/loading_predictor_observer.cc
[delete] https://crrev.com/af150f277f03b99c7e1a49e6ea4807f1185fa74d/chrome/browser/net/loading_predictor_observer.h
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_data_collector.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_data_collector.h
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_data_collector_unittest.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_predictor.h
[add] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_predictor_tab_helper.cc
[add] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_predictor_tab_helper.h
[add] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_predictor_tab_helper_unittest.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_predictor_unittest.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_stats_collector_unittest.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_test_util.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/loading_test_util.h
[delete] https://crrev.com/af150f277f03b99c7e1a49e6ea4807f1185fa74d/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc
[delete] https://crrev.com/af150f277f03b99c7e1a49e6ea4807f1185fa74d/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.h
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/test/BUILD.gn
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/e9fcce491e0fd41f86b1a27583af87bc91586684/content/browser/loader/resource_hints_impl.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 11 2018

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

commit 2f220c33f92dce7d83ec65d516a06c3e37144bd3
Author: Hayato Ito <hayato@chromium.org>
Date: Mon Jun 11 08:03:12 2018

Revert "predictor: Switch to WebContentsObserver to observe resource loads."

This reverts commit e9fcce491e0fd41f86b1a27583af87bc91586684.

Reason for revert:
Findit identified the culprit r565609 with confidence 70.0%.
See https://bugs.chromium.org/p/chromium/issues/detail?id=851232


Original change's description:
> predictor: Switch to WebContentsObserver to observe resource loads.
> 
> This CL is needed to make the preconnect predictor work with the Network
> Service. The CL removes the LoadingPredictorObserver class that has been
> notified by the ChromeResourceDispatcherHostDelegate and introduces instead the
> LoadingPredictorTabHelper which implements the WebContentsObserver to notify
> the predictor about resource loads.
> 
> All uses of the net::URLRequest and predictors::URLRequestSummary was replaced
> by using the predictors::NavigationID or the content::mojom::ResourceLoadInfo.
> 
> The WebContentsObserver::ResourceLoadComplete() should work with and
> without the Network Service, so we don't have to keep the old path.
> 
> Bug:  809583 
> Change-Id: I05c0aa97cfab1d773285633e6c5c3ef6a91367ca
> Reviewed-on: https://chromium-review.googlesource.com/1039526
> Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Reviewed-by: Michael Wasserman <msw@chromium.org>
> Reviewed-by: Benoit L <lizeb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#565609}

TBR=msw@chromium.org,thestig@chromium.org,mmenke@chromium.org,lizeb@chromium.org,alexilin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  809583 
Change-Id: If1bf3e4b6e98fbd716b9b994b0ee0c69cafc7036
Reviewed-on: https://chromium-review.googlesource.com/1094955
Reviewed-by: Hayato Ito <hayato@chromium.org>
Commit-Queue: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565945}
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/BUILD.gn
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[add] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/net/loading_predictor_observer.cc
[add] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/net/loading_predictor_observer.h
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_data_collector.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_data_collector.h
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_data_collector_unittest.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_predictor.h
[delete] https://crrev.com/b8771c718b64645ac4e32a22d696c826d7e921c6/chrome/browser/predictors/loading_predictor_tab_helper.cc
[delete] https://crrev.com/b8771c718b64645ac4e32a22d696c826d7e921c6/chrome/browser/predictors/loading_predictor_tab_helper.h
[delete] https://crrev.com/b8771c718b64645ac4e32a22d696c826d7e921c6/chrome/browser/predictors/loading_predictor_tab_helper_unittest.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_predictor_unittest.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_stats_collector_unittest.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_test_util.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/loading_test_util.h
[add] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc
[add] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.h
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/test/BUILD.gn
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/2f220c33f92dce7d83ec65d516a06c3e37144bd3/content/browser/loader/resource_hints_impl.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 13 2018

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

commit 068c897faa72a8a8e814783f04f2d7a042e677c1
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Wed Jun 13 09:37:09 2018

Reland "predictor: Switch to WebContentsObserver to observe resource loads."

This reverts commit 2f220c33f92dce7d83ec65d516a06c3e37144bd3.

Reason for revert: TBD

Original change's description:
> Revert "predictor: Switch to WebContentsObserver to observe resource loads."
> 
> This reverts commit e9fcce491e0fd41f86b1a27583af87bc91586684.
> 
> Reason for revert:
> Findit identified the culprit r565609 with confidence 70.0%.
> See https://bugs.chromium.org/p/chromium/issues/detail?id=851232
> 
> 
> Original change's description:
> > predictor: Switch to WebContentsObserver to observe resource loads.
> > 
> > This CL is needed to make the preconnect predictor work with the Network
> > Service. The CL removes the LoadingPredictorObserver class that has been
> > notified by the ChromeResourceDispatcherHostDelegate and introduces instead the
> > LoadingPredictorTabHelper which implements the WebContentsObserver to notify
> > the predictor about resource loads.
> > 
> > All uses of the net::URLRequest and predictors::URLRequestSummary was replaced
> > by using the predictors::NavigationID or the content::mojom::ResourceLoadInfo.
> > 
> > The WebContentsObserver::ResourceLoadComplete() should work with and
> > without the Network Service, so we don't have to keep the old path.
> > 
> > Bug:  809583 
> > Change-Id: I05c0aa97cfab1d773285633e6c5c3ef6a91367ca
> > Reviewed-on: https://chromium-review.googlesource.com/1039526
> > Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
> > Reviewed-by: Matt Menke <mmenke@chromium.org>
> > Reviewed-by: Lei Zhang <thestig@chromium.org>
> > Reviewed-by: Michael Wasserman <msw@chromium.org>
> > Reviewed-by: Benoit L <lizeb@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#565609}
> 
> TBR=msw@chromium.org,thestig@chromium.org,mmenke@chromium.org,lizeb@chromium.org,alexilin@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  809583 
> Change-Id: If1bf3e4b6e98fbd716b9b994b0ee0c69cafc7036
> Reviewed-on: https://chromium-review.googlesource.com/1094955
> Reviewed-by: Hayato Ito <hayato@chromium.org>
> Commit-Queue: Hayato Ito <hayato@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#565945}

TBR=msw@chromium.org,thestig@chromium.org,hayato@chromium.org,mmenke@chromium.org,lizeb@chromium.org,alexilin@chromium.org

Change-Id: I10444584eb2a216db16066cd5909e51ed4e27fda
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  809583 
Reviewed-on: https://chromium-review.googlesource.com/1095874
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566776}
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/BUILD.gn
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[delete] https://crrev.com/9a94c1e20ca591be29c325410f3efb13ee62c6a5/chrome/browser/net/loading_predictor_observer.cc
[delete] https://crrev.com/9a94c1e20ca591be29c325410f3efb13ee62c6a5/chrome/browser/net/loading_predictor_observer.h
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_data_collector.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_data_collector.h
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_data_collector_unittest.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_predictor.h
[add] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_predictor_tab_helper.cc
[add] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_predictor_tab_helper.h
[add] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_predictor_tab_helper_unittest.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_predictor_unittest.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_stats_collector_unittest.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_test_util.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/loading_test_util.h
[delete] https://crrev.com/9a94c1e20ca591be29c325410f3efb13ee62c6a5/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.cc
[delete] https://crrev.com/9a94c1e20ca591be29c325410f3efb13ee62c6a5/chrome/browser/predictors/resource_prefetch_predictor_tab_helper.h
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/test/BUILD.gn
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/chrome/test/base/in_process_browser_test.cc
[modify] https://crrev.com/068c897faa72a8a8e814783f04f2d7a042e677c1/content/browser/loader/resource_hints_impl.cc

@alex, are you still blocked on this? Can you please provide an update? thanks!
I'm still blocked on  https://crbug.com/850066 . I'd be much easier for me to switch to the preconnect and the preresolve Network Service API at the same time.
alexilin:  The HostResolver API just landed, though it has no consumers yet.  See NetworkContext::ResolveHost.  There's another CL out which allows creating host resolver objects which can be used off the main thread, if you need to do stuff on the IO thread still (Though without any network stuff on the IO thread, that's probably not necessary)
Blockedon: 838763
Blockedon: -850066
Labels: -Proj-Servicification-Canary Proj-Servicification
alex, what's the plan for this bug? I assume we can also remove this from canary blocking.
Labels: Proj-Servicification-Canary
I have a patch that hopefully will be landed soon: https://crrev.com/c/1165550

This blocks canary until then.
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 17

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

commit 5b417db655388d726c0724f5a12425a5ddd68904
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Fri Aug 17 18:34:00 2018

predictors: Use NetworkContext's APIs to preconnect sockets and resolve hosts.

This CL makes the predictor compatible with the network service. All used APIs
correctly work with the disabled network service as well, so there is no need
to keep both code paths.

Since all requests to the NetworkContext should be made on the UI thread, this
CL modifies predictors::PreconnectManager class so it lives entirely on the UI
thread instead of IO thread.

We cannot issue synchronous requests to the proxy resolution service with the
network service. This CL removes skipping preresolving in cases when a proxy is
enabled, since it relies on synchronous call to the proxy resolution service.
Follow-up CL that sends an asynchronous request to the proxy resolution service
in parallel with a resolve host request will be submitted shortly.

Bug:  809583 
Change-Id: I1afe9895fc1fba163d8519df465f98a4c6a5e552
Reviewed-on: https://chromium-review.googlesource.com/1165550
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584135}
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/BUILD.gn
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/predictors/loading_predictor.h
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/predictors/loading_predictor_unittest.cc
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/predictors/preconnect_manager.cc
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/predictors/preconnect_manager.h
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/predictors/preconnect_manager_unittest.cc
[add] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/predictors/resolve_host_client_impl.cc
[add] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/predictors/resolve_host_client_impl.h
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/renderer_host/chrome_render_message_filter.cc
[modify] https://crrev.com/5b417db655388d726c0724f5a12425a5ddd68904/chrome/browser/renderer_host/chrome_render_message_filter.h

Status: Fixed (was: Started)
Project Member

Comment 22 by bugdroid1@chromium.org, Aug 21

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

commit bf27433365da7384944e85611f8ea77a6f7ff3eb
Author: Alexandr Ilin <alexilin@chromium.org>
Date: Tue Aug 21 08:34:33 2018

predictors: Set extra flags in ResolveHostClientImpl

The predictor used to set the is_speculative flag and lower the priority of
resolve host request. This was missed in the CL https://crrev.com/c/1165550.
This CL restores this parameters for the predictor.

Bug:  809583 
Change-Id: Iebfd2ca28bed1a854b6e3b929441b79cfc178f80
Reviewed-on: https://chromium-review.googlesource.com/1180965
Reviewed-by: Benoit L <lizeb@chromium.org>
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584688}
[modify] https://crrev.com/bf27433365da7384944e85611f8ea77a6f7ff3eb/chrome/browser/predictors/resolve_host_client_impl.cc

Thanks Alexandr!
There are still a bunch of PredictorBrowserTest tests listed in testing/buildbot/filters/mojo.fyi.network_browser_tests.filter. Are these for the old implementation? Can we just early out in those tests if network service is enabled?
These tests were for the old implementation. I'm removing them here https://crrev.com/c/1179884.
Thanks

Sign in to add a comment