This test is sometimes failing with text difference: 2010-09-15 01:44:47,251 executive.py:301 DEBUG "/usr/bin/wdiff --start-delete=##WDIFF_DEL## --end-delete=##WDIFF_END## --start-insert=##WDIFF_ADD## --end-insert=##WDIFF_END## /b/slave/webkit-rel-linux/build/src/webkit/Release/../../../layout-test-results/fast/dom/DeviceOrientation/add-listener-from-callback-expected.txt /b/slave/webkit-rel-linux/build/src/webkit/Release/../../../layout-test-results/fast/dom/DeviceOrientation/add-listener-from-callback-actual.txt" took 0.01s 2010-09-15 01:44:47,252 dump_render_tree_thread.py:408 DEBUG Thread-2 fast/dom/DeviceOrientation/add-listener-from-callback.html failed: Text diff mismatch 2010-09-15 01:44:47,258 printing.py:535 INFO fast/dom/DeviceOrientation/add-listener-from-callback.html -> unexpected text diff mismatch (http://buildbot.jail.google.com/buildbot/chromium/builders/Webkit%20Linux/builds/29744/steps/webkit_tests/logs/stdio)
Created attachment 67677 [details] Patch
Comment on attachment 67677 [details] Patch > + There was a race in the add-listener-from-callback test: it used to do > + setMockOrientation() and then window.addEventListner(), but since the > + event caused by setMockOrientation() is fired asynchronously, it would > + sometimes be fired after the new event listener was added, causing the > + test to receive one more event than it expected. You're right that the test is flaky, but I think this reasoning is wrong. Both the calls to setMockDeviceOrientation() and addEventListener() will trigger events to fire asynchronously. The order is deterministic based on the the order in which the functions are called. The problem is that when we call addEventListener(), an event is sent to all registered event listeners on that page (whether this is the correct behaviour is questionable, but should be addressed elsewhere I think). So in the existing test, both the calls to setMockDeviceOrientation() and addEventListener() will trigger events to fire asynchronously on both listeners (both will be registered by the time the event fires), giving a total of five events. The race condition is whether the test finishes after the third event, before the fourth event fires. > if (++firstListenerEvents == 1) > setMockOrientation(11.1, 22.2, 33.3); I think I was wrong to update the mock orientation - it's extra complication and not the point of this test. > - else if (firstListenerEvents > 2) > + else if (firstListenerEvents == 2) > + window.addEventListener('deviceorientation', secondListener); I think the best fix is to just add the second listener from the first listener, and check that a total of three events fire. I already have a patch and will upload it.
Created attachment 67679 [details] Patch
(In reply to comment #3) > Created an attachment (id=67679) [details] > Patch Ah, this is much simpler. I'd r+ if I could :)
Comment on attachment 67679 [details] Patch r=me
Committed r67557: <http://trac.webkit.org/changeset/67557>