NQE: Plumb RTT and throughput estimates to the UI thread

NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=700215
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
TBR=bmcquade@chromium.org, petewil@chromium.org

Review-Url: https://codereview.chromium.org/2717153002
Cr-Commit-Position: refs/heads/master@{#456252}
Committed: https://chromium.googlesource.com/chromium/src/+/bddab11adb5f2abcf364ba53f8382df1d0b3098d

[tbansal1, 2017-02-26 08:27:51]

Description was changed from

==========
add rtt observer

BUG=
==========

to

==========
add rtt observer

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[tbansal1, 2017-02-26 08:28:15]

Patchset #1 (id:1) has been deleted

[tbansal1, 2017-02-26 08:28:20]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-26 08:28:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-26 09:31:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-26 09:31:29]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-03-04 05:53:14]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-04 05:53:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-04 06:47:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-04 06:47:57]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-03-04 06:52:09]

Patchset #1 (id:20001) has been deleted

[tbansal1, 2017-03-09 00:19:28]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 00:20:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2017-03-09 00:28:38]

Description was changed from

==========
add rtt observer

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[commit-bot: I haz the power, 2017-03-09 01:16:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-09 01:16:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tbansal1, 2017-03-09 07:35:31]

Patchset #1 (id:40001) has been deleted

[tbansal1, 2017-03-10 00:55:09]

Description was changed from

==========
NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=700215
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[tbansal1, 2017-03-10 00:55:24]

tbansal@chromium.org changed reviewers:
+ ryansturm@chromium.org

[tbansal1, 2017-03-10 00:55:25]

ryansturm: ptal. Thanks.

[RyanSturm, 2017-03-10 19:16:44]

https://codereview.chromium.org/2717153002/diff/60001/chrome/browser/net/nqe/...
File chrome/browser/net/nqe/ui_network_quality_estimator_service.h (right):

