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.
Created attachment 172315 [details] Patch
Comment on attachment 172315 [details] 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?
Comment on attachment 172315 [details] 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.
Comment on attachment 172315 [details] 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?
(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).
Comment on attachment 172315 [details] Patch Clearing flags on attachment: 172315 Committed r133461: <http://trac.webkit.org/changeset/133461>
All reviewed patches have been landed. Closing bug.