[Webkit-unassigned] [Bug 163161] Extend event stream to include interpolated events and add a force press test that uses that interpolation

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 10 13:11:26 PDT 2016


https://bugs.webkit.org/show_bug.cgi?id=163161

Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #291127|review?                     |review-
              Flags|                            |

--- Comment #6 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 291127
  --> https://bugs.webkit.org/attachment.cgi?id=291127
Patch

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

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:307
> +        if (phase != UITouchPhaseCancelled && phase != UITouchPhaseBegan && phase != UITouchPhaseEnded
> +            && phase != UITouchPhaseStationary)

No need to wrap this.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:977
> +    
> +    

Remove one of the blank lines.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:988
> +        NSMutableDictionary *newEvent = [endEvent mutableCopy];

We're not using ARC, so this is leaked.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:997
> +        while ((startTouch = [startEnumerator nextObject]) && (endTouch = [endEnumerator nextObject])) {

This assumes that the touches are listed in "id" order. Maybe you should just bail if the start and end events have touches with non-matching ids? At the very least, you need to find matching touches by id.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:999
> +            NSMutableDictionary *newTouch = [[endTouch mutableCopy] autorelease];

Perferable to just explicitly -release after the [newTouches addObject:newTouch] below.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:1002
> +                newTouch[HIDEventXKey] = [NSNumber numberWithDouble:interpolations[interpolationType]([startTouch[HIDEventXKey] doubleValue], [endTouch[HIDEventXKey] doubleValue], t)];

I think you can use @() here.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:1005
> +                newTouch[HIDEventYKey] = [NSNumber numberWithDouble:interpolations[interpolationType]([startTouch[HIDEventYKey] doubleValue], [endTouch[HIDEventYKey] doubleValue], t)];

Ditto.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:1008
> +                newTouch[HIDEventPressureKey] = [NSNumber numberWithDouble:interpolations[interpolationType]([startTouch[HIDEventPressureKey] doubleValue], [endTouch[HIDEventPressureKey] doubleValue], t)];

Ditto.

> Tools/WebKitTestRunner/ios/HIDEventGenerator.mm:1029
> +        if (interpolate) {
> +            NSArray *newEvents = [self interpolatedEvents:eventInfo];
> +            [self processEventsArray:newEvents withStartTime:startTime];
> +        } else {

I don't like here how you generate the interpolated events in the middle of dispatching. I think you should do a first pass over -events and make a new events array with the interpolated events, and then dispatch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161010/f7da9028/attachment.html>


More information about the webkit-unassigned mailing list