101208 2012-11-05 04:47:09 -0800 fast/events/popup-allowed-from-gesture-initiated-event.html is flaky 2012-11-05 06:08:31 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 cdumez cdumez abarth aestes d-r kenneth tmpsantos webkit.review.bot oldest_to_newest 758425 0 cdumez 2012-11-05 04:47:09 -0800 fast/events/popup-allowed-from-gesture-initiated-event.html is flaky. We sometimes get the following diff on EFL port: --- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/fast/events/popup-allowed-from-gesture-initiated-event-expected.txt +++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/fast/events/popup-allowed-from-gesture-initiated-event-actual.txt @@ -1,4 +1,3 @@ Click Here Click Here Too PASS win is non-null. -PASS win is non-null. 758429 1 172315 cdumez 2012-11-05 04:52:21 -0800 Created attachment 172315 Patch 758435 2 172315 kenneth 2012-11-05 05:09:17 -0800 Comment on attachment 172315 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172315&action=review > LayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.html:34 > + setTimeout(doneHandler, 1); why not just 0? > LayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.html:41 > + setTimeout(doneHandler, 1); So it can take a while to close? hmm, so we are just making it less flaky? 758457 3 172315 cdumez 2012-11-05 05:47:57 -0800 Comment on attachment 172315 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172315&action=review >> LayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.html:34 >> + setTimeout(doneHandler, 1); > > why not just 0? I honestly copied this from another test (fast/dom/Window/new-window-opener.html). 0 would certainly work but since the window takes a while to close apparently, I would actually do the opposite and put a bigger value here (like 250 milliseconds) to avoid invoking doneHandler() too many times. >> LayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.html:41 >> + setTimeout(doneHandler, 1); > > So it can take a while to close? hmm, so we are just making it less flaky? It completely resolves flakiness for me. I don't see why this would make it only "less" flaky. We are properly waiting for the window to close so no flakiness is possible here. 758461 4 172315 kenneth 2012-11-05 05:51:33 -0800 Comment on attachment 172315 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172315&action=review >>> LayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.html:41 >>> + setTimeout(doneHandler, 1); >> >> So it can take a while to close? hmm, so we are just making it less flaky? > > It completely resolves flakiness for me. I don't see why this would make it only "less" flaky. We are properly waiting for the window to close so no flakiness is possible here. Sure, I See that now. It keeps calling until it is closed. So what if it never closes? Should you have a recursion count? 758462 5 cdumez 2012-11-05 05:57:43 -0800 (In reply to comment #4) > (From update of attachment 172315 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=172315&action=review > > >>> LayoutTests/fast/events/popup-allowed-from-gesture-initiated-event.html:41 > >>> + setTimeout(doneHandler, 1); > >> > >> So it can take a while to close? hmm, so we are just making it less flaky? > > > > It completely resolves flakiness for me. I don't see why this would make it only "less" flaky. We are properly waiting for the window to close so no flakiness is possible here. > > Sure, I See that now. It keeps calling until it is closed. So what if it never closes? Should you have a recursion count? Other tests I've seen that wait for a window to close() don't seem to do anything specific regarding recursion. Actually, what will happen is that the test will time out (and therefore fail). 758466 6 172315 webkit.review.bot 2012-11-05 06:08:27 -0800 Comment on attachment 172315 Patch Clearing flags on attachment: 172315 Committed r133461: <http://trac.webkit.org/changeset/133461> 758467 7 webkit.review.bot 2012-11-05 06:08:31 -0800 All reviewed patches have been landed. Closing bug. 172315 2012-11-05 04:52:21 -0800 2012-11-05 06:08:25 -0800 Patch 101208_popup_flaky.patch text/plain 3933 cdumez ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA2NDRmMjNmLi4wNmU4NWVkIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTItMTEt MDUgIENocmlzdG9waGUgRHVtZXogIDxjaHJpc3RvcGhlLmR1bWV6QGludGVsLmNvbT4KKworICAg ICAgICBmYXN0L2V2ZW50cy9wb3B1cC1hbGxvd2VkLWZyb20tZ2VzdHVyZS1pbml0aWF0ZWQtZXZl bnQuaHRtbCBpcyBmbGFreQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MTAxMjA4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgQ2xvc2UgdGhlIG5ldyBwb3B1cCBiZXR3ZWVuIGVhY2ggdGVzdCB0byBtYWtlIHN1 cmUgdGhlIG5ldyB3aW5kb3cKKyAgICAgICAgZG9lcyBub3QgbWFzayB0aGUgYnV0dG9ucyBhbmQg dGhlcmVmb3JlIHByZXZlbnQgdGhlbSBmcm9tIGJlaW5nCisgICAgICAgIGNsaWNrZWQuIFRoaXMg YWRkcmVzc2VzIGZsYWtpbmVzcyBmb3IgdGhpcyB0ZXN0LgorCisgICAgICAgICogZmFzdC9ldmVu dHMvcG9wdXAtYWxsb3dlZC1mcm9tLWdlc3R1cmUtaW5pdGlhdGVkLWV2ZW50Lmh0bWw6CisKIDIw MTItMTEtMDUgIEtyaXN0w7NmIEtvc3p0ecOzICA8a2tyaXN0b2ZAaW5mLnUtc3plZ2VkLmh1Pgog CiAgICAgICAgIFtRdF0gVW5yZXZpZXdlZCBnYXJkZW5pbmcuCmRpZmYgLS1naXQgYS9MYXlvdXRU ZXN0cy9mYXN0L2V2ZW50cy9wb3B1cC1hbGxvd2VkLWZyb20tZ2VzdHVyZS1pbml0aWF0ZWQtZXZl bnQuaHRtbCBiL0xheW91dFRlc3RzL2Zhc3QvZXZlbnRzL3BvcHVwLWFsbG93ZWQtZnJvbS1nZXN0 dXJlLWluaXRpYXRlZC1ldmVudC5odG1sCmluZGV4IDAzMTI0MzcuLmU2YzgzZGUgMTAwNjQ0Ci0t LSBhL0xheW91dFRlc3RzL2Zhc3QvZXZlbnRzL3BvcHVwLWFsbG93ZWQtZnJvbS1nZXN0dXJlLWlu aXRpYXRlZC1ldmVudC5odG1sCisrKyBiL0xheW91dFRlc3RzL2Zhc3QvZXZlbnRzL3BvcHVwLWFs bG93ZWQtZnJvbS1nZXN0dXJlLWluaXRpYXRlZC1ldmVudC5odG1sCkBAIC03LDggKzcsMTAgQEAK ICAgICAgICAgPGJ1dHRvbiBpZD0iYnV0dG9uMiIgb25jbGljaz0idGVzdEJ1dHRvbjIoKSI+Q2xp Y2sgSGVyZSBUb288L2J1dHRvbj4KICAgICAgICAgPGJ1dHRvbiBpZD0idGVzdCIgb25jbGljaz0i cG9wdXAoKSIgc3R5bGU9ImRpc3BsYXk6bm9uZSI+PC9idXR0b24+CiAgICAgICAgIDxkaXYgaWQ9 ImNvbnNvbGUiPjwvZGl2PgotICAgICAgICA8c2NyaXB0PiAKKyAgICAgICAgPHNjcmlwdD4KKyAg ICAgICAgICAgIHZhciB0ZXN0TnVtID0gMDsKICAgICAgICAgICAgIHZhciB3aW47CisKICAgICAg ICAgICAgIGZ1bmN0aW9uIHBvcHVwKCkgewogICAgICAgICAgICAgICAgIHdpbiA9IHdpbmRvdy5v cGVuKCJhYm91dDpibGFuayIsICJibGFuayIpOwogICAgICAgICAgICAgICAgIHNob3VsZEJlTm9u TnVsbCgid2luIik7CkBAIC0yNCwyNiArMjYsNDggQEAKICAgICAgICAgICAgICAgICBkb2N1bWVu dC5nZXRFbGVtZW50QnlJZCgidGVzdCIpLmRpc3BhdGNoRXZlbnQoY2xpY2tFdmVudCk7CiAgICAg ICAgICAgICB9CiAKLSAgICAgICAgICAgIGlmICh3aW5kb3cudGVzdFJ1bm5lcikgewotICAgICAg ICAgICAgICAgIHRlc3RSdW5uZXIuZHVtcEFzVGV4dCgpOwotICAgICAgICAgICAgICAgIHRlc3RS dW5uZXIuc2V0Q2FuT3BlbldpbmRvd3MoKTsKLSAgICAgICAgICAgICAgICB0ZXN0UnVubmVyLnNl dFBvcHVwQmxvY2tpbmdFbmFibGVkKHRydWUpOwotICAgICAgICAgICAgICAgIHRlc3RSdW5uZXIu c2V0Q2xvc2VSZW1haW5pbmdXaW5kb3dzV2hlbkNvbXBsZXRlKHRydWUpOwotICAgICAgICAgICAg ICAgIHRlc3RSdW5uZXIud2FpdFVudGlsRG9uZSgpOworICAgICAgICAgICAgZnVuY3Rpb24gY2xv c2VXaW5kb3dBbmRSdW5OZXh0VGVzdCgpIHsKKyAgICAgICAgICAgICAgICBpZiAoIXdpbikKKyAg ICAgICAgICAgICAgICAgICAgbmV4dFRlc3QoKTsKIAotICAgICAgICAgICAgICAgIHZhciBidXR0 b24xID0gZG9jdW1lbnQuZ2V0RWxlbWVudEJ5SWQoImJ1dHRvbjEiKTsKLSAgICAgICAgICAgICAg ICB2YXIgYnV0dG9uMiA9IGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJidXR0b24yIik7CisgICAg ICAgICAgICAgICAgd2luLmNsb3NlKCk7CisgICAgICAgICAgICAgICAgc2V0VGltZW91dChkb25l SGFuZGxlciwgMSk7CisKKyAgICAgICAgICAgICAgICBmdW5jdGlvbiBkb25lSGFuZGxlcigpIHsK KyAgICAgICAgICAgICAgICAgICAgaWYgKHdpbi5jbG9zZWQpIHsKKyAgICAgICAgICAgICAgICAg ICAgICAgIHNldFRpbWVvdXQobmV4dFRlc3QsIDEpOworICAgICAgICAgICAgICAgICAgICAgICAg cmV0dXJuOworICAgICAgICAgICAgICAgICAgICB9CisgICAgICAgICAgICAgICAgICAgIHNldFRp bWVvdXQoZG9uZUhhbmRsZXIsIDEpOworICAgICAgICAgICAgICAgIH0KKyAgICAgICAgICAgIH0K IAotICAgICAgICAgICAgICAgIGlmICh3aW5kb3cuZXZlbnRTZW5kZXIpIHsKKyAgICAgICAgICAg IGZ1bmN0aW9uIG5leHRUZXN0KCkgeworICAgICAgICAgICAgICAgIGlmICh0ZXN0TnVtID09IDAp IHsKKyAgICAgICAgICAgICAgICAgICAgdmFyIGJ1dHRvbjEgPSBkb2N1bWVudC5nZXRFbGVtZW50 QnlJZCgiYnV0dG9uMSIpOwogICAgICAgICAgICAgICAgICAgICBldmVudFNlbmRlci5tb3VzZU1v dmVUbyhidXR0b24xLm9mZnNldExlZnQgKyBidXR0b24xLm9mZnNldFdpZHRoIC8gMiwgYnV0dG9u MS5vZmZzZXRUb3AgKyBidXR0b24xLm9mZnNldEhlaWdodCAvIDIpOwogICAgICAgICAgICAgICAg ICAgICBldmVudFNlbmRlci5tb3VzZURvd24oKTsKICAgICAgICAgICAgICAgICAgICAgZXZlbnRT ZW5kZXIubW91c2VVcCgpOworICAgICAgICAgICAgICAgIH0gZWxzZSBpZiAodGVzdE51bSA9PSAx KSB7CisgICAgICAgICAgICAgICAgICAgIHZhciBidXR0b24yID0gZG9jdW1lbnQuZ2V0RWxlbWVu dEJ5SWQoImJ1dHRvbjIiKTsKICAgICAgICAgICAgICAgICAgICAgZXZlbnRTZW5kZXIubW91c2VN b3ZlVG8oYnV0dG9uMi5vZmZzZXRMZWZ0ICsgYnV0dG9uMi5vZmZzZXRXaWR0aCAvIDIsIGJ1dHRv bjIub2Zmc2V0VG9wICsgYnV0dG9uMi5vZmZzZXRIZWlnaHQgLyAyKTsKICAgICAgICAgICAgICAg ICAgICAgZXZlbnRTZW5kZXIubW91c2VEb3duKCk7CiAgICAgICAgICAgICAgICAgICAgIGV2ZW50 U2VuZGVyLm1vdXNlVXAoKTsKKyAgICAgICAgICAgICAgICB9IGVsc2UgeworICAgICAgICAgICAg ICAgICAgICB0ZXN0UnVubmVyLm5vdGlmeURvbmUoKTsKICAgICAgICAgICAgICAgICB9CisgICAg ICAgICAgICAgICAgKyt0ZXN0TnVtOworICAgICAgICAgICAgICAgIGNsb3NlV2luZG93QW5kUnVu TmV4dFRlc3QoKTsKKyAgICAgICAgICAgIH0KKworICAgICAgICAgICAgaWYgKHdpbmRvdy50ZXN0 UnVubmVyKSB7CisgICAgICAgICAgICAgICAgdGVzdFJ1bm5lci5kdW1wQXNUZXh0KCk7CisgICAg ICAgICAgICAgICAgdGVzdFJ1bm5lci5zZXRDYW5PcGVuV2luZG93cygpOworICAgICAgICAgICAg ICAgIHRlc3RSdW5uZXIuc2V0UG9wdXBCbG9ja2luZ0VuYWJsZWQodHJ1ZSk7CisgICAgICAgICAg ICAgICAgdGVzdFJ1bm5lci5zZXRDbG9zZVJlbWFpbmluZ1dpbmRvd3NXaGVuQ29tcGxldGUodHJ1 ZSk7CisgICAgICAgICAgICAgICAgdGVzdFJ1bm5lci53YWl0VW50aWxEb25lKCk7CiAKLSAgICAg ICAgICAgICAgICB0ZXN0UnVubmVyLm5vdGlmeURvbmUoKTsKKyAgICAgICAgICAgICAgICBuZXh0 VGVzdCgpOwogICAgICAgICAgICAgfQogICAgICAgICA8L3NjcmlwdD4gCiAgICAgPC9ib2R5PiAK