Bug 54641 - [Qt] ImageDiff sometimes hangs
Summary: [Qt] ImageDiff sometimes hangs
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Csaba Osztrogonác
Keywords: Qt, QtTriaged
Depends on:
Reported: 2011-02-17 05:39 PST by Csaba Osztrogonác
Modified: 2011-03-28 06:44 PDT (History)
5 users (show)

See Also:

proposed fix (978 bytes, patch)
2011-02-17 08:47 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2011-02-17 05:39:59 PST
On r78808 it is easily reproducable: (Using platform mac to get many pixel differences)
$ Tools/Scripts/run-webkit-tests --debug --platform mac -p svg/W3C-SVG-1.1-SE/coords-dom-01-f.svg svg/W3C-SVG-1.1-SE/coords-dom-02-f.svg svg/W3C-SVG-1.1-SE/coords-dom-03-f.svg svg/W3C-SVG-1.1-SE/coords-dom-04-f.svg svg/W3C-SVG-1.1-SE/filters-image-03-f.svg svg/W3C-SVG-1.1-SE/paths-dom-02-f.svg

During the 5. test ImageDiff hangs:
Tools/DumpRenderTree/qt/ImageDiff.cpp:138: if (fwrite(data.constData(), 1, data.length(), stdout)) {}

I'm going to debug and fix this bug. It is the first step to run Qt pixel tests. ;)
Comment 1 Csaba Osztrogonác 2011-02-17 08:47:11 PST
Created attachment 82816 [details]
proposed fix
Comment 2 Csaba Osztrogonác 2011-02-17 08:57:43 PST
ImageDiff was written by Ariya and reviewed by Simon: https://bugs.webkit.org/show_bug.cgi?id=27813

ImageDiff is able to compare pairs of images.
actualImage and baselineImage should be cleared after all comparison:
+            actualImage = QImage();
+            baselineImage = QImage();
Comment 3 Andreas Kling 2011-02-17 08:59:08 PST
Comment on attachment 82816 [details]
proposed fix

View in context: https://bugs.webkit.org/attachment.cgi?id=82816&action=review

> Tools/DumpRenderTree/qt/ImageDiff.cpp:146
> +            actualImage = QImage();
> +            baselineImage = QImage();

This should be below the next } so they get cleared if one of them isNull(), too.
Comment 4 Andreas Kling 2011-02-17 09:03:36 PST
Comment on attachment 82816 [details]
proposed fix

Superficial review fail, didn't realize this was an FSM. r=me, motherlover :)
Comment 5 WebKit Commit Bot 2011-02-17 09:40:04 PST
The commit-queue encountered the following flaky tests while processing attachment 82816 [details]:

media/invalid-media-url-crash.html bug 51138 (author: inferno@chromium.org)
The commit-queue is continuing to process your patch.
Comment 6 WebKit Commit Bot 2011-02-17 09:41:47 PST
Comment on attachment 82816 [details]
proposed fix

Clearing flags on attachment: 82816

Committed r78840: <http://trac.webkit.org/changeset/78840>
Comment 7 WebKit Commit Bot 2011-02-17 09:41:52 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Caio Marcelo de Oliveira Filho 2011-03-28 06:39:23 PDT
Revision r78840 cherry-picked into qtwebkit-2.1 with commit 3d8aeee68ff7943cc50e4b321b1071195b671d0a <http://gitorious.org/webkit/qtwebkit/commit/3d8aeee68ff7943cc50e4b321b1071195b671d0a>
Comment 9 Caio Marcelo de Oliveira Filho 2011-03-28 06:43:54 PDT
Also merged into qtwebkit-2.1.x