Adding a parameter for previews NQE triggering threshold

Adding a parameter for previews NQE triggering threshold

This paramater allows triggering previews at different
EffectiveConnectionTypes. Currently, previews are triggered only on
SLOW_2G, but in the future we may wish to change it to 2G without
needing to wait for release cycles. This change also adds some basic
testing around all of the previews params.

BUG=687757
Review-Url: https://codereview.chromium.org/2667243003
Cr-Commit-Position: refs/heads/master@{#447693}
Committed: https://chromium.googlesource.com/chromium/src/+/34be2d9f24b8c8b4056eba4d477321762bb783df

[RyanSturm, 2017-02-02 00:04:43]

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

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

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

[RyanSturm, 2017-02-02 00:05:57]

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

[RyanSturm, 2017-02-02 00:05:58]

tbansal: PTAL

[tbansal1, 2017-02-02 00:13:40]

lgtm % nits. Thanks for doing this.

https://codereview.chromium.org/2667243003/diff/1/components/previews/core/pr...
File components/previews/core/previews_experiments.cc (right):

https://codereview.chromium.org/2667243003/diff/1/components/previews/core/pr...
components/previews/core/previews_experiments.cc:170: if
(!base::StringToInt(param_value, &effective_type_value)) {
You may want to use this function:
https://cs.chromium.org/chromium/src/net/nqe/effective_connection_type.cc?rcl...
if you prefer a more descriptive name in the config files.

https://codereview.chromium.org/2667243003/diff/1/components/previews/core/pr...
components/previews/core/previews_experiments.cc:171: effective_type_value =
Instead of 2 static casts, may be just return E_C_T_SLOW2G here.

[commit-bot: I haz the power, 2017-02-02 00:20:40]

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

[commit-bot: I haz the power, 2017-02-02 00:20:40]

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...)

[RyanSturm, 2017-02-02 00:21:41]

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

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

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

[RyanSturm, 2017-02-02 00:22:28]

https://codereview.chromium.org/2667243003/diff/1/components/previews/core/pr...
File components/previews/core/previews_experiments.cc (right):

https://codereview.chromium.org/2667243003/diff/1/components/previews/core/pr...
components/previews/core/previews_experiments.cc:170: if
(!base::StringToInt(param_value, &effective_type_value)) {
On 2017/02/02 00:13:40, tbansal1 wrote:
> You may want to use this function:
>
https://cs.chromium.org/chromium/src/net/nqe/effective_connection_type.cc?rcl...
> if you prefer a more descriptive name in the config files.

Done. Thanks.

https://codereview.chromium.org/2667243003/diff/1/components/previews/core/pr...
components/previews/core/previews_experiments.cc:171: effective_type_value =
On 2017/02/02 00:13:40, tbansal1 wrote:
> Instead of 2 static casts, may be just return E_C_T_SLOW2G here.

Acknowledged.

[tbansal1, 2017-02-02 00:25:33]

lgtm.

[RyanSturm, 2017-02-02 00:41:32]

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

[commit-bot: I haz the power, 2017-02-02 00:41:44]

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-02 00:55:57]

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

[commit-bot: I haz the power, 2017-02-02 00:55:57]

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...)

[RyanSturm, 2017-02-02 01:22:26]

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

[commit-bot: I haz the power, 2017-02-02 01:22:50]

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

[RyanSturm, 2017-02-02 01:36:46]

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

[RyanSturm, 2017-02-02 01:36:57]

Patchset #3 (id:40001) has been deleted

[RyanSturm, 2017-02-02 01:37:03]

Patchset #3 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-02-02 01:37:14]

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

[RyanSturm, 2017-02-02 01:37:53]

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

[commit-bot: I haz the power, 2017-02-02 01:38:18]

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

[RyanSturm, 2017-02-02 02:20:52]

The CQ bit was unchecked by ryansturm@chromium.org

[RyanSturm, 2017-02-02 02:20:52]

The CQ bit was checked by ryansturm@chromium.org

[RyanSturm, 2017-02-02 02:20:52]

The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/2667243003/#ps100001
(title: "typo")

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

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-02 03:22:07]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1486002052382370,
"parent_rev": "41fd6ea9c3ae787b3133e96cd2e9a845056dbd24", "commit_rev":
"34be2d9f24b8c8b4056eba4d477321762bb783df"}

[commit-bot: I haz the power, 2017-02-02 03:26:12]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1486002052382370,
"parent_rev": "41fd6ea9c3ae787b3133e96cd2e9a845056dbd24", "commit_rev":
"34be2d9f24b8c8b4056eba4d477321762bb783df"}

[commit-bot: I haz the power, 2017-02-02 03:26:16]

Description was changed from

==========
Adding a parameter for previews NQE triggering threshold

This paramater allows triggering previews at different
EffectiveConnectionTypes. Currently, previews are triggered only on
SLOW_2G, but in the future we may wish to change it to 2G without
needing to wait for release cycles. This change also adds some basic
testing around all of the previews params.

BUG=687757
==========

to

==========
Adding a parameter for previews NQE triggering threshold

This paramater allows triggering previews at different
EffectiveConnectionTypes. Currently, previews are triggered only on
SLOW_2G, but in the future we may wish to change it to 2G without
needing to wait for release cycles. This change also adds some basic
testing around all of the previews params.

BUG=687757

Review-Url: https://codereview.chromium.org/2667243003
Cr-Commit-Position: refs/heads/master@{#447693}
Committed:
https://chromium.googlesource.com/chromium/src/+/34be2d9f24b8c8b4056eba4d4773...
==========

[commit-bot: I haz the power, 2017-02-02 03:26:17]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/34be2d9f24b8c8b4056eba4d4773...