WebKitTestRunner may still crash in the scenario where a marker event is dequeued and handled after uiScriptComplete has been called. While this scenario should never happen when running iOS interaction tests serially on a single simulator, this seems to happen occasionally when tests are run in parallel. We can fix this by making UI script execution smart enough to defer script completion until all events (currently tap, double tap and hardware keyboard) have been handled.
<rdar://problem/23428601>
Created attachment 265200 [details] Patch
Comment on attachment 265200 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265200&action=review > Tools/WebKitTestRunner/UIScriptContext/Bindings/UIScriptController.idl:53 > - void uiScriptComplete(DOMString result); > + void uiScriptCompleteAfterHandlingEvents(DOMString result); This probably could have been an implementation detail of uiScriptComplete instead of requiring a rename. We don't list all the other things that happen between calling this and actually being /done/. > Tools/WebKitTestRunner/UIScriptContext/UIScriptContext.h:53 > unsigned prepareForAsyncTask(JSValueRef taskCallback); > + unsigned prepareForAsyncTaskBlockingScriptCompletion(JSValueRef taskCallback); Do we ever want async tasks *not* to block uiScriptComplete? Is it ever bad if they do? It would be nice to only have one that always does the right thing, to make it harder to write a broken test (and not change the name like above).
Comment on attachment 265200 [details] Patch Attachment 265200 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/411197 New failing tests: fast/harness/concurrent-ui-side-scripts.html fast/harness/ui-side-scripts.html
Created attachment 265208 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 265272 [details] Patch
Comment on attachment 265272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265272&action=review > Tools/WebKitTestRunner/UIScriptContext/UIScriptContext.h:49 > +const unsigned minimumPersistentCallbackID = 1000; > +typedef enum { > + CallbackTypeNonPersistent, > + CallbackTypeWillBeginZooming = minimumPersistentCallbackID, > + CallbackTypeDidEndZooming = minimumPersistentCallbackID + 1, > + CallbackTypeDidShowKeyboard = minimumPersistentCallbackID + 2, > + CallbackTypeDidHideKeyboard = minimumPersistentCallbackID + 3 > +} CallbackType; I think it's better to reserve the low range for persistent callbacks; that way, the upper bound for other callbacks is much higher.
Comment on attachment 265272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265272&action=review >> Tools/WebKitTestRunner/UIScriptContext/UIScriptContext.h:49 >> +} CallbackType; > > I think it's better to reserve the low range for persistent callbacks; that way, the upper bound for other callbacks is much higher. That makes sense -- changed to use minimumNonPersistentCallbackID.
Created attachment 265286 [details] Patch
Comment on attachment 265286 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=265286&action=review > Tools/WebKitTestRunner/UIScriptContext/UIScriptContext.cpp:158 > + m_uiScriptResultsPendingCompletion.add(m_currentScriptCallbackID, result ? JSStringRetain(result) : result); : nullptr I think. > Tools/WebKitTestRunner/UIScriptContext/UIScriptContext.cpp:188 > + for (WTF::KeyValuePair<unsigned, Task> entry : m_callbacks) { auto > Tools/WebKitTestRunner/UIScriptContext/UIScriptContext.h:42 > +const unsigned minimumNonPersistentCallbackID = 1000; How about firstNonPersistentCallbackID and a blank line after this.
Committed r192314: <http://trac.webkit.org/changeset/192314>