Bug 119790 - [Qt] Activate Page Visibility API layout tests
Summary: [Qt] Activate Page Visibility API layout tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 120418
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-14 03:19 PDT by Benjamin Dupont
Modified: 2013-08-28 07:27 PDT (History)
7 users (show)

See Also:


Attachments
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (3.39 KB, patch)
2013-08-14 03:26 PDT, Benjamin Dupont
allan.jensen: review+
bedupont: commit-queue-
Details | Formatted Diff | Diff
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (2) (3.39 KB, patch)
2013-08-14 03:56 PDT, Benjamin Dupont
no flags Details | Formatted Diff | Diff
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (3) (3.45 KB, patch)
2013-08-14 05:25 PDT, Benjamin Dupont
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (4) (3.47 KB, patch)
2013-08-14 06:14 PDT, Benjamin Dupont
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Dupont 2013-08-14 03:19:25 PDT
With https://bugs.webkit.org/show_bug.cgi?id=109422 we can activate visibility API layout tests.
Furthermore, void TestRunner::setPageVisibility(const char* visibility) method must be implemented.
Comment 1 Benjamin Dupont 2013-08-14 03:26:27 PDT
Created attachment 208711 [details]
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file.

Tests results:
fast/events/page-visibility-iframe-delete-test.html:
This test checks that the page visibility event propagation does not crash the browser when frames are added / deleted.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


Loaded all frames.
Visibility of main document changed.
Visibility of sub frame 2 changed.
PASS successfullyParsed is true

TEST COMPLETE


fast/events/page-visibility-iframe-move-test.html:
Content-Type: text/plain
This test checks that an iframe that moves between pages with different visibility will have the correct visibility value.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


Window 1 Loaded
Window 2 Loaded
PASS window.document.hidden is false
PASS window2.document.hidden is false
PASS iframe.contentDocument.hidden is false
PASS window.document.hidden is true
PASS window2.document.hidden is false
PASS iframe.contentDocument.hidden is false
Adopted iframe to Window 1
PASS window.document.hidden is true
PASS window2.document.hidden is false
PASS iframe.contentDocument.hidden is true
PASS successfullyParsed is true

TEST COMPLETE


fast/events/page-visibility-iframe-propagation-test.html:
This test checks that Page Visibility state events are propagated to child frames.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


Main Page:
PASS document.visibilityState is "visible"
PASS document.hidden is false
Child Frame:
PASS childFrame.contentDocument.visibilityState is "visible"
PASS childFrame.contentDocument.hidden is false
Main Page:
PASS document.visibilityState is "hidden"
PASS document.hidden is true
Child Frame:
PASS childFrame.contentDocument.visibilityState is "hidden"
PASS childFrame.contentDocument.hidden is true
Main Page:
PASS document.visibilityState is "visible"
PASS document.hidden is false
Child Frame:
PASS childFrame.contentDocument.visibilityState is "visible"
PASS childFrame.contentDocument.hidden is false
PASS successfullyParsed is true

TEST COMPLETE


fast/events/page-visibility-null-view.html:
This test checks that Page Visibility state values are correct when a document has no defaultView.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS subdocument.defaultView == null is true
PASS subdocument.visibilityState is "hidden"
PASS subdocument.hidden is true
PASS successfullyParsed is true

TEST COMPLETE


fast/events/page-visibility-transition-test.html:
This test checks that Page Visibility state values are correct and the event changes are fired correctly.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS document.visibilityState is "visible"
PASS document.hidden is false
PASS document.visibilityState is "hidden"
PASS document.hidden is true
PASS document.visibilityState is "hidden"
PASS document.hidden is true
PASS document.visibilityState is "prerender"
PASS document.hidden is true
PASS document.visibilityState is "visible"
PASS document.hidden is false
PASS document.visibilityState is "unloaded"
PASS document.hidden is true
PASS document.visibilityState is "visible"
PASS document.hidden is false
PASS successfullyParsed is true

TEST COMPLETE


fast/frames/page-visibility-crash.html:
PASS. WebKit didn't crash
Comment 2 Allan Sandfeld Jensen 2013-08-14 03:34:31 PDT
Comment on attachment 208711 [details]
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file.

LGTM
Comment 3 Benjamin Dupont 2013-08-14 03:47:37 PDT
Comment on attachment 208711 [details]
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file.

Must change QString...
Comment 4 Benjamin Dupont 2013-08-14 03:56:17 PDT
Created attachment 208713 [details]
 Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (2)

Use QLatin1String instead of QString.
Comment 5 Benjamin Dupont 2013-08-14 05:25:01 PDT
Created attachment 208717 [details]
 Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (3)

