New issue
Advanced search Search tips

Issue 819244 link

Starred by 4 users

Issue metadata

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

Blocking:
issue 598073



Sign in to add a comment

NetworkService: Need a way to observe the NetworkQualityEstimator

Project Member Reported by mmenke@chromium.org, Mar 6 2018

Issue description

For the network service project, the NetworkQualityEstimator needs to run out of process.  We'll need to hook up a way to observe its state.
 

Comment 1 by mmenke@chromium.org, Mar 15 2018

Summary: NetworkService: Need a way to observe the NetworkQualityEstimator (was: NetworkService: Need a way to observer the NetworkQualityEstimator)
Cc: tbansal@chromium.org
+ tbansal@

Comment 3 by bengr@google.com, Mar 21 2018

Status: Assigned (was: Untriaged)

Comment 4 by bengr@google.com, Mar 21 2018

Cc: -tbansal@chromium.org
Owner: tbansal@chromium.org
I think this can follow similar code path as NetworkConnectionTracker.
Status: Started (was: Assigned)
Cc: mmenke@chromium.org
Labels: Proj-Servicification-Canary OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
+mmenke@

Matt, one question came up. Where should the client-side interface of NQE live (the interface that observes state changes of NQE's service-side interface)? 

Does SystemNetworkContextManager sound right? IOThread globals owns the current in-process NQE (https://cs.chromium.org/chromium/src/chrome/browser/io_thread.cc?rcl=a60c8b678f340df16da050c0f2bc126d3e1255fb&l=780). 

Comment 8 by mmenke@chromium.org, May 15 2018

It depends...Are we going to want to expose this to content embedders, or
just to Chrome code?  If it's just for Chrome, adding it to
SystemNetworkContextManager makes sense.  If we care about other content
embedders, we may want to make it owned and accessible through some content
interface, though I'm not at all confident of that.

Or if we only have one or two observers, and don't expect more, we could
even just have them directly listen to the NetworkService.
mmenke and xunjieli: Thanks for the help.

I guess I am not sure how the NQE JavaScript API would work with servicification. The same question applies for NCN NetInfo API which is currently plumbed via //content: https://cs.chromium.org/chromium/src/content/browser/net/browser_online_state_observer.cc?type=cs&q=f:browser.*online.*state&sq=package:chromium&g=0&l=15

If I understand correctly, eventually it has to be updated...right?
Oh, this is passed to the renderer?  I hadn't realized that.  We could proxy information through the BrowserOnlineStateObserver, like we do now, and have two process hopes, but that seems inefficient, so what we really should do is have renderer get a connection on creation.

The general way to get a connection to a global object is through a connector, but I don't know how to set them up, nor if a connector can give a connection to a pre-existing object (Without using globals).
Yes, currently NQE is passed to renderers similar to NCN. Here is the implementation (which is very similar to BrowserOnlineStateObserver): https://cs.chromium.org/chromium/src/content/browser/net/network_quality_observer_impl.cc

However, my understanding is that current implementation of BrowserOnlineStateObserver is not compatible with network servicification since BrowserOnlineStateObserver accesses global NCN directly from browser process. 

So, it seems that client-side interfaces of NCN (and NQE) need to live in //content. Is my understanding correct? If so, is there a good place for them in //content?
Yes, they/d need to be in content/common, since they'll be needed by both the renderer and browser process.  More important, though, is how we plumb the mojo pipes through them, and that I'm not sure of.
Thanks, in that case, I will just follow the NCN servicification path. Once BrowserOnlineStateObserver moves to content/common, I can work on moving the NQE JavaScript plumbing as well.

Comment 14 by dxie@chromium.org, May 29 2018

tbansal@, can you update the latest on this bug?
Re #14: I have started the CL on it, but it got deprioritized. I hope to get back to it next week or so.

Comment 16 by dxie@chromium.org, Jun 7 2018

hi tbansal@, are you looking into it?

Comment 17 by dxie@chromium.org, Jun 7 2018

Labels: -Pri-3 Pri-1
marking it as P1 as we need this for network service canary.
@tbansal: Any updates here?
Working on it. I will send out the CL in next couple of days.
Project Member

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

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

commit afbf221e8d9b975353c40a0072409c582297e3dc
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Jun 20 18:05:46 2018

Add mechanism to force ECT computation value in NQE

Add an API to Network Quality Estimator that can be called
to force the computed Effective Connection Type value.
This will be used in a subsequent CL.

Change-Id: I96d3dea84af0c87ba841cf1115269ce7709a6e0c
Bug:  819244 
Reviewed-on: https://chromium-review.googlesource.com/1107489
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568922}
[modify] https://crrev.com/afbf221e8d9b975353c40a0072409c582297e3dc/net/nqe/network_quality_estimator.cc
[modify] https://crrev.com/afbf221e8d9b975353c40a0072409c582297e3dc/net/nqe/network_quality_estimator.h
[modify] https://crrev.com/afbf221e8d9b975353c40a0072409c582297e3dc/net/nqe/network_quality_estimator_params.cc
[modify] https://crrev.com/afbf221e8d9b975353c40a0072409c582297e3dc/net/nqe/network_quality_estimator_params.h
[modify] https://crrev.com/afbf221e8d9b975353c40a0072409c582297e3dc/net/nqe/network_quality_estimator_unittest.cc

Comment 21 by dxie@chromium.org, Jun 27 2018

how are we looking here?
Project Member

Comment 23 by bugdroid1@chromium.org, Jun 27 2018

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

commit cd6a6cb18074df1f1baca96ba4ddc1469e3137e6
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Jun 27 22:03:22 2018

Add a mojom call to observe network quality change events

Network Quality Estimator (NQE) is a part of Chromium network
stack, and posts notifications about network quality
change events to its registered observers. The current
architecture assumes that network service runs in the
same process as browser.

This CL adds a mojom call for observers to request network
quality change notifications over a mojom interface.
This allows us to provide notifications to the browser
process from the network process.

NetworkQualityTracker class lives in the browser process,
receives notifications from the network process, and in turn notifies
its own registered network quality observers.

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I3a01b40b2b963f924c4eacebc353317c140d5ebb
Bug:  819244 
Reviewed-on: https://chromium-review.googlesource.com/1058528
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570915}
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/chrome/browser/browser_process.h
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/chrome/browser/browser_process_impl.h
[add] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/chrome/browser/net/network_quality_tracker_browsertest.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/chrome/test/BUILD.gn
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/chrome/test/base/testing_browser_process.h
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/content/public/browser/network_connection_tracker_unittest.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/content/public/test/DEPS
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/content/public/test/network_service_test_helper.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/net/nqe/network_quality_estimator_params.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/BUILD.gn
[add] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/network_quality_estimator_manager.cc
[add] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/network_quality_estimator_manager.h
[add] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/network_quality_estimator_manager_unittest.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/network_service.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/network_service.h
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/public/cpp/BUILD.gn
[add] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/public/cpp/network_quality_tracker.cc
[add] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/public/cpp/network_quality_tracker.h
[add] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/public/cpp/network_quality_tracker_unittest.cc
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/public/mojom/BUILD.gn
[add] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/public/mojom/network_quality_estimator_manager.mojom
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/public/mojom/network_service.mojom
[modify] https://crrev.com/cd6a6cb18074df1f1baca96ba4ddc1469e3137e6/services/network/public/mojom/network_service_test.mojom

