Web Automation: All key events should be processed before sending response

[Joseph Pecoraro]

Summary:
All key events should be processed before sending response.

For an automation test that does something like:

    element = self.driver.find_element_by_id("emptyTextArea")
    self.assertEqual("", element.get_attribute("value"))
    element.send_keys("hello world")
    self.assertEqual("hello world", element.get_attribute("value"))

Logging shows we send NSEvents for all the keyboard actions, but because WebPageProxy queues them, a later Automation event (evaluateJavaScriptFunction) happens before the keys have finished sending.

    Safari: (WebKit) >>> WebAutomationSession::performKeyboardInteractions - start
    Safari: (WebKit) >>> WebAutomationSession::performKeyboardInteractions - sending
    Safari: (WebKit) >>> KEY DOWN: NSEvent: type=KeyDown loc=(0,600) time=490938962.2 flags=0 win=0x0 winNum=13545 ctxt=0x0 chars="h" unmodchars="h" repeat=0 keyCode=0
    Safari: (WebKit) >>> WebPageProxy::handleKeyboardEvent - 1
    Safari: (WebKit) >>> WebPageProxy::handleKeyboardEvent sending
    Safari: (WebKit) >>> KEY UP: NSEvent: type=KeyUp loc=(0,600) time=490938962.2 flags=0 win=0x0 winNum=13545 ctxt=0x0 chars="h" unmodchars="h" repeat=0 keyCode=0
    Safari: (WebKit) >>> WebPageProxy::handleKeyboardEvent - 2
    Safari: (WebKit) >>> KEY DOWN: NSEvent: type=KeyDown loc=(0,600) time=490938962.2 flags=0 win=0x0 winNum=13545 ctxt=0x0 chars="e" unmodchars="e" repeat=0 keyCode=0
    Safari: (WebKit) >>> WebPageProxy::handleKeyboardEvent - 3
    Safari: (WebKit) >>> KEY UP: NSEvent: type=KeyUp loc=(0,600) time=490938962.2 flags=0 win=0x0 winNum=13545 ctxt=0x0 chars="e" unmodchars="e" repeat=0 keyCode=0
    Safari: (WebKit) >>> WebPageProxy::handleKeyboardEvent - 4
    ...
    Safari: (WebKit) >>> KEY DOWN: NSEvent: type=KeyDown loc=(0,600) time=490938962.2 flags=0 win=0x0 winNum=13545 ctxt=0x0 chars="d" unmodchars="d" repeat=0 keyCode=0
    Safari: (WebKit) >>> WebPageProxy::handleKeyboardEvent - 21
    Safari: (WebKit) >>> KEY UP: NSEvent: type=KeyUp loc=(0,600) time=490938962.2 flags=0 win=0x0 winNum=13545 ctxt=0x0 chars="d" unmodchars="d" repeat=0 keyCode=0
    Safari: (WebKit) >>> WebPageProxy::handleKeyboardEvent - 22
    Safari: (WebKit) >>> WebAutomationSession::performKeyboardInteractions - done
    Safari: (WebKit) >>> WebPageProxy::didReceiveEvent sending
    Safari: (WebKit) >>> WebAutomationSession::evaluateJavaScriptFunction - start
    Safari: (WebKit) >>> WebPageProxy::didReceiveEvent sending
    Safari: (WebKit) >>> WebPageProxy::didReceiveEvent sending

[Joseph Pecoraro]

<rdar://problem/27505943>

[Joseph Pecoraro]

Created attachment 284402 [details]
[PATCH] Proposed Fix

[WebKit Commit Bot]

Attachment 284402 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/Automation/WebAutomationSession.h:112:  The parameter name "callback" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 284402 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=284402&action=review

> Source/WebKit2/UIProcess/Automation/WebAutomationSession.cpp:1006
> +    if (auto callback = m_pendingKeyboardEventsFlushedCallbacksPerPage.take(page->pageID()))
> +        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout));
> +    m_pendingKeyboardEventsFlushedCallbacksPerPage.set(page->pageID(), WTFMove(callback));

This does multiple hash table lookups, and also, set is less efficient than add. The efficient way to code this is to call add with nullptr, like this:

    auto& callbackInMap = m_pendingKeyboardEventsFlushedCallbacksPerPage.add(page->pageID(), nullptr).iterator->value;
    if (callbackInMap)
        callbackInMap->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout))
    callbackInMap = WTFMove(callback);

[BJ Burg]

Comment on attachment 284402 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=284402&action=review

LGTM but please check EWS oranges.

> Source/WebKit2/ChangeLog:16
> +        notify it is complete only after WebPageProxy, notifies it of

Nit: extra ,

> Source/WebKit2/ChangeLog:17
> +        having exhausted its key of key events.

Nit: its key of key events?

[Joseph Pecoraro]

(In reply to comment #5)
> Comment on attachment 284402 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=284402&action=review
> 
> LGTM but please check EWS oranges.

The are all about "editing/deleting/delete-emoji.html" test failures happening everywhere right now.

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/203635>