Bug 53427 - [Qt][WK2] Canvas tests make css2.1 ones failing
: [Qt][WK2] Canvas tests make css2.1 ones failing
Status: RESOLVED FIXED
: WebKit
WebKit2
: 528+ (Nightly build)
: PC All
: P3 Normal
Assigned To:
:
: LayoutTestFailure, Qt, QtTriaged
:
:
  Show dependency treegraph
 
Reported: 2011-01-31 08:48 PST by
Modified: 2012-02-03 08:28 PST (History)


Attachments
patch (3.74 KB, patch)
2011-06-08 23:58 PST, Zoltan Herczeg
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-01-31 08:48:46 PST
Pretty annoying bug. If one runs canvas tests than all of the css2.1 tests failing with WTR.
Without the canvas tests there is only 3 failure in css2.1. The whole story happening only on
the bot's 32 bit Debian environment. On the same machine in a 64 bit chroot there is no such
problem.
------- Comment #1 From 2011-02-01 04:44:24 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.
------- Comment #2 From 2011-02-01 05:00:20 PST -------
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.
------- Comment #3 From 2011-02-01 05:03:50 PST -------
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)
------- Comment #4 From 2011-05-27 06:14:53 PST -------
QApplication::fontMetrics().averageCharWidth() doubles during the running of the test. Something affects this member. Do you have any idea?
------- Comment #5 From 2011-05-27 06:24:35 PST -------
(Besides QApplication::setFont() which is not happen)
------- Comment #6 From 2011-05-30 05:36:25 PST -------
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.
------- Comment #7 From 2011-05-31 07:42:34 PST -------
Another big step:

Source/WebCore/html/HTMLCanvasElement.cpp
HTMLCanvasElement::~HTMLCanvasElement()

m_context.clear();

Can cause this. Tee conditions are not yet known.
------- Comment #8 From 2011-06-02 04:54:30 PST -------
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).
------- Comment #9 From 2011-06-02 04:57:09 PST -------
Zecke, you made this code before. Maybe you have some idea.
------- Comment #10 From 2011-06-02 06:32:42 PST -------
If I comment out the mentioned line, the tests are passed.

 FontCustomPlatformData::~FontCustomPlatformData()
 {
-    QFontDatabase::removeApplicationFont(m_handle);
+//    QFontDatabase::removeApplicationFont(m_handle);
 }
------- Comment #11 From 2011-06-08 03:59:35 PST -------
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.
------- Comment #12 From 2011-06-08 23:58:20 PST -------
Created an attachment (id=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 #13 From 2011-06-09 01:37:41 PST -------
(From update of attachment 96555 [details])
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
------- Comment #14 From 2011-06-09 01:49:13 PST -------
Landed in http://trac.webkit.org/changeset/88435
------- Comment #15 From 2011-11-11 05:31:57 PST -------
Reopen, because this bug is appeared again after migrating from Qt 4.8 to Qt 5.0, because workaround doesn't work anymore.
------- Comment #16 From 2011-11-16 09:15:25 PST -------
(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.
------- Comment #17 From 2012-02-03 07:16:04 PST -------
Is this related to the work Pierre has being doing with raw fonts?!
------- Comment #18 From 2012-02-03 08:28:56 PST -------
(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.