Summary: | [Qt][WK2] Canvas tests make css2.1 ones failing | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Balazs Kelemen <kbalazs> | ||||
Component: | WebKit2 | Assignee: | Zoltan Herczeg <zherczeg> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cshu, jesus, laszlo.gombos, ossy, pierre.rossi, zecke | ||||
Priority: | P3 | Keywords: | LayoutTestFailure, Qt, QtTriaged | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Balazs Kelemen
2011-01-31 08:48:46 PST
I have reproduced the problem on a different 32 bit Linux environment. Since the css2.1 tests are testing more basic and so important parts, I think it would be better to skip the canvas tests for now. I am going to upload the patch. After a look of the number of the css2.1 and the canvas tests, and some discussion with Ossy I have rejected the idea of skipping the canvas tests. Some snippet from the output of run-webkit-tests -2 --verbose running canvas/philip/tests/type.extend.html -> succeeded running canvas/philip/tests/type.name.html -> succeeded running canvas/philip/tests/type.replace.html -> succeeded running css1/box_properties/float_on_text_elements.html -> failed running css1/classification/white_space.html -> failed running css1/units/zero-duration-without-units.html -> succeeded running css2.1/t09-c5526c-display-00-e.html -> failed running css2.1/t0402-c71-fwd-parsing-00-f.html -> failed running css2.1/t0402-c71-fwd-parsing-01-f.html -> failed running css2.1/t0402-c71-fwd-parsing-02-f.html -> failed running css2.1/t0402-c71-fwd-parsing-03-f.html -> failed running css2.1/t0402-c71-fwd-parsing-04-f.html -> failed ... (lot of failing css2.1 tests) running css2.1/t1204-root-e.html -> failed running css2.1/t1205-c561-list-displ-00-b.html -> failed | Closing DumpTool | | Opening DumpTool | running css2.1/t1205-c563-list-type-00-b.html -> succeeded running css2.1/t1205-c563-list-type-01-b.html -> succeeded running css2.1/t1205-c564-list-img-00-b-g.html -> succeeded running css2.1/t1205-c565-list-pos-00-b.html -> succeeded ... (almost all the css2.1 tests runs successfully from here) QApplication::fontMetrics().averageCharWidth() doubles during the running of the test. Something affects this member. Do you have any idea? (Besides QApplication::setFont() which is not happen) Big progress! The increase happen during: WebCore/bindings/js/GCController.cpp: gcTimerFired() Don't ask how I found it ;) I suspect a Qt specific object do bad things during its destructor. Another big step: Source/WebCore/html/HTMLCanvasElement.cpp HTMLCanvasElement::~HTMLCanvasElement() m_context.clear(); Can cause this. Tee conditions are not yet known. FontCustomPlatformDataQt.cpp QFontDatabase::removeApplicationFont(m_handle); - is actually successful in case of WebKit2's DRT, but fails with normal DRT. I think the normal DRT has an extra reference to the font, which keeps it alive (so the fail hides the issue). Zecke, you made this code before. Maybe you have some idea. If I comment out the mentioned line, the tests are passed. FontCustomPlatformData::~FontCustomPlatformData() { - QFontDatabase::removeApplicationFont(m_handle); +// QFontDatabase::removeApplicationFont(m_handle); } Now I know whz the tests are passed in DRT: DumpRenderTree::open() calls QFontDatabase::removeAllApplicationFonts() this call hides this particular bug. Since the Qt port moves to QRawFont, QFontDatabase will be dropped from the source, it is enough to mimic the DRT behaviour in WKTR for now, even if it is wrong. I have no better idea at the moment. Created attachment 96555 [details]
patch
Ugly, but this is how DRT works, and just hides the bug. We need to refactor and fix the platform font management code in the future.
Comment on attachment 96555 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=96555&action=review You're right, it is a little ugly, but necessary workaround to make WK2 tests work. r=me to go ahead with some typo fix. Is there any bug report for moving to QRawFont? > LayoutTests/ChangeLog:9 > + * platform/qt-wk2/Skipped: > + + Unskip canvas tests. > Tools/ChangeLog:8 > + Adding a workaround for this issue by mimicking the behaviour of DumpRenderTree. s/mimicking/mimicing Landed in http://trac.webkit.org/changeset/88435 Reopen, because this bug is appeared again after migrating from Qt 4.8 to Qt 5.0, because workaround doesn't work anymore. (In reply to comment #15) > Reopen, because this bug is appeared again after migrating from Qt 4.8 to Qt 5.0, because workaround doesn't work anymore. Actually we still have FontDatabase::removeAllApplicationFonts - maybe the Qt5 implementation is not sufficient, need to check - but it came out that we have a much more basic issue which is that we don't use the test fonts with Qt5. Tracked in https://bugs.webkit.org/show_bug.cgi?id=72513. Is this related to the work Pierre has being doing with raw fonts?! (In reply to comment #17) > Is this related to the work Pierre has being doing with raw fonts?! Not really. This bug has been solved long time ago thanks to Zoltan. The new failures was not a reappearing of that but another bug that has also been fixed. |