Summary: | LayoutTests/fast/css/font-face-data-uri.html doesn't test what it claims to | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | anton muhin <antonm> | ||||||||
Component: | WebCore Misc. | Assignee: | Yuzo Fujishima <yuzo> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, commit-queue, eric, jparent, webkit.review.bot, yuzo | ||||||||
Priority: | P3 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
anton muhin
2010-11-22 02:23:29 PST
Hi, It should also pass for Chromium, with the same *-expected.txt, once WebKit for Chromium is rolled past 72504. 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. 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 ) (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 Hi, Anton, From which waterfall did you get the dashboard URL? 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? 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. Renamed the bug because this is not really specific to Chromium. Created attachment 74837 [details]
Patch
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. 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. Created attachment 74840 [details]
Added comment
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. Created attachment 74897 [details]
Use polling to wait for font load completion
Comment on attachment 74897 [details]
Use polling to wait for font load completion
ok
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 yuzo@google.com. The commit-queue is continuing to process your patch. 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> All reviewed patches have been landed. Closing bug. 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 It looks like this made the test flakey and should be rolled out. No? Sorry, but r72751 seems to have fixed the flakiness. Updated the Chromium test expectation (r72752). |