Project Member

Comment 24 by bugdroid1@chromium.org, Jun 28 2018

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

commit 5b60c588b0271c2b064256cd7f8ecdb238144df3
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Jun 28 05:11:27 2018

Fix the network quality browser test

Initialize the value of a variable to avoid triggering
use of uninitialized variable error

Bug:  819244 
Change-Id: Ida4ccc55c872ebaeb54d41c35f237e6930c8aa7b
TBR: xunjieli@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1117932
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571032}
[modify] https://crrev.com/5b60c588b0271c2b064256cd7f8ecdb238144df3/chrome/browser/net/network_quality_tracker_browsertest.cc

tbansal, is this fixed or do you have more CLs coming?
Currently, the mojo call only sends ECT updates to the browser process. I need to add RTT and downlink updates too.
Project Member

Comment 27 by bugdroid1@chromium.org, Jul 11

Project Member

Comment 28 by bugdroid1@chromium.org, Jul 12

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

commit 88c8cc52351f93d11e0bfac3b75ded5b420b131b
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Jul 12 19:56:46 2018

Network quality servicification of RTT and downlink estimates

Send the RTT and downlink estimates over the mojo channel
to the browser process. The RTT and downlink estimates are
sent over an existing mojo message which carrier the current
effective connection type estimate.

