[webkit-reviews] review granted: [Bug 204664] [iOS] WKWebView touch event gesture recognition should not block the application process main thread when possible : [Attachment 385027] Fix iOS tests on non-internal builds

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 6 17:08:38 PST 2019


Tim Horton <thorton at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 204664: [iOS] WKWebView touch event gesture recognition should not block
the application process main thread when possible
https://bugs.webkit.org/show_bug.cgi?id=204664

Attachment 385027: Fix iOS tests on non-internal builds

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




--- Comment #4 from Tim Horton <thorton at apple.com> ---
Comment on attachment 385027
  --> https://bugs.webkit.org/attachment.cgi?id=385027
Fix iOS tests on non-internal builds

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

> Source/WebKit/UIProcess/WebPageProxy.cpp:2818
> +	       --m_handlingPreventableTouchStartCount;

This needs some sort of care in the underflow case. Also, it's possible we
don't want to do all this work in the error case (at least on crash?)? I'm not
really sure.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:168
> +#define USE_DEFERRING_GESTURE_RECOGNIZERS ENABLE(IOS_TOUCH_EVENTS)

This is fairly surprising to me. I don't usually like it when a USE() or
ENABLE() is in a header that you can possibly not have (so prefix header or
Platform.h/FeatureDefines.h and friends are OK, but here would not be), but
��‍♂️

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4211
> +    [_twoFingerDoubleTapGestureRecognizer setEnabled:NO];
> +    [_twoFingerDoubleTapGestureRecognizer setEnabled:YES];

Wonder if we should just add a _wk_cancel category method on UIGR :) this comes
up enough


More information about the webkit-reviews mailing list