Attachment 346606 Details for Bug 188334 – Patch

[patch] Patch

bug-188334-20180805162448.patch (text/plain), 4.64 KB, created by Wenson Hsieh on 2018-08-05 16:24:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-08-05 16:24:48 PDT

Size: 4.64 KB

patch

obsolete

>Subversion Revision: 234569
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 1f12887fd1c1bfabbad1a112a4c1676c4c7e129f..803bef96c8113ac4e746c8e4b8ea4088d8e8f200 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,37 @@
>+2018-08-05  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Layout tests that send HID events cause WebKitTestRunner to crash on recent SDKs
>+        https://bugs.webkit.org/show_bug.cgi?id=188334
>+        <rdar://problem/40630074>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        To mark the end of previously dispatched IOHID events, HIDEventGenerator currently sends a vendor-defined event
>+        and stores the completion callback ID for the previously dispatched events as vendor-defined data. When this
>+        vendor-defined marker event is handled by the application, we then read the callback ID back from the event, map
>+        it to a completion block, and invoke the completion block to signal that the previous HID event has been
>+        processed.
>+
>+        This callback ID is an unsigned, so we tell IOKit that we need `sizeof(unsigned)` (4 bytes) to store it. On
>+        shipping software, IOKit clamps this to a minimum of 8 bytes, i.e. `sizeof(CFIndex)`. When we later call
>+        IOHIDEventGetIntegerValue to read the value of our vendor-defined data as a CFIndex, we get our expected
>+        callback ID because the buffer was clamped to 8 bytes.
>+
>+        However, on recent iOS SDKs that contain the fix for <rdar://problem/20082284>, IOKit no longer clamps the size
>+        of the vendor-defined data buffer to 8 bytes. This means that when we try to use IOHIDEventGetIntegerValue to
>+        read our callback ID back, we end up getting a CFIndex where the lower 4 bytes are the callback ID we wrote, and
>+        the upper 4 bytes are garbage. In the case where any of these upper 4 bytes are non-zero, we fail to map the
>+        callback ID to a completion handler, and so we never finish dispatching the HID event, causing an exception to
>+        be thrown.
>+
>+        To fix this, we adjust callback ID to be a CFIndex, which matches IOHIDEventGetIntegerValue's return type.
>+
>+        * WebKitTestRunner/ios/HIDEventGenerator.mm:
>+        (+[HIDEventGenerator nextEventCallbackID]):
>+        (-[HIDEventGenerator _sendMarkerHIDEventWithCompletionBlock:]):
>+
>+        Also refactor a bit of `-_sendMarkerHIDEventWithCompletionBlock:` by using auto and move semantics.
>+
> 2018-08-03  Ben Richards  <benton_richards@apple.com>
> 
>         We should cache the compiled sandbox profile in a data vault
>diff --git a/Tools/WebKitTestRunner/ios/HIDEventGenerator.mm b/Tools/WebKitTestRunner/ios/HIDEventGenerator.mm
>index 3d50dedd8d69eae979e9e7749cb58c0ee4b07255..29262a4e94a447dd0868f412b00367c3d1addaef 100644
>--- a/Tools/WebKitTestRunner/ios/HIDEventGenerator.mm
>+++ b/Tools/WebKitTestRunner/ios/HIDEventGenerator.mm
>@@ -171,9 +171,9 @@ + (HIDEventGenerator *)sharedHIDEventGenerator
>     return eventGenerator;
> }
> 
>-+ (unsigned)nextEventCallbackID
>++ (CFIndex)nextEventCallbackID
> {
>-    static unsigned callbackID = 0;
>+    static CFIndex callbackID = 0;
>     return ++callbackID;
> }
> 
>@@ -479,24 +479,21 @@ - (BOOL)_sendHIDEvent:(IOHIDEventRef)eventRef
> 
> - (BOOL)_sendMarkerHIDEventWithCompletionBlock:(void (^)(void))completionBlock
> {
>-    unsigned callbackID = [HIDEventGenerator nextEventCallbackID];
>-    void (^completionBlockCopy)() = Block_copy(completionBlock);
>-    [_eventCallbacks setObject:completionBlockCopy forKey:@(callbackID)];
>+    auto callbackID = [HIDEventGenerator nextEventCallbackID];
>+    [_eventCallbacks setObject:Block_copy(completionBlock) forKey:@(callbackID)];
> 
>-    uint64_t machTime = mach_absolute_time();
>-    RetainPtr<IOHIDEventRef> markerEvent = adoptCF(IOHIDEventCreateVendorDefinedEvent(kCFAllocatorDefault,
>-        machTime,
>+    auto markerEvent = adoptCF(IOHIDEventCreateVendorDefinedEvent(kCFAllocatorDefault,
>+        mach_absolute_time(),
>         kHIDPage_VendorDefinedStart + 100,
>         0,
>         1,
>         (uint8_t*)&callbackID,
>-        sizeof(unsigned),
>+        sizeof(CFIndex),
>         kIOHIDEventOptionNone));
>     
>     if (markerEvent) {
>-        markerEvent.get();
>-        dispatch_async(dispatch_get_main_queue(), ^{
>-            uint32_t contextID = [UIApplication sharedApplication].keyWindow._contextId;
>+        dispatch_async(dispatch_get_main_queue(), [markerEvent = WTFMove(markerEvent)] {
>+            auto contextID = [UIApplication sharedApplication].keyWindow._contextId;
>             ASSERT(contextID);
>             BKSHIDEventSetDigitizerInfo(markerEvent.get(), contextID, false, false, NULL, 0, 0);
>             [[UIApplication sharedApplication] _enqueueHIDEvent:markerEvent.get()];

Actions: View | Formatted Diff | Diff

Attachments on bug 188334: 346606