The mojo message is sent only if there is a significant change
in the values of either the RTT or the downlink estimate.

In the next CL, I will move the network quality consumers
in the browser process over to NetworkQualityTracker.

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I01bbc7af2e7e3cf41f69e7ac19f8edd9ec59de90
Bug:  819244 
Reviewed-on: https://chromium-review.googlesource.com/1130233
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574685}
[modify] https://crrev.com/88c8cc52351f93d11e0bfac3b75ded5b420b131b/chrome/browser/net/network_quality_tracker_browsertest.cc
[modify] https://crrev.com/88c8cc52351f93d11e0bfac3b75ded5b420b131b/services/network/network_quality_estimator_manager.cc
[modify] https://crrev.com/88c8cc52351f93d11e0bfac3b75ded5b420b131b/services/network/network_quality_estimator_manager.h
[modify] https://crrev.com/88c8cc52351f93d11e0bfac3b75ded5b420b131b/services/network/network_quality_estimator_manager_unittest.cc
[modify] https://crrev.com/88c8cc52351f93d11e0bfac3b75ded5b420b131b/services/network/public/cpp/network_quality_tracker.cc
[modify] https://crrev.com/88c8cc52351f93d11e0bfac3b75ded5b420b131b/services/network/public/cpp/network_quality_tracker.h
[modify] https://crrev.com/88c8cc52351f93d11e0bfac3b75ded5b420b131b/services/network/public/cpp/network_quality_tracker_unittest.cc
[modify] https://crrev.com/88c8cc52351f93d11e0bfac3b75ded5b420b131b/services/network/public/mojom/network_quality_estimator_manager.mojom

tbansal@: Looks like RTT and downlink estimates are in. Any more work on this bug? 
I need to move the network quality observers to use the new tracker class, and then reenable some of the tests that are currently disabled when NetworkServicification is enabled.
Project Member

Comment 31 by bugdroid1@chromium.org, Jul 17

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

commit 7acc205a4672b70da9589501e86d41ad5fa63817
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Jul 17 14:32:12 2018

Network quality servicification of transport RTT

Send the transport RTT over the mojo channel to the
browser process. The transport RTT is sent over an
existing mojo message which carrier the other network
quality estimates.

The mojo message is sent only if there is a significant change
in the values of the estimates.

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ief33a27239e88b368cf403b4578bfde329bbabbf
Bug:  819244 
Reviewed-on: https://chromium-review.googlesource.com/1139084
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575636}
[modify] https://crrev.com/7acc205a4672b70da9589501e86d41ad5fa63817/chrome/browser/net/network_quality_tracker_browsertest.cc
[modify] https://crrev.com/7acc205a4672b70da9589501e86d41ad5fa63817/services/network/network_quality_estimator_manager.cc
[modify] https://crrev.com/7acc205a4672b70da9589501e86d41ad5fa63817/services/network/network_quality_estimator_manager.h
[modify] https://crrev.com/7acc205a4672b70da9589501e86d41ad5fa63817/services/network/network_quality_estimator_manager_unittest.cc
[modify] https://crrev.com/7acc205a4672b70da9589501e86d41ad5fa63817/services/network/public/cpp/network_quality_tracker.cc
[modify] https://crrev.com/7acc205a4672b70da9589501e86d41ad5fa63817/services/network/public/cpp/network_quality_tracker.h
[modify] https://crrev.com/7acc205a4672b70da9589501e86d41ad5fa63817/services/network/public/cpp/network_quality_tracker_unittest.cc
[modify] https://crrev.com/7acc205a4672b70da9589501e86d41ad5fa63817/services/network/public/mojom/network_quality_estimator_manager.mojom

