Wire WebFonts intervention v2 for ECT of 3G

Wire WebFonts intervention v2 for ECT of 3G

Wire Network Quality Estimator's (NQE)
WebEffectiveConnectionType to RemoteFontFaceSource to make
WebFonts intervention v2 effectively work when Effective
Connection Type (ECT) is 3G.

BUG=578029
Committed: https://crrev.com/22e15bab5fc56217572c1f6e09fc349751866fda
Cr-Commit-Position: refs/heads/master@{#419817}

[tbansal1, 2016-09-16 00:38:17]

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

[commit-bot: I haz the power, 2016-09-16 00:38:49]

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

[tbansal1, 2016-09-16 00:46:01]

Description was changed from

==========
web fonts intervention on ect 3g

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

to

==========
Wire WebFonts intervention v2 for ECT of 3G

Wire Network Quality Estimator's (NQE) 
WebEffectiveConnectionType to RemoteFontFaceSource to make
WebFonts intervention v2 effectively work when Effective
Connection Type (ECT) is 3G.

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

[tbansal1, 2016-09-16 00:46:35]

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

[tbansal1, 2016-09-16 00:46:36]

toyoshim: ptal at *. Thanks.

[tbansal1, 2016-09-16 00:46:38]

toyoshim: ptal at *. Thanks.

[commit-bot: I haz the power, 2016-09-16 01:56:44]

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

[commit-bot: I haz the power, 2016-09-16 01:56:45]

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

[Takashi Toyoshima, 2016-09-16 05:57:08]

Thank you for adding 3G!
Looks almost good.
Can you check the NOTREACHED case I commented?

[Takashi Toyoshima, 2016-09-16 05:57:16]

https://codereview.chromium.org/2349593002/diff/1/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2349593002/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:15133: +    <message
name="IDS_FLAGS_ENABLE_WEBFONTS_INTERVENTION_V2_CHOICE_ENABLED_WITH_3G"
desc="Text to indicate a experiment group name for
enable-webfonts-intervention-v2">
Not mandatory, but dictionary order would be better here. So, this new
description could follow the 2G description below.

The order of 2G, 3G, and slow 2G sounds weird in terms of connection speed,
but... it's a little like a matter of taste.

Same for other places there you are adding 3G variation.

https://codereview.chromium.org/2349593002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp (right):

https://codereview.chromium.org/2349593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp:37: NOTREACHED();
This line seems to be reached when slow-2g is specified.

[tbansal1, 2016-09-16 06:45:27]

toyoshim: ptal. thanks.

https://codereview.chromium.org/2349593002/diff/1/chrome/app/generated_resour...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2349593002/diff/1/chrome/app/generated_resour...
chrome/app/generated_resources.grd:15133: +    <message
name="IDS_FLAGS_ENABLE_WEBFONTS_INTERVENTION_V2_CHOICE_ENABLED_WITH_3G"
desc="Text to indicate a experiment group name for
enable-webfonts-intervention-v2">
On 2016/09/16 05:57:16, toyoshim wrote:
> Not mandatory, but dictionary order would be better here. So, this new
> description could follow the 2G description below.
> 
> The order of 2G, 3G, and slow 2G sounds weird in terms of connection speed,
> but... it's a little like a matter of taste.
> 
> Same for other places there you are adding 3G variation.

Using 2G, 3G, Slow2G in that order. It matches dictionary order, although
it sounds a bit weird (at least in terms of connection speed).

https://codereview.chromium.org/2349593002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp (right):

https://codereview.chromium.org/2349593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp:37: NOTREACHED();
On 2016/09/16 05:57:16, toyoshim wrote:
> This line seems to be reached when slow-2g is specified.

Done.

[Takashi Toyoshima, 2016-09-16 09:20:25]

Thanks. LGTM.

[tbansal1, 2016-09-16 15:58:58]

tbansal@chromium.org changed reviewers:
+ kinuko@chromium.org, tkent@chromium.org

[tbansal1, 2016-09-16 15:58:58]

kinuko: ptal at content/
tkent: third_part/Webkit/
Thanks.

[tbansal1, 2016-09-16 16:03:48]

Description was changed from

==========
Wire WebFonts intervention v2 for ECT of 3G

Wire Network Quality Estimator's (NQE) 
WebEffectiveConnectionType to RemoteFontFaceSource to make
WebFonts intervention v2 effectively work when Effective
Connection Type (ECT) is 3G.

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

to

==========
Wire WebFonts intervention v2 for ECT of 3G

Wire Network Quality Estimator's (NQE) 
WebEffectiveConnectionType to RemoteFontFaceSource to make
WebFonts intervention v2 effectively work when Effective
Connection Type (ECT) is 3G.

BUG=578029
==========

[tkent, 2016-09-20 00:40:22]

third_part/Webkit/ lgtm

[Takashi Toyoshima, 2016-09-20 06:46:44]

tbansal, since kinuko is taking a maternity leave now, you may want to find
another reviewer for content

[kinuko, 2016-09-20 14:01:13]

On 2016/09/20 06:46:44, toyoshim wrote:
> tbansal, since kinuko is taking a maternity leave now, you may want to find
> another reviewer for content

Just found this; rs/lgtm for content/

[tbansal1, 2016-09-20 16:40:02]

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

[commit-bot: I haz the power, 2016-09-20 16:40: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, 2016-09-20 18:13:47]

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

[commit-bot: I haz the power, 2016-09-20 18:13:48]

Dry run: This issue passed the CQ dry run.

[tbansal1, 2016-09-20 18:14:09]

The CQ bit was checked by tbansal@chromium.org

[tbansal1, 2016-09-20 18:14:09]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
toyoshim@chromium.org, tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/2349593002/#ps40001
(title: "rebased")

[commit-bot: I haz the power, 2016-09-20 18:14:37]

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-09-20 18:20:43]

Description was changed from

==========
Wire WebFonts intervention v2 for ECT of 3G

Wire Network Quality Estimator's (NQE) 
WebEffectiveConnectionType to RemoteFontFaceSource to make
WebFonts intervention v2 effectively work when Effective
Connection Type (ECT) is 3G.

BUG=578029
==========

to

==========
Wire WebFonts intervention v2 for ECT of 3G

Wire Network Quality Estimator's (NQE) 
WebEffectiveConnectionType to RemoteFontFaceSource to make
WebFonts intervention v2 effectively work when Effective
Connection Type (ECT) is 3G.

BUG=578029
==========

[commit-bot: I haz the power, 2016-09-20 18:20:44]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-20 18:23:29]

Description was changed from

==========
Wire WebFonts intervention v2 for ECT of 3G

Wire Network Quality Estimator's (NQE) 
WebEffectiveConnectionType to RemoteFontFaceSource to make
WebFonts intervention v2 effectively work when Effective
Connection Type (ECT) is 3G.

BUG=578029
==========

to

==========
Wire WebFonts intervention v2 for ECT of 3G

Wire Network Quality Estimator's (NQE)
WebEffectiveConnectionType to RemoteFontFaceSource to make
WebFonts intervention v2 effectively work when Effective
Connection Type (ECT) is 3G.

BUG=578029

Committed: https://crrev.com/22e15bab5fc56217572c1f6e09fc349751866fda
Cr-Commit-Position: refs/heads/master@{#419817}
==========

[commit-bot: I haz the power, 2016-09-20 18:23:30]

Patchset 3 (id:??) landed as
https://crrev.com/22e15bab5fc56217572c1f6e09fc349751866fda
Cr-Commit-Position: refs/heads/master@{#419817}

[Takashi Toyoshima, 2016-09-21 06:31:44]

Thanks, Kinuko-san :)