NQE: Move params from the estimator class to the params class

NQE: Move params from the estimator class to the params class

Move the network quality estimator (NQE) config params from
n_q_e.cc to n_q_e_params class. This is a refactor change with no
functional change. Long term, this would make it easier for net
embedders (Chrome and Cronet embedders) to configure NQE.

BUG=638308
TBR=bengr@chromium.org

Review-Url: https://codereview.chromium.org/2858743002
Cr-Commit-Position: refs/heads/master@{#469353}
Committed: https://chromium.googlesource.com/chromium/src/+/35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281

[tbansal1, 2017-05-02 20:55:25]

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

[commit-bot: I haz the power, 2017-05-02 20:55:46]

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-05-02 21:10:26]

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

[commit-bot: I haz the power, 2017-05-02 21:10:26]

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

[tbansal1, 2017-05-03 01:04:32]

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

[commit-bot: I haz the power, 2017-05-03 01:05:22]

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-05-03 03:06:07]

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

[commit-bot: I haz the power, 2017-05-03 03:06:08]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-05-03 16:36:56]

Description was changed from

==========
NQE params as class

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

to

==========
NQE: Move params from the estimator class to the params class

Move the network quality estimator (NQE) config params from
n_q_e.cc to n_q_e_params class. This is a refactor change with no
functional change. Long term, this would make it easier for net
embedders (Chrome and Cronet embedders) to configure NQE.

BUG=638308
==========

[tbansal1, 2017-05-03 16:37:03]

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

[tbansal1, 2017-05-03 16:37:15]

Patchset #2 (id:20001) has been deleted

[tbansal1, 2017-05-03 16:37:21]

Patchset #2 (id:40001) has been deleted

[commit-bot: I haz the power, 2017-05-03 16:37:27]

Dry run: CQ is trying da patch.

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

[tbansal1, 2017-05-03 16:37:29]

Patchset #1 (id:1) has been deleted

[tbansal1, 2017-05-03 16:37:47]

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

[tbansal1, 2017-05-03 16:37:48]

ryansturm: ptal. Thanks.

[RyanSturm, 2017-05-03 17:41:02]

lgtm % nits

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

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
net/nqe/network_quality_estimator.cc:625: //
|params_.correlation_uma_logging_probability()| to reduce overhead involved
Just remove this reference to the logging probability method; it's very obvious.

"The histogram is recorded randomly to reduce overhead involved with parse
histograms. Recording the correlation on each request is unnecessary."

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
File net/nqe/network_quality_estimator.h (left):

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
net/nqe/network_quality_estimator.h:819: const bool
persistent_cache_reading_enabled_;
move this too

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
File net/nqe/network_quality_estimator_params.h (right):

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:81: // Returns an uninitialized value
if the effective connection type has not
s/unitialized/unset/

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:82: // been forced via the |params|
provided to this class. Returns an initialized
s/an initialized/a/

[commit-bot: I haz the power, 2017-05-03 17:45:10]

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

[commit-bot: I haz the power, 2017-05-03 17:45:11]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-05-03 23:16:43]

Patchset #2 (id:80001) has been deleted

[tbansal1, 2017-05-03 23:17:26]

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

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
net/nqe/network_quality_estimator.cc:625: //
|params_.correlation_uma_logging_probability()| to reduce overhead involved
On 2017/05/03 17:41:02, Ryan Sturm wrote:
> Just remove this reference to the logging probability method; it's very
obvious.
> 
> "The histogram is recorded randomly to reduce overhead involved with parse
> histograms. Recording the correlation on each request is unnecessary."

Done.

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
File net/nqe/network_quality_estimator.h (left):

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
net/nqe/network_quality_estimator.h:819: const bool
persistent_cache_reading_enabled_;
On 2017/05/03 17:41:02, Ryan Sturm wrote:
> move this too

Done.

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
File net/nqe/network_quality_estimator_params.h (right):

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:81: // Returns an uninitialized value
if the effective connection type has not
On 2017/05/03 17:41:02, Ryan Sturm wrote:
> s/unitialized/unset/

Done.

https://codereview.chromium.org/2858743002/diff/60001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:82: // been forced via the |params|
provided to this class. Returns an initialized
On 2017/05/03 17:41:02, Ryan Sturm wrote:
> s/an initialized/a/

Done.

[tbansal1, 2017-05-03 23:17:33]

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

[commit-bot: I haz the power, 2017-05-03 23:18:17]

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-05-04 00:32:00]

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

[commit-bot: I haz the power, 2017-05-04 00:32:02]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2017-05-04 16:18:20]

Description was changed from

==========
NQE: Move params from the estimator class to the params class

Move the network quality estimator (NQE) config params from
n_q_e.cc to n_q_e_params class. This is a refactor change with no
functional change. Long term, this would make it easier for net
embedders (Chrome and Cronet embedders) to configure NQE.

BUG=638308
==========

to

==========
NQE: Move params from the estimator class to the params class

Move the network quality estimator (NQE) config params from
n_q_e.cc to n_q_e_params class. This is a refactor change with no
functional change. Long term, this would make it easier for net
embedders (Chrome and Cronet embedders) to configure NQE.

BUG=638308
TBR=bengr@chromium.org
==========

[tbansal1, 2017-05-04 16:18:42]

TBR'ing bengr for BUILD.gn.

[tbansal1, 2017-05-04 16:18:46]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2017-05-04 16:18:46]

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/2858743002/#ps100001
(title: "ryansturm comments")

[commit-bot: I haz the power, 2017-05-04 16:19:11]

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-05-04 16:23:36]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493914726346570,
"parent_rev": "b3500e91d389f23d3af70a8f8ac1afa21b1432d7", "commit_rev":
"35bdb0b1aa17566fbdbc83d4dffa6ef4946c2281"}

[commit-bot: I haz the power, 2017-05-04 16:24:08]

Description was changed from

==========
NQE: Move params from the estimator class to the params class

Move the network quality estimator (NQE) config params from
n_q_e.cc to n_q_e_params class. This is a refactor change with no
functional change. Long term, this would make it easier for net
embedders (Chrome and Cronet embedders) to configure NQE.

BUG=638308
TBR=bengr@chromium.org
==========

to

==========
NQE: Move params from the estimator class to the params class

Move the network quality estimator (NQE) config params from
n_q_e.cc to n_q_e_params class. This is a refactor change with no
functional change. Long term, this would make it easier for net
embedders (Chrome and Cronet embedders) to configure NQE.

BUG=638308
TBR=bengr@chromium.org

Review-Url: https://codereview.chromium.org/2858743002
Cr-Commit-Position: refs/heads/master@{#469353}
Committed:
https://chromium.googlesource.com/chromium/src/+/35bdb0b1aa17566fbdbc83d4dffa...
==========

[commit-bot: I haz the power, 2017-05-04 16:24:09]

Committed patchset #2 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/35bdb0b1aa17566fbdbc83d4dffa...