Why is this a P1?
Bugs that block running network service experiments on Canary on Windows are all marked P1.
hi, tbansal@, are you still working on this bug? We need this bug fixed in order to do an experiment with Network S13N. Let me know if we need to reprioritize this bug.
Project Member

Comment 35 by bugdroid1@chromium.org, Aug 2

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

commit fd2e773fa8bafe858d992e973778cad05fd95d0a
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Aug 02 23:07:26 2018

Add mechanism to add RTT/throughput estimates observer to NQT

Add mechanism to add HTTP RTT, transport RTT, downlink throughout
estimate change observer to NetworkQualityTracker.

In the next CL, an IO object owned by UINetworkQualityEstimatorService
would subscribe to these changes, and then pass them on to the UI
thread.

Bug:  819244 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ieeb786c9376af7afe9f2e8bff85468067c4c4ecf
Reviewed-on: https://chromium-review.googlesource.com/1144470
Reviewed-by: Nick Carter <nick@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580367}
[modify] https://crrev.com/fd2e773fa8bafe858d992e973778cad05fd95d0a/content/public/browser/network_connection_tracker_unittest.cc
[modify] https://crrev.com/fd2e773fa8bafe858d992e973778cad05fd95d0a/services/network/public/cpp/network_quality_tracker.cc
[modify] https://crrev.com/fd2e773fa8bafe858d992e973778cad05fd95d0a/services/network/public/cpp/network_quality_tracker.h
[modify] https://crrev.com/fd2e773fa8bafe858d992e973778cad05fd95d0a/services/network/public/cpp/network_quality_tracker_unittest.cc

can this be marked as Fixed?
It's not fixed yet. I need to move the observers to Network Quality Tracker. Probably 1 week more.
Project Member

Comment 38 by bugdroid1@chromium.org, Aug 10

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

commit 5ac5335452b7f3f5f88837c9dfabda46d51e4795
Author: Tarun Bansal <tbansal@chromium.org>
Date: Fri Aug 10 19:45:52 2018

NQE servicification of client hints

Use network quality tracker instead of network quality estimator (NQE)
to obtain network quality when attaching network quality
client hints for main frame resources.

Network quality tracker can provide network service when network
service is enabled as well as when network service is disabled.

Change-Id: Iafa641ecb487bb00aecc0d3f1a18cfcdabba3de8
Bug:  819244 
Reviewed-on: https://chromium-review.googlesource.com/1170568
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582296}
[modify] https://crrev.com/5ac5335452b7f3f5f88837c9dfabda46d51e4795/chrome/browser/client_hints/client_hints.cc
[modify] https://crrev.com/5ac5335452b7f3f5f88837c9dfabda46d51e4795/chrome/browser/client_hints/client_hints_browsertest.cc

Project Member

Comment 39 by bugdroid1@chromium.org, Aug 13

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

commit 5e9e06bd8360724bf82f3ada2e53da76f921a009
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Aug 13 21:35:45 2018

NQE servicification of data saver proxy

Use network quality tracker instead of network quality estimator (NQE)
to obtain network quality in data reduction proxy component.

Network quality tracker can provide network service when network
service is enabled as well as when network service is disabled.

data_reduction_proxy_service.h receives network quality
estimates on UI thread from network quality tracker.
On receiving estimates, it posts them to d_r_p_io_data
which lives on IO thread, and consumes the estimates.

