[webkit-reviews] review granted: [Bug 209268] Implement support for cursor interactions on iPad : [Attachment 393938] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Tim Horton <thorton at apple.com>'s
request for review:
Bug 209268: Implement support for cursor interactions on iPad
https://bugs.webkit.org/show_bug.cgi?id=209268

Attachment 393938: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 393938
  --> https://bugs.webkit.org/attachment.cgi?id=393938
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8122
> +    static dispatch_once_t onceToken;
> +    dispatch_once(&onceToken, ^{
> +	   // System apps will always be linked on the current OS, so
> +	   // check them before the linked-on-or-after.
> +
> +	   // <rdar://problem/59521967> iAd Video does not respond to mouse
events, only touch events
> +	   if (WebCore::IOSApplication::isNews() ||
WebCore::IOSApplication::isStocks())
> +	       shouldUseMouseGestureRecognizer = NO;
> +
> +	   if
(WebKit::linkedOnOrAfter(WebKit::SDKVersion::FirstThatSendsNativeMouseEvents))
> +	       return;
> +
> +	   // <rdar://problem/59155220> Some Feedly UI does not respond to
mouse events, only touch events
> +	   if (WebCore::IOSApplication::isFeedly())
> +	       shouldUseMouseGestureRecognizer = NO;
> +    });

Is the thread safety really needed here? Can it be called from more than one
thread concurrently?

- If so, is everything the dispatch_once block does safe to be called from any
thread?

- If not, I suggest instead writing this:

    static const BOOL shouldUseMouseGestureRecognizer = []() -> BOOL {
	... code here ...
    }();

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8127
> +- (void)setupMouseGestureRecognizer

The verb "set up" is two words, not one. So I would name this
setUpMouseGestureRecognizer.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:8154
> +- (void)setupCursorInteraction

And name this setUpCursorInteraction.