Convert CSS font-display test to wpt

Convert CSS font-display test to wpt

BUG=724932
Review-Url: https://codereview.chromium.org/2895123002
Cr-Commit-Position: refs/heads/master@{#475843}
Committed: https://chromium.googlesource.com/chromium/src/+/0f84373780cd004423764f91042dfba40b8ff83f

[Kunihiko Sakamoto, 2017-05-22 03:55:09]

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

[commit-bot: I haz the power, 2017-05-22 03:55: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-22 05:18:37]

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

[commit-bot: I haz the power, 2017-05-22 05:18:38]

Dry run: This issue passed the CQ dry run.

[Kunihiko Sakamoto, 2017-05-22 06:29:38]

Description was changed from

==========
Convert CSS font-display test to wpt

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

to

==========
Convert CSS font-display test to wpt

BUG=724932
==========

[Kunihiko Sakamoto, 2017-05-22 09:54:50]

ksakamoto@chromium.org changed reviewers:
+ foolip@chromium.org

[Kunihiko Sakamoto, 2017-05-22 09:54:51]

Philip, could you take a look?

[foolip, 2017-05-24 11:49:31]

Sorry I haven't had time to look at this yet. I'll be OOO for a while, if
necessary please ask another reviewer.

[foolip, 2017-05-29 15:35:20]

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display-expected.html
(right):

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display-expected.html:4:
@font-face {
This block ought not be needed, wpt assumes that Ahem is present. If you remove
it, does the test pass anyway?

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html
(right):

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html:4:
<link rel="match" href="font-display-expected.html">
Please name this font-display-ref.html instead, that's the normal naming
convention in web-platform-tests. We have -expected.txt files that are local
only and not upstreamed, so having -expected.html might be confusing too.

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html:31:
url = 'resources/slow-ahem-loading.py?ms=' + config.delay + '&t=' + config.time;
The t parameter isn't read by slow-ahem-loading.py, should it be, or should it
be removed from the config here?

[Kunihiko Sakamoto, 2017-05-30 04:08:35]

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

[commit-bot: I haz the power, 2017-05-30 04:09:01]

Dry run: CQ is trying da patch.

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

[Kunihiko Sakamoto, 2017-05-30 04:11:10]

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display-expected.html
(right):

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display-expected.html:4:
@font-face {
On 2017/05/29 15:35:20, foolip wrote:
> This block ought not be needed, wpt assumes that Ahem is present. If you
remove
> it, does the test pass anyway?

Ah, the test passed without this rule. Removed.

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html
(right):

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html:4:
<link rel="match" href="font-display-expected.html">
On 2017/05/29 15:35:20, foolip wrote:
> Please name this font-display-ref.html instead, that's the normal naming
> convention in web-platform-tests. We have -expected.txt files that are local
> only and not upstreamed, so having -expected.html might be confusing too.

Done.

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html:31:
url = 'resources/slow-ahem-loading.py?ms=' + config.delay + '&t=' + config.time;
On 2017/05/29 15:35:20, foolip wrote:
> The t parameter isn't read by slow-ahem-loading.py, should it be, or should it
> be removed from the config here?

|t| is a dummy parameter that makes sure every config has unique font URL.
(Although it's not really needed currently, as |delay| parameter is already
unique.)

[commit-bot: I haz the power, 2017-05-30 06:14:14]

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

[commit-bot: I haz the power, 2017-05-30 06:14:16]

Dry run: This issue passed the CQ dry run.

[foolip, 2017-05-30 11:35:29]

lgtm % |t|

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html
(right):

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html:31:
url = 'resources/slow-ahem-loading.py?ms=' + config.delay + '&t=' + config.time;
On 2017/05/30 04:11:10, Kunihiko Sakamoto wrote:
> On 2017/05/29 15:35:20, foolip wrote:
> > The t parameter isn't read by slow-ahem-loading.py, should it be, or should
it
> > be removed from the config here?
> 
> |t| is a dummy parameter that makes sure every config has unique font URL.
> (Although it's not really needed currently, as |delay| parameter is already
> unique.)

Since it's not needed, can you remove it? It now very much looks like it's
supposed to do something timing-related and that it's a mistake that it doesn't.

[Kunihiko Sakamoto, 2017-05-31 04:53:50]

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html
(right):

https://codereview.chromium.org/2895123002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/css-font-display/font-display.html:31:
url = 'resources/slow-ahem-loading.py?ms=' + config.delay + '&t=' + config.time;
On 2017/05/30 11:35:29, foolip wrote:
> On 2017/05/30 04:11:10, Kunihiko Sakamoto wrote:
> > On 2017/05/29 15:35:20, foolip wrote:
> > > The t parameter isn't read by slow-ahem-loading.py, should it be, or
should
> it
> > > be removed from the config here?
> > 
> > |t| is a dummy parameter that makes sure every config has unique font URL.
> > (Although it's not really needed currently, as |delay| parameter is already
> > unique.)
> 
> Since it's not needed, can you remove it? It now very much looks like it's
> supposed to do something timing-related and that it's a mistake that it
doesn't.

OK, removed.

[Kunihiko Sakamoto, 2017-05-31 04:54:05]

The CQ bit was checked by ksakamoto@chromium.org

[Kunihiko Sakamoto, 2017-05-31 04:54:06]

The patchset sent to the CQ was uploaded after l-g-t-m from foolip@chromium.org
Link to the patchset: https://codereview.chromium.org/2895123002/#ps40001
(title: "removed |t|")

[commit-bot: I haz the power, 2017-05-31 04:54:20]

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-31 05:00:55]

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

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

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Kunihiko Sakamoto, 2017-05-31 05:12:03]

The CQ bit was checked by ksakamoto@chromium.org

[Kunihiko Sakamoto, 2017-05-31 05:12:04]

The patchset sent to the CQ was uploaded after l-g-t-m from foolip@chromium.org
Link to the patchset: https://codereview.chromium.org/2895123002/#ps60001
(title: "rebase, update network service expectation")

[commit-bot: I haz the power, 2017-05-31 05:12:19]

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-31 06:42:32]

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

[commit-bot: I haz the power, 2017-05-31 06:42:33]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Kunihiko Sakamoto, 2017-05-31 06:48:15]

The CQ bit was checked by ksakamoto@chromium.org

[commit-bot: I haz the power, 2017-05-31 06:48: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, 2017-05-31 07:53:29]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1496213295868730,
"parent_rev": "b9f51ee9599e574847906c650240ea2e4fbc69fb", "commit_rev":
"0f84373780cd004423764f91042dfba40b8ff83f"}

[commit-bot: I haz the power, 2017-05-31 07:53:38]

Description was changed from

==========
Convert CSS font-display test to wpt

BUG=724932
==========

to

==========
Convert CSS font-display test to wpt

BUG=724932

Review-Url: https://codereview.chromium.org/2895123002
Cr-Commit-Position: refs/heads/master@{#475843}
Committed:
https://chromium.googlesource.com/chromium/src/+/0f84373780cd004423764f91042d...
==========

[commit-bot: I haz the power, 2017-05-31 07:53:40]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/0f84373780cd004423764f91042d...