Bug:  819244 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I8a89fae921c15d5e006c9d51cb19cb6116688fb1
Reviewed-on: https://chromium-review.googlesource.com/1171592
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582711}
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/chrome/browser/data_saver/data_saver_browsertest.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/chrome/browser/net/network_quality_tracker_browsertest.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/chrome/test/base/testing_browser_process.h
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/BUILD.gn
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/DEPS
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/services/network/BUILD.gn
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/services/network/public/cpp/network_quality_tracker.cc
[modify] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/services/network/public/cpp/network_quality_tracker.h
[add] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/services/network/test/test_network_quality_tracker.cc
[add] https://crrev.com/5e9e06bd8360724bf82f3ada2e53da76f921a009/services/network/test/test_network_quality_tracker.h

Project Member

Comment 40 by bugdroid1@chromium.org, Aug 14

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

commit aaca598dcdc207d2f586e866f3214b53bc01ac28
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue Aug 14 00:03:11 2018

Revert "NQE servicification of data saver proxy"

This reverts commit 5e9e06bd8360724bf82f3ada2e53da76f921a009.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 582711 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzVlOWUwNmJkODM2MDcyNGJmODJmM2FkYTJlNTNkYTc2ZjkyMWEwMDkM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29%2832%29/52049

Sample Failed Step: network_service_browser_tests

Original change's description:
> NQE servicification of data saver proxy
> 
> Use network quality tracker instead of network quality estimator (NQE)
> to obtain network quality in data reduction proxy component.
> 
> Network quality tracker can provide network service when network
> service is enabled as well as when network service is disabled.
> 
> data_reduction_proxy_service.h receives network quality
> estimates on UI thread from network quality tracker.
> On receiving estimates, it posts them to d_r_p_io_data
> which lives on IO thread, and consumes the estimates.
> 
> Bug:  819244 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I8a89fae921c15d5e006c9d51cb19cb6116688fb1
> Reviewed-on: https://chromium-review.googlesource.com/1171592
> Reviewed-by: Helen Li <xunjieli@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582711}

Change-Id: Ida144bf271de7c3cf7a9794679c71ffcd9a55d07
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  819244 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1173573
Cr-Commit-Position: refs/heads/master@{#582757}
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/chrome/browser/data_saver/data_saver_browsertest.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/chrome/browser/net/network_quality_tracker_browsertest.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/chrome/test/base/testing_browser_process.h
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/BUILD.gn
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/DEPS
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/services/network/BUILD.gn
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/services/network/public/cpp/network_quality_tracker.cc
[modify] https://crrev.com/aaca598dcdc207d2f586e866f3214b53bc01ac28/services/network/public/cpp/network_quality_tracker.h
[delete] https://crrev.com/84abf8e6330b94853e5c3aa70ba235dafffb45ec/services/network/test/test_network_quality_tracker.cc
[delete] https://crrev.com/84abf8e6330b94853e5c3aa70ba235dafffb45ec/services/network/test/test_network_quality_tracker.h

Cc: cduvall@chromium.org
Project Member

Comment 42 by bugdroid1@chromium.org, Aug 14

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

commit 8649d8eef08028ba6b4cb2cc864a1073cf5ce731
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Aug 14 22:51:42 2018

Reland of NQE servicification of data saver proxy

Use network quality tracker instead of network quality estimator (NQE)
to obtain network quality in data reduction proxy component.

Network quality tracker can provide network service when network
service is enabled as well as when network service is disabled.

data_reduction_proxy_service.h receives network quality
estimates on UI thread from network quality tracker.
On receiving estimates, it posts them to d_r_p_io_data
which lives on IO thread, and consumes the estimates.

PS#2 is the original CL that got reverted.

Bug:  819244 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Id9f28abaea42a7eb55dd3647c88d13a427a4e5ae
TBR: xunjieli@chromium.org, sky@chromium.org, ryansturm@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1174698
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583065}
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/chrome/browser/data_saver/data_saver_browsertest.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/chrome/browser/net/network_quality_tracker_browsertest.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/chrome/test/base/testing_browser_process.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/chrome/test/base/testing_browser_process.h
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/BUILD.gn
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/DEPS
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/services/network/BUILD.gn
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/services/network/public/cpp/network_quality_tracker.cc
[modify] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/services/network/public/cpp/network_quality_tracker.h
[add] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/services/network/test/test_network_quality_tracker.cc
[add] https://crrev.com/8649d8eef08028ba6b4cb2cc864a1073cf5ce731/services/network/test/test_network_quality_tracker.h

