Bug 49898 – LayoutTests/fast/css/font-face-data-uri.html doesn't test what it claims to

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[anton muhin]

http://trac.webkit.org/changeset/72504 added the test in the subj.  And it apparently requires proper expectations: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Fcss%2Ffont-face-data-uri.html&master=ChromiumWebkit

Yuzo, may you have a look if diffs are ok for Chromium?

Disabling for now.

[Yuzo Fujishima]

Hi,

It should also pass for Chromium, with the same *-expected.txt, once WebKit for Chromium is rolled past 72504.

[anton muhin]

Are you sure?  It doesn't pass on canaries which are using WebKit's ToT

(In reply to comment #1)
> Hi,
> 
> It should also pass for Chromium, with the same *-expected.txt, once WebKit for Chromium is rolled past 72504.

[Yuzo Fujishima]

Hmm, weird.

Where did you see the failures?

I checked http://build.chromium.org/p/chromium.webkit/waterfall but
didn't see the failures there. (font-face-data-uri.html is said to be failing/flaky
but no failures are shown in http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcss%2Ffont-face-data-uri.html%2Csvg%2Fcustom%2Fuse-infinite-recursion.svg%2Cfast%2Ftext%2Ffind-backwards.html%2Cmedia%2Fvideo-play-pause-exception.html%2Csvg%2Fcustom%2FselectSubString.html )

[anton muhin]

(In reply to comment #3)
> Hmm, weird.
> 
> Where did you see the failures?
> 
> I checked http://build.chromium.org/p/chromium.webkit/waterfall but
> didn't see the failures there. (font-face-data-uri.html is said to be failing/flaky
> but no failures are shown in http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcss%2Ffont-face-data-uri.html%2Csvg%2Fcustom%2Fuse-infinite-recursion.svg%2Cfast%2Ftext%2Ffind-backwards.html%2Cmedia%2Fvideo-play-pause-exception.html%2Csvg%2Fcustom%2FselectSubString.html )

Yuzo, see the URL to dashboard in the first comment.  I duplicate it here for your convenience:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Fcss%2Ffont-face-data-uri.html&master=ChromiumWebkit

[Yuzo Fujishima]

Hi, Anton,

From which waterfall did you get the dashboard URL?

[anton muhin]

Hi, Yuzo.

It is http://build.chromium.org/p/chromium.webkit/waterfall

See for example http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Linux/builds/2346

(In reply to comment #5)
> Hi, Anton,
> 
> From which waterfall did you get the dashboard URL?

[Yuzo Fujishima]

LayoutTests/fast/css/font-face-data-uri.html intends to compare text rendering by local font to that by data URI-encoded web font, but it actually doesn't. It is passing for Mac just by accident.

I'll upload a patch shortly.

[Yuzo Fujishima]

Renamed the bug because this is not really specific to Chromium.

[Yuzo Fujishima]

Created attachment 74837 [details]
Patch

[Kent Tamura]

Comment on attachment 74837 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74837&action=review

> LayoutTests/fast/css/font-face-data-uri.html:50
> +    window.setTimeout(testMain, 200);

Is the font loaded asynchronously?  Can we know font loading completion in JavaScript?
This setTimeout() looks to make this test flaky.

[Shinichiro Hamaji]

Comment on attachment 74837 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74837&action=review

r- for Kent-san's comment. One more comment from me.

> LayoutTests/fast/css/font-face-data-uri.html:40
> +    document.body.offsetTop;

I think it would be better to comment the intention of this line.

[Yuzo Fujishima]

Created attachment 74840 [details]
Added comment

[Yuzo Fujishima]

Thank you for the review.

As to document.body.offsetTop,
I added a comment.

As to the use of timer,
there is currently no good way to know when the font download completes.
As far as I tested, 200 msec is nearly the shortest wait without flakiness.

[Yuzo Fujishima]

Created attachment 74897 [details]
Use polling to wait for font load completion

[Kent Tamura]

Comment on attachment 74897 [details]
Use polling to wait for font load completion

ok

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 74897 [details]:

fast/css/font-face-data-uri.html

Please file bugs against the tests.  These tests were authored by [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 74897 [details]
Use polling to wait for font load completion

Clearing flags on attachment: 74897

Committed r72749: <http://trac.webkit.org/changeset/72749>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/72749 might have broken SnowLeopard Intel Release (Tests)
The following tests are not passing:
fast/css/font-face-data-uri.html

[Eric Seidel (no email)]

It looks like this made the test flakey and should be rolled out.  No?

[Yuzo Fujishima]

Sorry, but r72751 seems to have fixed the flakiness.

[Yuzo Fujishima]

Updated the Chromium test expectation (r72752).