NQE: Make params a class

NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to the Params class
from the NQE class to make it simpler to query the params again and 
again. This CL does not make any functional change.

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

Review-Url: https://codereview.chromium.org/2775223004
Cr-Commit-Position: refs/heads/master@{#460582}
Committed: https://chromium.googlesource.com/chromium/src/+/6b61964c84f3d3bdcb1f8bc0e136e992f9ae7758

[tbansal1, 2017-03-27 21:31:56]

Description was changed from

==========
NQE: Make params a class

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

to

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to within the Params class
from the NQE class.

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

[tbansal1, 2017-03-27 21:32:34]

Description was changed from

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to within the Params class
from the NQE class.

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

to

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to the Params class
from the NQE class to make it simpler to query the params again and 
again. This CL does not make any functional change.

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

[tbansal1, 2017-03-27 21:33:48]

Patchset #1 (id:1) has been deleted

[tbansal1, 2017-03-27 21:33:56]

Description was changed from

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to the Params class
from the NQE class to make it simpler to query the params again and 
again. This CL does not make any functional change.

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

to

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to the Params class
from the NQE class to make it simpler to query the params again and 
again. This CL does not make any functional change.

BUG=705691
==========

[tbansal1, 2017-03-27 21:35:27]

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

[tbansal1, 2017-03-27 21:35:28]

ryansturm: ptal. Thanks.

[tbansal1, 2017-03-27 21:36:04]

Description was changed from

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to the Params class
from the NQE class to make it simpler to query the params again and 
again. This CL does not make any functional change.

BUG=705691
==========

to

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to the Params class
from the NQE class to make it simpler to query the params again and 
again. This CL does not make any functional change.

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

[tbansal1, 2017-03-27 21:36:09]

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

[commit-bot: I haz the power, 2017-03-27 21:36: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-03-27 23:14:53]

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

[commit-bot: I haz the power, 2017-03-27 23:14:54]

Dry run: This issue passed the CQ dry run.

[RyanSturm, 2017-03-28 17:23:11]

I think the comments need to be changed, but it might be a matter of preference.

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

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator.h:653: // Params to config the network quality
estimator.
s/config/configure/?

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

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:23: class
NetworkQualityEstimatorParams {
class comment.

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:26: const std::map<std::string,
std::string>& params);
can you add a method comment to this constructor to detail what |params| is?

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:30: // Returns the algorithm that
should be used for computing effective
A lot of these comments can remove the references to |params|. Once the class is
established and you comment params above, the comments should describe what they
return without talking about field trials or params. For the invalid/defaults,
just mention what the default is:

// Returns the algorithm that should be used for computing effective
// connection type. Returns an empty string if a valid algorithm was
// not specified.

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:89: const std::map<std::string,
std::string> params_;
Add a comment to this.

[tbansal1, 2017-03-29 18:04:41]

Patchset #2 (id:40001) has been deleted

[tbansal1, 2017-03-29 18:17:40]

Patchset #2 (id:60001) has been deleted

[tbansal1, 2017-03-29 18:17:51]

ryansturm: ptal. Thanks.

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

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator.h:653: // Params to config the network quality
estimator.
On 2017/03/28 17:23:11, Ryan Sturm wrote:
> s/config/configure/?

Done.

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

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:23: class
NetworkQualityEstimatorParams {
On 2017/03/28 17:23:11, Ryan Sturm wrote:
> class comment.

Done.

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:26: const std::map<std::string,
std::string>& params);
On 2017/03/28 17:23:11, Ryan Sturm wrote:
> can you add a method comment to this constructor to detail what |params| is?

Done.

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:30: // Returns the algorithm that
should be used for computing effective
On 2017/03/28 17:23:11, Ryan Sturm wrote:
> A lot of these comments can remove the references to |params|. Once the class
is
> established and you comment params above, the comments should describe what
they
> return without talking about field trials or params. For the invalid/defaults,
> just mention what the default is:
> 
> // Returns the algorithm that should be used for computing effective
> // connection type. Returns an empty string if a valid algorithm was
> // not specified.

Done.

https://codereview.chromium.org/2775223004/diff/20001/net/nqe/network_quality...
net/nqe/network_quality_estimator_params.h:89: const std::map<std::string,
std::string> params_;
On 2017/03/28 17:23:11, Ryan Sturm wrote:
> Add a comment to this.

Done.

[RyanSturm, 2017-03-29 21:45:42]

lgtm

[tbansal1, 2017-03-29 21:46:34]

The CQ bit was checked by tbansal@chromium.org

[commit-bot: I haz the power, 2017-03-29 21:47:27]

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-29 23:53:53]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490823994469200,
"parent_rev": "a2c84a7664eceb5cbeaae5bf1ec32da0060e3097", "commit_rev":
"6b61964c84f3d3bdcb1f8bc0e136e992f9ae7758"}

[commit-bot: I haz the power, 2017-03-29 23:54:46]

Description was changed from

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to the Params class
from the NQE class to make it simpler to query the params again and 
again. This CL does not make any functional change.

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

to

==========
NQE: Make params a class

Make network quality estimator (NQE) params a class.

Next CL will move the storing of the params to the Params class
from the NQE class to make it simpler to query the params again and 
again. This CL does not make any functional change.

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

Review-Url: https://codereview.chromium.org/2775223004
Cr-Commit-Position: refs/heads/master@{#460582}
Committed:
https://chromium.googlesource.com/chromium/src/+/6b61964c84f3d3bdcb1f8bc0e136...
==========

[commit-bot: I haz the power, 2017-03-29 23:54:47]

Committed patchset #2 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/6b61964c84f3d3bdcb1f8bc0e136...