Bug 31591 – [Qt] Crashing tests after updating to Qt-4.6.0

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Csaba Osztrogonác]

After we updated to Qt-4.6.0-stable/Qt-4.6.0-RC1 there are
24 crashing tests on QtBuildBot. These crashes caused by
previously tests.

I think "crasher" test leave DumpRenderTree in "bad state",
which cause the crash in one of the following test.

Until we find and fix DRT, we should put "crasher" tests
into skiplist to make our QtBuildBot happier.

There are 3 tests which are "crasher" and "crashed" tests
at the same time. If we put the original "crasher" to skiplist,
the original "crashed" test turn "crasher". Putting both of
them solve the problem.

[Csaba Osztrogonác]

Created attachment 43368 [details]
proposed patch

I propose, we should put problematic tests into skiplist temporarily. (until fix)

[Csaba Osztrogonác]

We can simple reproduce the crash:
WebKitTools/Scripts/run-webkit-tests CRASHER CRASHED

[Csaba Osztrogonác]

(In reply to comment #1)
> Created an attachment (id=43368) [details]
> proposed patch
Sending        LayoutTests/ChangeLog
Sending        LayoutTests/platform/qt/Skipped
Transmitting file data ..
Committed revision 51078.

[Simon Hausmann]

Closing as it seems the patch has been landed :)

[Csaba Osztrogonác]

Only a small patch landed to make QtBuildbot happy.
Bugfix is coming soon ...

[Andras Becsi]

Created attachment 44144 [details]
proposed fix

Refactor DRT to not crash on these tests, and re-enable them in the Skipped list.

[Andras Becsi]

(In reply to comment #6)
> Created an attachment (id=44144) [details]
> proposed fix
> 
> Refactor DRT to not crash on these tests, and re-enable them in the Skipped
> list.

This reveales some other tests, which have a notifyDone() problem described in:
https://bugs.webkit.org/show_bug.cgi?id=31626

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44144 [details] without any errors.

[Andras Becsi]

Created attachment 44147 [details]
patch v2

Change deleteLater() to delete in DumpRenderTree::closeRemainingWindows(), cause deleteLater() seems to cause some flakyness in DRT and sometimes some http tests fail.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44147 [details] without any errors.

[Csaba Osztrogonác]

Comment on attachment 44147 [details]
patch v2

cq- -ed, because we would like to land manually.

[Kenneth Rohde Christiansen]

Comment on attachment 44147 [details]
patch v2

r- for comments given on irc

Mostly style issues like:

QWebPage* page =static_cast<QWebPage *>(new WebPage(container, this));

which should be

QWebPage* page = static_cast<QWebPage*>(new WebPage(container, this));

etc

[Andras Becsi]

Created attachment 44230 [details]
patch v3

Style corrections made.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44230 [details] without any errors.

[Kenneth Rohde Christiansen]

Comment on attachment 44230 [details]
patch v3


> +    /*
> +    * Create a dummy container object to track the page in DRT.
> +    * QObject is used instead of QWidget to prevent DRT from
> +    * showing the main view when deleting the container.
> +    */

Please use // comment style inside code. That is our style

> +    QObject* container = new QObject(m_mainView);
> +    //create a QWebPage we want to return

Missing space between // and create

> +    QWebPage* page =static_cast<QWebPage*>(new WebPage(container, this));

Missing space after =

> +    //gets cleaned up in closeRemainingWindows()
>      windows.append(container);

same thing

> +
> +    //connect the needed signals to the page

same thing

> +    connect(page, SIGNAL(frameCreated(QWebFrame*)),
> +            this, SLOT(connectFrame(QWebFrame*)));

Make this a one liner


> +    connect(page, SIGNAL(loadFinished(bool)),
> +            m_controller, SLOT(maybeDump(bool)));

same as above

> +// Also count the main view

change to // Include the main view in the count

> +    return windows.count() + 1;
>  }

[Andras Becsi]

Created attachment 44233 [details]
another style update

Builder bots are red so cq-

[WebKit Review Bot]

Attachment 44233 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKitTools/DumpRenderTree/qt/DumpRenderTree.cpp:721:  Extra space before ( in function call  [whitespace/parens] [4]
WebKitTools/DumpRenderTree/qt/DumpRenderTree.cpp:723:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2

[Andras Becsi]

(In reply to comment #17)
> Attachment 44233 [details] did not pass style-queue:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> WebKitTools/DumpRenderTree/qt/DumpRenderTree.cpp:721:  Extra space before ( in
> function call  [whitespace/parens] [4]
> WebKitTools/DumpRenderTree/qt/DumpRenderTree.cpp:723:  Extra space before ( in
> function call  [whitespace/parens] [4]
> Total errors found: 2

We'll fix that before landing.

[Csaba Osztrogonác]

>Extra space before ( in function call  [whitespace/parens] [4]
removed before landing as discussed with Kenneth on #qtwebkit.

Sending        LayoutTests/ChangeLog
Sending        LayoutTests/platform/qt/Skipped
Sending        WebKitTools/ChangeLog
Sending        WebKitTools/DumpRenderTree/qt/DumpRenderTree.cpp
Sending        WebKitTools/DumpRenderTree/qt/DumpRenderTree.h
Transmitting file data .....
Committed revision 51634.