Changeset 234599 in webkit


Ignore:
Timestamp:
Aug 6, 2018 6:59:04 AM (6 years ago)
Author:
Wenson Hsieh
Message:

[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 Tim Horton.

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.

Location:
trunk/Tools
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Tools/ChangeLog

    r234569 r234599  
     12018-08-06  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [iOS] Layout tests that send HID events cause WebKitTestRunner to crash on recent SDKs
     4        https://bugs.webkit.org/show_bug.cgi?id=188334
     5        <rdar://problem/40630074>
     6
     7        Reviewed by Tim Horton.
     8
     9        To mark the end of previously dispatched IOHID events, HIDEventGenerator currently sends a vendor-defined event
     10        and stores the completion callback ID for the previously dispatched events as vendor-defined data. When this
     11        vendor-defined marker event is handled by the application, we then read the callback ID back from the event, map
     12        it to a completion block, and invoke the completion block to signal that the previous HID event has been
     13        processed.
     14
     15        This callback ID is an unsigned, so we tell IOKit that we need `sizeof(unsigned)` (4 bytes) to store it. On
     16        shipping software, IOKit clamps this to a minimum of 8 bytes, i.e. `sizeof(CFIndex)`. When we later call
     17        IOHIDEventGetIntegerValue to read the value of our vendor-defined data as a CFIndex, we get our expected
     18        callback ID because the buffer was clamped to 8 bytes.
     19
     20        However, on recent iOS SDKs that contain the fix for <rdar://problem/20082284>, IOKit no longer clamps the size
     21        of the vendor-defined data buffer to 8 bytes. This means that when we try to use IOHIDEventGetIntegerValue to
     22        read our callback ID back, we end up getting a CFIndex where the lower 4 bytes are the callback ID we wrote, and
     23        the upper 4 bytes are garbage. In the case where any of these upper 4 bytes are non-zero, we fail to map the
     24        callback ID to a completion handler, and so we never finish dispatching the HID event, causing an exception to
     25        be thrown.
     26
     27        To fix this, we adjust callback ID to be a CFIndex, which matches IOHIDEventGetIntegerValue's return type.
     28
     29        * WebKitTestRunner/ios/HIDEventGenerator.mm:
     30        (+[HIDEventGenerator nextEventCallbackID]):
     31        (-[HIDEventGenerator _sendMarkerHIDEventWithCompletionBlock:]):
     32
     33        Also refactor a bit of `-_sendMarkerHIDEventWithCompletionBlock:` by using auto and move semantics.
     34
    1352018-08-03  Ben Richards  <benton_richards@apple.com>
    236
  • trunk/Tools/WebKitTestRunner/ios/HIDEventGenerator.mm

    r222890 r234599  
    172172}
    173173
    174 + (unsigned)nextEventCallbackID
    175 {
    176     static unsigned callbackID = 0;
     174+ (CFIndex)nextEventCallbackID
     175{
     176    static CFIndex callbackID = 0;
    177177    return ++callbackID;
    178178}
     
    480480- (BOOL)_sendMarkerHIDEventWithCompletionBlock:(void (^)(void))completionBlock
    481481{
    482     unsigned callbackID = [HIDEventGenerator nextEventCallbackID];
    483     void (^completionBlockCopy)() = Block_copy(completionBlock);
    484     [_eventCallbacks setObject:completionBlockCopy forKey:@(callbackID)];
    485 
    486     uint64_t machTime = mach_absolute_time();
    487     RetainPtr<IOHIDEventRef> markerEvent = adoptCF(IOHIDEventCreateVendorDefinedEvent(kCFAllocatorDefault,
    488         machTime,
     482    auto callbackID = [HIDEventGenerator nextEventCallbackID];
     483    [_eventCallbacks setObject:Block_copy(completionBlock) forKey:@(callbackID)];
     484
     485    auto markerEvent = adoptCF(IOHIDEventCreateVendorDefinedEvent(kCFAllocatorDefault,
     486        mach_absolute_time(),
    489487        kHIDPage_VendorDefinedStart + 100,
    490488        0,
    491489        1,
    492490        (uint8_t*)&callbackID,
    493         sizeof(unsigned),
     491        sizeof(CFIndex),
    494492        kIOHIDEventOptionNone));
    495493   
    496494    if (markerEvent) {
    497         markerEvent.get();
    498         dispatch_async(dispatch_get_main_queue(), ^{
    499             uint32_t contextID = [UIApplication sharedApplication].keyWindow._contextId;
     495        dispatch_async(dispatch_get_main_queue(), [markerEvent = WTFMove(markerEvent)] {
     496            auto contextID = [UIApplication sharedApplication].keyWindow._contextId;
    500497            ASSERT(contextID);
    501498            BKSHIDEventSetDigitizerInfo(markerEvent.get(), contextID, false, false, NULL, 0, 0);
Note: See TracChangeset for help on using the changeset viewer.