QStringLiteral is the fastest with C++11.
Comment 6 Early Warning System Bot 2013-08-14 05:30:39 PDT
Comment on attachment 208717 [details]
 Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (3)

Attachment 208717 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1451815
Comment 7 Early Warning System Bot 2013-08-14 05:31:26 PDT
Comment on attachment 208717 [details]
 Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (3)

Attachment 208717 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1458707
Comment 8 Benjamin Dupont 2013-08-14 06:14:45 PDT
Created attachment 208721 [details]
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (4)
Comment 9 WebKit Commit Bot 2013-08-14 07:28:14 PDT
Comment on attachment 208721 [details]
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (4)

Rejecting attachment 208721 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:

(1 failure)

Failed to run "['Tools/Scripts/build-webkit', '--release']" exit_code: 65
s.sb
make: *** wait: No child processes.  Stop.
make: *** Waiting for unfinished jobs....
make: *** wait: No child processes.  Stop.
Command /bin/sh failed with exit code 2


** BUILD FAILED **


The following build commands failed:
	PhaseScriptExecution "Generate Derived Sources" "/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit2.build/Release/Derived Sources.build/Script-C0CE72841247E66800BC0EC4.sh"
(1 failure)

Full output: http://webkit-queues.appspot.com/results/1451846
Comment 10 Allan Sandfeld Jensen 2013-08-14 07:40:37 PDT
Comment on attachment 208721 [details]
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (4)

Trying again, the bot looks sick
Comment 11 WebKit Commit Bot 2013-08-14 08:06:00 PDT
Comment on attachment 208721 [details]
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (4)

Rejecting attachment 208721 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
CE72841247E66800BC0EC4.sh"
(1 failure)

Failed to run "['Tools/Scripts/build-webkit', '--release']" exit_code: 65
ild processes.  Stop.
make: *** Waiting for unfinished jobs....
make: *** wait: No child processes.  Stop.
Command /bin/sh failed with exit code 2


** BUILD FAILED **


The following build commands failed:
	PhaseScriptExecution "Generate Derived Sources" "/Volumes/Data/EWS/WebKit/WebKitBuild/WebKit2.build/Release/Derived Sources.build/Script-C0CE72841247E66800BC0EC4.sh"
(1 failure)

Full output: http://webkit-queues.appspot.com/results/1462121
Comment 12 Allan Sandfeld Jensen 2013-08-14 09:50:52 PDT
Comment on attachment 208721 [details]
Implement the needed API and remove Page Visibility API test from Qt TestExpectations file. (4)

Clearing flags on attachment: 208721

Committed r154053: <http://trac.webkit.org/changeset/154053>
Comment 13 Allan Sandfeld Jensen 2013-08-14 09:50:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Allan Sandfeld Jensen 2013-08-15 04:44:02 PDT
The unskipped tests are flaky. Are you sure you don't need to reset the visibility?
Comment 15 Benjamin Dupont 2013-08-20 01:38:41 PDT
(In reply to comment #14)
> The unskipped tests are flaky. Are you sure you don't need to reset the visibility?
I don't understand why is it necessary to reset the visibility at the end of the test because it's set when the QWebPage is created. 
The problem with our Qt implementation is that we haven't the possibility to specify an initial state (Page API offer this possibility but not our QWebPage API). We don't provide this feature because we just need to initial the state when we are creating the Page.
In the DRT layout reset function, if we call the QWebPage setVisibilityState function with visible in parameter, then an event will be fired to the JS application and test results won't match with the expected file...

When I manually launched the unskipped tests one by one (PSA my first comment), I obtained the expected results.

Thus, if this is really the root cause, we can add a new QWebPage API to reset this visibility state but it will be used only for layout tests...
Comment 16 Jocelyn Turcotte 2013-08-20 03:16:12 PDT
(In reply to comment #15)
> (In reply to comment #14)
> > The unskipped tests are flaky. Are you sure you don't need to reset the visibility?
> I don't understand why is it necessary to reset the visibility at the end of the test because it's set when the QWebPage is created. 

This might not be always the case, but I think a single QWebPage is used for sequentially launched tests by DRT.
Comment 17 Benjamin Dupont 2013-08-20 06:51:17 PDT
(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > The unskipped tests are flaky. Are you sure you don't need to reset the visibility?
> > I don't understand why is it necessary to reset the visibility at the end of the test because it's set when the QWebPage is created. 
> 
> This might not be always the case, but I think a single QWebPage is used for sequentially launched tests by DRT.
Ok that's why there is a reset function...

In the DRT, is it possible to access directly to Page API?

If we can't, I'm not enthusiastic to add a resetVisibilityState QWebPage API just for layout tests... 

How should we proceed?