Bug 34258

Summary: [Qt] Implement the display() method of the layout test controller
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, benjamin, commit-queue, kenneth, ossy
Priority: P2 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Implement the method
none
Implement the method
none
Move the window offscreen kenneth: review+, kenneth: commit-queue+

Description Benjamin Poulain 2010-01-28 06:12:55 PST
Currently, LayoutTestController::display() is not implemented in the Qt port. This make it impossible to tests problems related to painting.
Comment 1 Benjamin Poulain 2010-01-28 06:16:35 PST
Created attachment 47611 [details]
Implement the method

With this patch, the web view is shown.
Calling display multiple time is not a problem, call QWidget::show() again is just ignored.
Comment 2 WebKit Commit Bot 2010-01-28 06:41:11 PST
Comment on attachment 47611 [details]
Implement the method

Clearing flags on attachment: 47611

Committed r54000: <http://trac.webkit.org/changeset/54000>
Comment 3 WebKit Commit Bot 2010-01-28 06:41:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Csaba Osztrogon√°c 2010-01-28 08:45:16 PST
(In reply to comment #3)
> All reviewed patches have been landed.  Closing bug.

Rolled out by http://trac.webkit.org/changeset/54002, because it broke buildbot:
http://build.webkit.org/results/Qt%20Linux%20Release/r54000%20%286648%29/results.html
Comment 5 Benjamin Poulain 2010-01-29 02:13:07 PST
Created attachment 47691 [details]
Implement the method

Some tests are unstable regarding events. It is not safe to process all events in LayoutTestController::reset().

Since we still need a synchronous paint, I have added painting on a pixmap when display() is called.
Comment 6 Kenneth Rohde Christiansen 2010-01-29 03:49:09 PST
Comment on attachment 47691 [details]
Implement the method

Sounsd like an OK idea
Comment 7 WebKit Commit Bot 2010-01-29 04:17:16 PST
Comment on attachment 47691 [details]
Implement the method

Clearing flags on attachment: 47691

Committed r54054: <http://trac.webkit.org/changeset/54054>
Comment 8 WebKit Commit Bot 2010-01-29 04:17:22 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Andras Becsi 2010-01-29 16:41:07 PST
(In reply to comment #6)
> (From update of attachment 47691 [details])
> Sounsd like an OK ide

This patch has introduced an annoying effect of flickering pop-up frames and shown webviews when running the test suit.
This is a regression as I see it, because the 12000 Mac tests do not show any of this behaviour. Is there a way to fix this in Qt? Kenneth, what do you think?
Comment 10 Kenneth Rohde Christiansen 2010-01-30 07:55:49 PST
Benjamin implemented this for forcing paints, and we actually dont need to show the window for doing that, now we paint to a pixmap, so that could be changed.

If the behaviour is different than the other platforms we need to investigate why.
Comment 11 Benjamin Poulain 2010-01-30 08:23:27 PST
(In reply to comment #10)
> Benjamin implemented this for forcing paints, and we actually dont need to show
> the window for doing that, now we paint to a pixmap, so that could be changed.
> 
> If the behaviour is different than the other platforms we need to investigate
> why.

Showing the window is the only way I found to catch some bugs.

(In reply to comment #9)
> This is a regression as I see it, because the 12000 Mac tests do not show any
> of this behaviour. Is there a way to fix this in Qt? Kenneth, what do you
> think?

On Mac the window is shown off screen. I don't know if that works on all platform(?).
Comment 12 Benjamin Poulain 2010-01-30 10:31:58 PST
Created attachment 47770 [details]
Move the window offscreen

Here is a patch to move the window offscreen.
It should work on X11, I don't know on the other systems (some window manager might reset the position on screen).

Changes:
+ don't show the window on screen
+ still works with plugins and drag-and-drop
- never get any expose event

Note that on X11, without this patch, you can simply run a second X11 server to run the tests.
Comment 13 Kenneth Rohde Christiansen 2010-01-30 15:19:58 PST
Comment on attachment 47770 [details]
Move the window offscreen

Fine with me
Comment 14 Andras Becsi 2010-01-30 15:38:40 PST
(In reply to comment #12)
> Created an attachment (id=47770) [details]
> Move the window offscreen
Thank you for the fast fix.