Record difference between the maximum and minimum wireless signal strength

Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

Also, cleanup by moving the methods in the anonymous namespace to
an anonymous namespace in namespace net.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=513681
Committed: https://crrev.com/e85ca85b510f8289da6337eedaee2019dbe3c9a7
Cr-Commit-Position: refs/heads/master@{#439238}

[tbansal1, 2016-12-13 00:20:01]

Patchset #1 (id:1) has been deleted

[tbansal1, 2016-12-13 00:21:11]

Description was changed from

==========
Record difference between maximum and minimum signal strength

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

to

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

BUG=513681
==========

[tbansal1, 2016-12-13 00:21:27]

Description was changed from

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

BUG=513681
==========

to

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=513681
==========

[tbansal1, 2016-12-13 00:21:33]

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

[tbansal1, 2016-12-13 00:21:56]

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

[tbansal1, 2016-12-13 00:21:57]

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

[tbansal1, 2016-12-13 00:21:57]

ryansturm: ptal. Thanks.

[tbansal1, 2016-12-13 00:21:57]

ryansturm: ptal. Thanks.

[commit-bot: I haz the power, 2016-12-13 00:22:47]

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, 2016-12-13 03:28:26]

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

[commit-bot: I haz the power, 2016-12-13 03:28:27]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2016-12-13 18:02:20]

lgtm % nit

https://codereview.chromium.org/2572483003/diff/20001/net/nqe/network_quality...
File net/nqe/network_quality_estimator.cc (right):

https://codereview.chromium.org/2572483003/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator.cc:48: namespace net {
Is this part of the change related? Can you add to the description that you are
cleaning up/refactoring?

https://codereview.chromium.org/2572483003/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator.cc:406: if (request.load_flags() &
LOAD_MAIN_FRAME_DEPRECATED) {
In a later CL can you refactor this block into a method like
"RecomputeConnectionMetricsOnMainFrameRequest".

It would be more clear when looking at UpdateSignalStrength() that it is called
when a main frame request happens or a connection type changes without having to
dig into NotifyHeadersReceived.

WDYT?

[tbansal1, 2016-12-13 21:14:03]

Description was changed from

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=513681
==========

to

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

Also, cleanup by moving the methods in the anonymous namespace to
an anonymous namespace in namespace net.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=513681
==========

[tbansal1, 2016-12-13 21:21:12]

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

[tbansal1, 2016-12-13 21:21:13]

bengr: ptal. Thanks.

https://codereview.chromium.org/2572483003/diff/20001/net/nqe/network_quality...
File net/nqe/network_quality_estimator.cc (right):

https://codereview.chromium.org/2572483003/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator.cc:48: namespace net {
On 2016/12/13 18:02:20, Ryan Sturm wrote:
> Is this part of the change related? Can you add to the description that you
are
> cleaning up/refactoring?

Done.

https://codereview.chromium.org/2572483003/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator.cc:406: if (request.load_flags() &
LOAD_MAIN_FRAME_DEPRECATED) {
On 2016/12/13 18:02:20, Ryan Sturm wrote:
> In a later CL can you refactor this block into a method like
> "RecomputeConnectionMetricsOnMainFrameRequest".
> 
> It would be more clear when looking at UpdateSignalStrength() that it is
called
> when a main frame request happens or a connection type changes without having
to
> dig into NotifyHeadersReceived.
> 
> WDYT?

Done.

[bengr, 2016-12-15 23:19:16]

lgtm

https://codereview.chromium.org/2572483003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2572483003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:39831: +    connections on Android
platform when the cellular signal strength was
on -> on the
was -> is

[tbansal1, 2016-12-16 17:44:14]

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

[tbansal1, 2016-12-16 17:44:15]

rkaplow: ptal at histograms.xml. Thanks.

[rkaplow, 2016-12-16 18:34:24]

lgtm

https://codereview.chromium.org/2572483003/diff/40001/net/nqe/network_quality...
File net/nqe/network_quality_estimator.cc (right):

https://codereview.chromium.org/2572483003/diff/40001/net/nqe/network_quality...
net/nqe/network_quality_estimator.cc:777: UMA_HISTOGRAM_COUNTS_100(
double checking 100 is a good max

[tbansal1, 2016-12-16 19:14:10]

https://codereview.chromium.org/2572483003/diff/40001/net/nqe/network_quality...
File net/nqe/network_quality_estimator.cc (right):

https://codereview.chromium.org/2572483003/diff/40001/net/nqe/network_quality...
net/nqe/network_quality_estimator.cc:777: UMA_HISTOGRAM_COUNTS_100(
On 2016/12/16 18:34:24, rkaplow wrote:
> double checking 100 is a good max

Yes, I think it is.

[tbansal1, 2016-12-16 19:14:35]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2016-12-16 19:14:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
ryansturm@chromium.org
Link to the patchset: https://codereview.chromium.org/2572483003/#ps40001
(title: "ryansturm comments")

[commit-bot: I haz the power, 2016-12-16 19:15:33]

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, 2016-12-17 00:00:36]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481915675902740,
"parent_rev": "92466f91c7a7ae129e19b1e24ed85bb8855f5204", "commit_rev":
"8bc82a3ded935d6f3e59a8c7fb4105258032a4e6"}

[commit-bot: I haz the power, 2016-12-17 00:01:04]

Description was changed from

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

Also, cleanup by moving the methods in the anonymous namespace to
an anonymous namespace in namespace net.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=513681
==========

to

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

Also, cleanup by moving the methods in the anonymous namespace to
an anonymous namespace in namespace net.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=513681

Review-Url: https://codereview.chromium.org/2572483003
==========

[commit-bot: I haz the power, 2016-12-17 00:01:07]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-12-17 00:03:02]

Description was changed from

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

Also, cleanup by moving the methods in the anonymous namespace to
an anonymous namespace in namespace net.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=513681

Review-Url: https://codereview.chromium.org/2572483003
==========

to

==========
Record difference between the maximum and minimum wireless signal strength

This will help in determining how the signal strength can be used to
adjust the network quality estimates.

Also, cleanup by moving the methods in the anonymous namespace to
an anonymous namespace in namespace net.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

BUG=513681

Committed: https://crrev.com/e85ca85b510f8289da6337eedaee2019dbe3c9a7
Cr-Commit-Position: refs/heads/master@{#439238}
==========

[commit-bot: I haz the power, 2016-12-17 00:03:03]

Patchset 2 (id:??) landed as
https://crrev.com/e85ca85b510f8289da6337eedaee2019dbe3c9a7
Cr-Commit-Position: refs/heads/master@{#439238}