RESOLVED FIXED 96480
REGRESSION(r128270): It made fast/events/popup-blocking-timers.html flakey 
https://bugs.webkit.org/show_bug.cgi?id=96480
Summary REGRESSION(r128270): It made fast/events/popup-blocking-timers.html flakey
Csaba Osztrogonác
Reported 2012-09-12 01:45:34 PDT
r128270 -------- --- /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/layout-test-results/fast/events/popup-blocking-timers-expected.txt +++ /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/layout-test-results/fast/events/popup-blocking-timers-actual.txt @@ -1,16 +1,2 @@ -Click Here (6 times) -Test calling window.open() directly. A popup should be allowed. -PASS newWindow is non-null. -Test calling window.open() with a 0 ms delay. A popup should be allowed. -PASS newWindow is non-null. -Test calling window.open() in a 100 ms interval. A popup should only be allowed on the first execution of the interval. -PASS newWindow is non-null. -Test calling window.open() in a 100 ms interval. A popup should only be allowed on the first execution of the interval. -PASS newWindow is undefined. -Test calling window.open() in a nested call to setTimeout(). A popup should not be allowed. -PASS newWindow is undefined. -Test calling window.open() with a 1000 ms delay. A popup should be allowed. -PASS newWindow is non-null. -Test calling window.open() with a 1001 ms delay. A popup should not be allowed. -PASS newWindow is undefined. +FAIL: Timed out waiting for notifyDone to be called r128275 -------- --- /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/layout-test-results/fast/events/popup-blocking-timers-expected.txt +++ /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/layout-test-results/fast/events/popup-blocking-timers-actual.txt @@ -5,12 +5,12 @@ PASS newWindow is non-null. Test calling window.open() in a 100 ms interval. A popup should only be allowed on the first execution of the interval. PASS newWindow is non-null. +Test calling window.open() with a 1000 ms delay. A popup should be allowed. +PASS newWindow is non-null. +Test calling window.open() in a nested call to setTimeout(). A popup should not be allowed. +PASS newWindow is undefined. Test calling window.open() in a 100 ms interval. A popup should only be allowed on the first execution of the interval. PASS newWindow is undefined. -Test calling window.open() in a nested call to setTimeout(). A popup should not be allowed. -PASS newWindow is undefined. -Test calling window.open() with a 1000 ms delay. A popup should be allowed. -PASS newWindow is non-null. Test calling window.open() with a 1001 ms delay. A popup should not be allowed. PASS newWindow is undefined.
Attachments
Patch (24.17 KB, patch)
2012-10-02 07:12 PDT, jochen 		no flags
DetailsFormatted DiffDiff
Patch for landing (24.28 KB, patch)
2012-10-02 11:50 PDT, jochen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2012-09-12 03:41:18 PDT
I skipped it - https://trac.webkit.org/changeset/128288 Please unskip it with the proper fix.
jochen
Comment 2 2012-09-17 04:42:21 PDT
(In reply to comment #1) > I skipped it - https://trac.webkit.org/changeset/128288 > Please unskip it with the proper fix. I checked the chromium test logs, and the test used to be flaky before (esp. this case when the ordering of events changes). I could imagine that it is less flaky, if we split it up in six individual tests, wdyt?
Adam Barth
Comment 3 2012-09-17 11:23:44 PDT
Sure, we can give that a try.
jochen
Comment 4 2012-10-02 07:09:46 PDT
*** Bug 96517 has been marked as a duplicate of this bug. ***
jochen
Comment 5 2012-10-02 07:12:05 PDT
Created attachment 166686 [details] Patch
Daniel Bates
Comment 6 2012-10-02 09:16:13 PDT
Comment on attachment 166686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=166686&action=review Looks sane to me. > LayoutTests/fast/events/popup-blocking-timers1.html:31 > + function clickButton() { > + var button = document.getElementById("test"); > + var buttonX = button.offsetLeft + button.offsetWidth / 2; > + var buttonY = button.offsetTop + button.offsetHeight / 2; > + if (window.eventSender) { > + eventSender.mouseMoveTo(buttonX, buttonY); > + eventSender.mouseDown(); > + eventSender.mouseUp(); > + } > + } I know that you derived this function from the function of the same name in the original test. I take it that it's necessary to use EventSender to actually simulate a mouse click of a person as opposed to calling click() on the HTML button element? > LayoutTests/fast/events/popup-blocking-timers2.html:1 > +<head> Unless you intend to test quirks mode, I suggest that we include a DOCTYPE, say <!DOCTYPE html>. > LayoutTests/fast/events/popup-blocking-timers2.html:33 > + function clickButton() { > + var button = document.getElementById("test"); > + var buttonX = button.offsetLeft + button.offsetWidth / 2; > + var buttonY = button.offsetTop + button.offsetHeight / 2; > + if (window.eventSender) { > + eventSender.mouseMoveTo(buttonX, buttonY); > + eventSender.mouseDown(); > + eventSender.mouseUp(); > + } > + } Ditto. > LayoutTests/fast/events/popup-blocking-timers3.html:41 > + function clickButton() { > + var button = document.getElementById("test"); > + var buttonX = button.offsetLeft + button.offsetWidth / 2; > + var buttonY = button.offsetTop + button.offsetHeight / 2; > + if (window.eventSender) { > + eventSender.mouseMoveTo(buttonX, buttonY); > + eventSender.mouseDown(); > + eventSender.mouseUp(); > + } > + } Ditto. > LayoutTests/fast/events/popup-blocking-timers4.html:35 > + function clickButton() { > + var button = document.getElementById("test"); > + var buttonX = button.offsetLeft + button.offsetWidth / 2; > + var buttonY = button.offsetTop + button.offsetHeight / 2; > + if (window.eventSender) { > + eventSender.mouseMoveTo(buttonX, buttonY); > + eventSender.mouseDown(); > + eventSender.mouseUp(); > + } > + } Ditto. > LayoutTests/fast/events/popup-blocking-timers5.html:36 > + function clickButton() { > + var button = document.getElementById("test"); > + var buttonX = button.offsetLeft + button.offsetWidth / 2; > + var buttonY = button.offsetTop + button.offsetHeight / 2; > + if (window.eventSender) { > + eventSender.mouseMoveTo(buttonX, buttonY); > + eventSender.mouseDown(); > + eventSender.mouseUp(); > + } > + } Ditto. > LayoutTests/fast/events/popup-blocking-timers6.html:36 > + function clickButton() { > + var button = document.getElementById("test"); > + var buttonX = button.offsetLeft + button.offsetWidth / 2; > + var buttonY = button.offsetTop + button.offsetHeight / 2; > + if (window.eventSender) { > + eventSender.mouseMoveTo(buttonX, buttonY); > + eventSender.mouseDown(); > + eventSender.mouseUp(); > + } > + } Ditto.
Daniel Bates
Comment 7 2012-10-02 09:18:51 PDT
(In reply to comment #6) > > LayoutTests/fast/events/popup-blocking-timers2.html:1 > > +<head> > > Unless you intend to test quirks mode, I suggest that we include a DOCTYPE, say <!DOCTYPE html>. Feel free to disregard this comment as the original test omitted a <!DOCTYPE> declaration and I don't anticipate that the result of this test will change in quirks mode.
jochen
Comment 8 2012-10-02 11:50:43 PDT
Created attachment 166724 [details] Patch for landing
jochen
Comment 9 2012-10-02 11:51:39 PDT
(In reply to comment #6) > (From update of attachment 166686 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166686&action=review > > Looks sane to me. > > > LayoutTests/fast/events/popup-blocking-timers1.html:31 > > + function clickButton() { > > + var button = document.getElementById("test"); > > + var buttonX = button.offsetLeft + button.offsetWidth / 2; > > + var buttonY = button.offsetTop + button.offsetHeight / 2; > > + if (window.eventSender) { > > + eventSender.mouseMoveTo(buttonX, buttonY); > > + eventSender.mouseDown(); > > + eventSender.mouseUp(); > > + } > > + } > > I know that you derived this function from the function of the same name in the original test. I take it that it's necessary to use EventSender to actually simulate a mouse click of a person as opposed to calling click() on the HTML button element? The call to click() would not be counted as a user action, and therefore no popups would be allowed at all > > > LayoutTests/fast/events/popup-blocking-timers2.html:1 > > +<head> > > Unless you intend to test quirks mode, I suggest that we include a DOCTYPE, say <!DOCTYPE html>. done
WebKit Review Bot
Comment 10 2012-10-02 13:10:59 PDT
Comment on attachment 166724 [details] Patch for landing Clearing flags on attachment: 166724 Committed r130200: <http://trac.webkit.org/changeset/130200>
WebKit Review Bot
Comment 11 2012-10-02 13:11:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.