https://codereview.chromium.org/2717153002/diff/60001/chrome/browser/net/nqe/...
chrome/browser/net/nqe/ui_network_quality_estimator_service.h:54: void
AddRTTAndThroughputEstimatesObserver(
I would add these to NetworkQualityProvider interface.

[tbansal1, 2017-03-10 21:57:12]

Patchset #3 (id:100001) has been deleted

[tbansal1, 2017-03-10 21:57:50]

ptal. Thanks.

https://codereview.chromium.org/2717153002/diff/60001/chrome/browser/net/nqe/...
File chrome/browser/net/nqe/ui_network_quality_estimator_service.h (right):

https://codereview.chromium.org/2717153002/diff/60001/chrome/browser/net/nqe/...
chrome/browser/net/nqe/ui_network_quality_estimator_service.h:54: void
AddRTTAndThroughputEstimatesObserver(
On 2017/03/10 19:16:44, Ryan Sturm wrote:
> I would add these to NetworkQualityProvider interface.

Done.

[tbansal1, 2017-03-10 21:57:56]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 21:58:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 22:04:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-10 22:04:26]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[RyanSturm, 2017-03-10 22:10:47]

https://codereview.chromium.org/2717153002/diff/120001/chrome/browser/net/nqe...
File chrome/browser/net/nqe/ui_network_quality_estimator_service.h (right):

https://codereview.chromium.org/2717153002/diff/120001/chrome/browser/net/nqe...
chrome/browser/net/nqe/ui_network_quality_estimator_service.h:114:
base::TimeDelta http_rtt_;
Are you still adding Getters for this CL or landing a separate one?

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
File net/nqe/network_quality_estimator.h (right):

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:176: void
AddRTTAndThroughputEstimatesObserver(
virtual

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:181: void
RemoveRTTAndThroughputEstimatesObserver(
virtual

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:249: void
AddRTTAndThroughputEstimatesObserver(
Override

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:254: void
RemoveRTTAndThroughputEstimatesObserver(
override

[tbansal1, 2017-03-10 23:22:11]

ptal. Thanks.

https://codereview.chromium.org/2717153002/diff/120001/chrome/browser/net/nqe...
File chrome/browser/net/nqe/ui_network_quality_estimator_service.h (right):

https://codereview.chromium.org/2717153002/diff/120001/chrome/browser/net/nqe...
chrome/browser/net/nqe/ui_network_quality_estimator_service.h:114:
base::TimeDelta http_rtt_;
On 2017/03/10 22:10:47, Ryan Sturm wrote:
> Are you still adding Getters for this CL or landing a separate one?

Separate CL.

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
File net/nqe/network_quality_estimator.h (right):

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:249: void
AddRTTAndThroughputEstimatesObserver(
On 2017/03/10 22:10:47, Ryan Sturm wrote:
> Override

Done.

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:249: void
AddRTTAndThroughputEstimatesObserver(
On 2017/03/10 22:10:47, Ryan Sturm wrote:
> Override

This is not going to work now since NQE does not implement
NetworkQualityProvider. If it is necessary, I can do it in future. It will also
require moving NetworkQualityProvider to a different file.

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:254: void
RemoveRTTAndThroughputEstimatesObserver(
On 2017/03/10 22:10:47, Ryan Sturm wrote:
> override

Done.

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:254: void
RemoveRTTAndThroughputEstimatesObserver(
On 2017/03/10 22:10:47, Ryan Sturm wrote:
> override

same as above.

[tbansal1, 2017-03-10 23:22:16]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 23:22:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 23:29:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-10 23:29:50]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[tbansal1, 2017-03-10 23:37:16]

Patchset #5 (id:160001) has been deleted

[tbansal1, 2017-03-10 23:39:28]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 23:39:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 23:49:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-10 23:49:44]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tbansal1, 2017-03-10 23:59:54]

Patchset #5 (id:180001) has been deleted

[tbansal1, 2017-03-11 00:00:00]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-11 00:00:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-11 00:07:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-11 00:07:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[tbansal1, 2017-03-11 00:15:56]

The CQ bit was checked by tbansal@chromium.org to run a CQ dry run

[tbansal1, 2017-03-11 00:16:16]

Patchset #5 (id:200001) has been deleted

[commit-bot: I haz the power, 2017-03-11 00:16:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-11 01:17:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-11 01:17:43]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2017-03-11 01:36:13]

lgtm

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
File net/nqe/network_quality_estimator.h (right):

https://codereview.chromium.org/2717153002/diff/120001/net/nqe/network_qualit...
net/nqe/network_quality_estimator.h:249: void
AddRTTAndThroughputEstimatesObserver(
On 2017/03/10 23:22:11, tbansal1 wrote:
> On 2017/03/10 22:10:47, Ryan Sturm wrote:
> > Override
> 
> Done.

Yup. No problem. I think eventually we might want to move it over, but since
they are both net layer interfaces I have no problem.

[tbansal1, 2017-03-11 01:39:40]

Description was changed from

==========
NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=700215
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=700215
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
TBR=bmcquade@chromium.org, petewil@chromium.org
==========

[tbansal1, 2017-03-11 01:41:04]

TBR'ing 
bmcquade@chromium.org for ukm_page_load_metrics_observer_unittest.cc
and petewil@chromium.org for offline_pages.

The changes in both these components are mechanical, and only affects the test
files.

[tbansal1, 2017-03-11 01:41:14]

The CQ bit was checked by tbansal@chromium.org

[commit-bot: I haz the power, 2017-03-11 01:41:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Pete Williamson, 2017-03-11 01:49:20]

petewil@chromium.org changed reviewers:
+ petewil@chromium.org

[Pete Williamson, 2017-03-11 01:49:21]

offline pages changes lgtm

[commit-bot: I haz the power, 2017-03-11 01:50:52]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1489196474886440,
"parent_rev": "2c5480e85b7f645bd2c08cfe5b6c24ffa22c1206", "commit_rev":
"bddab11adb5f2abcf364ba53f8382df1d0b3098d"}

[commit-bot: I haz the power, 2017-03-11 01:51:36]

Description was changed from

==========
NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=700215
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
TBR=bmcquade@chromium.org, petewil@chromium.org
==========

to

==========
NQE: Plumb RTT and throughput estimates to the UI thread

This CL makes the RTT and throughput estimates available on the UI
thread. It also makes it possible to add observers on UI thread that
can listen to changes in the RTT/throughput estimations.

BUG=700215
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
TBR=bmcquade@chromium.org, petewil@chromium.org

Review-Url: https://codereview.chromium.org/2717153002
Cr-Commit-Position: refs/heads/master@{#456252}
Committed:
https://chromium.googlesource.com/chromium/src/+/bddab11adb5f2abcf364ba53f838...
==========

[commit-bot: I haz the power, 2017-03-11 01:51:37]

Committed patchset #5 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/bddab11adb5f2abcf364ba53f838...