Bug 93979 – [WK2] Implement eventSender.scheduleAsynchronousKeyDown

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Sergio Villar Senin]

Needed by: fast/mutation/end-of-task-delivery.html

[Chris Dumez]

Created attachment 180989 [details]
Patch

[Benjamin Poulain]

Comment on attachment 180989 [details]
Patch

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

The patch looks good. Some nitpick:

> Tools/ChangeLog:14
> +        (WTR):

You can remove this.

> Tools/WebKitTestRunner/InjectedBundle/EventSendingController.cpp:269
> +    return EventSenderMessageBody.leakRef();

I think it would be reasonable to return a WKRetainPtr here to avoid accidents if someone forgets to use Adopt.

> Tools/WebKitTestRunner/TestController.cpp:800
> +            WKRetainPtr<WKStringRef> keyKey = adoptWK(WKStringCreateWithUTF8CString("Key"));
> +            WKStringRef key = static_cast<WKStringRef>(WKDictionaryGetItemForKey(messageBodyDictionary, keyKey.get()));
> +
> +            WKRetainPtr<WKStringRef> modifiersKey = adoptWK(WKStringCreateWithUTF8CString("Modifiers"));
> +            WKEventModifiers modifiers = static_cast<WKEventModifiers>(WKUInt64GetValue(static_cast<WKUInt64Ref>(WKDictionaryGetItemForKey(messageBodyDictionary, modifiersKey.get()))));
> +
> +            WKRetainPtr<WKStringRef> locationKey = adoptWK(WKStringCreateWithUTF8CString("Location"));
> +            unsigned location = static_cast<unsigned>(WKUInt64GetValue(static_cast<WKUInt64Ref>(WKDictionaryGetItemForKey(messageBodyDictionary, locationKey.get()))));
> +
> +            // Forward to WebProcess
> +            WKPageSetShouldSendEventsSynchronously(mainWebView()->page(), false);
> +            m_eventSenderProxy->keyDown(key, modifiers, location);
> +
> +            return;

This is duplicated code from the synchronous message handler.
Please refactor to make a common path as much as possible.

[Chris Dumez]

Created attachment 181870 [details]
Patch

Take Benjamin's feedback into consideration.

[Benjamin Poulain]

Comment on attachment 181870 [details]
Patch

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

Looks good.
One comment but it is not blocking:

> Tools/WebKitTestRunner/TestController.cpp:815
> +            didReceiveKeyDownMessageFromInjectedBundle(messageBodyDictionary, false);

You can avoid this kind of code by using a enum instead of a boolean
    didReceiveKeyDownMessageFromInjectedBundle(messageBodyDictionary, SendSynchronously);
    didReceiveKeyDownMessageFromInjectedBundle(messageBodyDictionary, SendAsynchronously);

[WebKit Review Bot]

Comment on attachment 181870 [details]
Patch

Clearing flags on attachment: 181870

Committed r139242: <http://trac.webkit.org/changeset/139242>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.