Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2763523002:
Lower the webfont loading priority only if the user is in V2 field trial (Closed)
DescriptionLower the webfont loading priority only if the user is in V2 field trial
Currently, the loading priority of web fonts is changed to Low if the
user is on 2G network (regardless of if Chrome is a part of
WebFontsInterventionV2 field trial or not). This CL adds an additional
constraint so that the loading priority is changed only if Chrome is
part of V2 field trial. This makes it possible to quantify the effect
of changing the loading priority as part of the WebFontsInterventionV2
field trial.
BUG=665504
Review-Url: https://codereview.chromium.org/2763523002
Cr-Commit-Position: refs/heads/master@{#458681}
Committed: https://chromium.googlesource.com/chromium/src/+/fd4fb29de4630be62b296e01987d7bd09d0e4a6e
Patch Set 1 #
Total comments: 2
Patch Set 2 : toyoshim comments #Messages
Total messages: 22 (14 generated)
Description was changed from ========== Webfont fix BUG= ========== to ========== Lower the webfont loading priority only if the user is in V2 field trial Currently, the loading priority of web fonts is changed to Low if the user is on 2G network (regardless of if Chrome is a part of WebFontsInterventionV2 field trial or not). This CL adds an additional constraint so that the loading priority is changed only if Chrome is part of V2 field trial. This makes it possible to quantify the effect of changing the loading priority as part of the WebFontsInterventionV2 field trial. BUG=665504 ==========
The CQ bit was checked by tbansal@chromium.org to run a CQ dry run
tbansal@chromium.org changed reviewers: + toyoshim@chromium.org
toyoshim: ptal. Thanks.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2763523002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp (right): https://codereview.chromium.org/2763523002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp:200: bool isV2Enabled = We have same code here. Can you factor out these checks as a function, isWebFontInterventionV2Enabled() in anonymous namespace, and use it from both two places?
toyoshim: ptal. Thanks. https://codereview.chromium.org/2763523002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp (right): https://codereview.chromium.org/2763523002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp:200: bool isV2Enabled = On 2017/03/21 08:46:58, Takashi Toyoshima wrote: > We have same code here. > Can you factor out these checks as a function, isWebFontInterventionV2Enabled() > in anonymous namespace, and use it from both two places? Done.
The CQ bit was checked by tbansal@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
tbansal@chromium.org changed reviewers: + kinuko@chromium.org
kinuko: ptal. Thanks.
lgtm
The CQ bit was checked by tbansal@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490169717402400,
"parent_rev": "2ea907e4d0ea17ae9bc9293694db1128f2237ddc", "commit_rev":
"fd4fb29de4630be62b296e01987d7bd09d0e4a6e"}
Message was sent while issue was closed.
Description was changed from ========== Lower the webfont loading priority only if the user is in V2 field trial Currently, the loading priority of web fonts is changed to Low if the user is on 2G network (regardless of if Chrome is a part of WebFontsInterventionV2 field trial or not). This CL adds an additional constraint so that the loading priority is changed only if Chrome is part of V2 field trial. This makes it possible to quantify the effect of changing the loading priority as part of the WebFontsInterventionV2 field trial. BUG=665504 ========== to ========== Lower the webfont loading priority only if the user is in V2 field trial Currently, the loading priority of web fonts is changed to Low if the user is on 2G network (regardless of if Chrome is a part of WebFontsInterventionV2 field trial or not). This CL adds an additional constraint so that the loading priority is changed only if Chrome is part of V2 field trial. This makes it possible to quantify the effect of changing the loading priority as part of the WebFontsInterventionV2 field trial. BUG=665504 Review-Url: https://codereview.chromium.org/2763523002 Cr-Commit-Position: refs/heads/master@{#458681} Committed: https://chromium.googlesource.com/chromium/src/+/fd4fb29de4630be62b296e01987d... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/fd4fb29de4630be62b296e01987d... |