Project Member

Comment 43 by bugdroid1@chromium.org, Aug 15

Project Member

Comment 44 by bugdroid1@chromium.org, Aug 16

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

commit 4d4e8ea6bfef9430c7243c4b839e9d02f82972bb
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Aug 16 21:12:34 2018

NQE servicification of offline pages

Use network quality tracker instead of network quality estimator (NQE)
to obtain network quality in offline pages.

Network quality tracker can provide network service when network
service is enabled as well as when network service is disabled.

Bug:  819244 
Change-Id: I0b2bad429c1e7745759a1172fb1fbaa843dd8b8b
Reviewed-on: https://chromium-review.googlesource.com/1171863
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583816}
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/chrome/browser/offline_pages/android/evaluation/offline_page_evaluation_bridge.cc
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/chrome/browser/offline_pages/android/request_coordinator_factory.cc
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/chrome/browser/offline_pages/offline_page_utils_unittest.cc
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/chrome/browser/offline_pages/test_request_coordinator_builder.cc
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/components/offline_pages/core/background/BUILD.gn
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/components/offline_pages/core/background/DEPS
[delete] https://crrev.com/8fd04b5fbba25e412874ff15c659c25fb9dcc12c/components/offline_pages/core/background/network_quality_provider_stub.cc
[delete] https://crrev.com/8fd04b5fbba25e412874ff15c659c25fb9dcc12c/components/offline_pages/core/background/network_quality_provider_stub.h
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/components/offline_pages/core/background/request_coordinator.cc
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/components/offline_pages/core/background/request_coordinator.h
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/components/offline_pages/core/background/request_coordinator_stub_taco.cc
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/components/offline_pages/core/background/request_coordinator_stub_taco.h
[modify] https://crrev.com/4d4e8ea6bfef9430c7243c4b839e9d02f82972bb/components/offline_pages/core/background/request_coordinator_unittest.cc

Project Member

Comment 45 by bugdroid1@chromium.org, Aug 16

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

commit dbb1d100523dc890d512954dec03e366613641b4
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Aug 16 23:12:29 2018

NQE servicification of UKM page load metrics

Use network quality tracker instead of network quality estimator (NQE)
in UKM page load metrics.

Network quality tracker can provide network service when network
service is enabled as well as when network service is disabled.

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Icffe39d2ae493934456ff911290bc50d53efab49
Bug:  819244 
Reviewed-on: https://chromium-review.googlesource.com/1177199
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583873}
[modify] https://crrev.com/dbb1d100523dc890d512954dec03e366613641b4/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.cc
[modify] https://crrev.com/dbb1d100523dc890d512954dec03e366613641b4/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer.h
[modify] https://crrev.com/dbb1d100523dc890d512954dec03e366613641b4/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/dbb1d100523dc890d512954dec03e366613641b4/chrome/browser/page_load_metrics/page_load_metrics_initialize.cc
[modify] https://crrev.com/dbb1d100523dc890d512954dec03e366613641b4/services/network/public/cpp/network_quality_tracker.h

hey, just wanted to check what's remaining. We want to try to close all Canary blockers by 8/24.
Status: Fixed (was: Started)
I think this can be closed now.  There is some cleanup work left, but I will continue that in a separate bug.
Thanks!

Sign in to add a comment