[webkit-reviews] review granted: [Bug 214295] [iOS] The completion handler in -handleKeyWebEvent:withCompletionHandler: is sometimes never called : [Attachment 404213] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 214295: [iOS] The completion handler in
-handleKeyWebEvent:withCompletionHandler: is sometimes never called
https://bugs.webkit.org/show_bug.cgi?id=214295

Attachment 404213: Patch

https://bugs.webkit.org/attachment.cgi?id=404213&action=review




--- Comment #2 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 404213
  --> https://bugs.webkit.org/attachment.cgi?id=404213
Patch

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

r=me

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:1068
> +    if (!_page || !_page->hasQueuedKeyEvent())

Is it possible for `_keyWebEventHandler` to be set without
`_page->hasQueuedKeyEvent()` (or vice versa)?  I feel like this should be an
`ASSERT` inside the `if` below to ensure that the state is synchronized.

> Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:376
> +	   auto keyDownEvent = adoptNS([[WebEvent alloc]
initWithKeyEventType:WebEventKeyDown timeStamp:CFAbsoluteTimeGetCurrent()
characters:@"a" charactersIgnoringModifiers:@"a" modifiers:0 isRepeating:NO
withFlags:0 withInputManagerHint:nil keyCode:0 isTabKey:NO]);

NIT: shouldn't `keyCode` be `65`?