Bug 53427 – [Qt][WK2] Canvas tests make css2.1 ones failing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Balazs Kelemen]

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.

[Balazs Kelemen]

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.

[Balazs Kelemen]

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.

[Balazs Kelemen]

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)

[Zoltan Herczeg]

QApplication::fontMetrics().averageCharWidth() doubles during the running of the test. Something affects this member. Do you have any idea?

[Zoltan Herczeg]

(Besides QApplication::setFont() which is not happen)

[Zoltan Herczeg]

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.

[Zoltan Herczeg]

Another big step:

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

m_context.clear();

Can cause this. Tee conditions are not yet known.

[Zoltan Herczeg]

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).

[Zoltan Herczeg]

Zecke, you made this code before. Maybe you have some idea.

[Zoltan Herczeg]

If I comment out the mentioned line, the tests are passed.

 FontCustomPlatformData::~FontCustomPlatformData()
 {
-    QFontDatabase::removeApplicationFont(m_handle);
+//    QFontDatabase::removeApplicationFont(m_handle);
 }

[Zoltan Herczeg]

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.

[Zoltan Herczeg]

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.

[Csaba Osztrogonác]

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

[Zoltan Herczeg]

Landed in http://trac.webkit.org/changeset/88435

[Csaba Osztrogonác]

Reopen, because this bug is appeared again after migrating from Qt 4.8 to Qt 5.0, because workaround doesn't work anymore.

[Balazs Kelemen]

(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.

[Jesus Sanchez-Palencia]

Is this related to the work Pierre has being doing with raw fonts?!

[Balazs Kelemen]

(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.