[webkit-reviews] review granted: [Bug 210625] [iOS WK1] -[_WebSafeForwarder asyncForwarder] uses non-static dispatch_once_t predicate : [Attachment 396720] Patch v1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 17 17:55:48 PDT 2020


Daniel Bates <dbates at webkit.org> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 210625: [iOS WK1] -[_WebSafeForwarder asyncForwarder] uses non-static
dispatch_once_t predicate
https://bugs.webkit.org/show_bug.cgi?id=210625

Attachment 396720: Patch v1

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




--- Comment #4 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 396720
  --> https://bugs.webkit.org/attachment.cgi?id=396720
Patch v1

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

Patch looks good.

> Source/WebKitLegacy/mac/WebView/WebView.mm:563
> +    _WebSafeAsyncForwarder *_asyncForwarder;

This is ok-as is. No change needed. The optimal solution would use RetainPtr
because it:

1. Removes the need for -release and nil assignment in -dealloc.
2. Annotates that this ivar is strongly held.
3. Future proofs code for possible ARC transition.

> Source/WebKitLegacy/mac/WebView/WebView.mm:5349
> + at synthesize asyncForwarder=_asyncForwarder;

This is ok as-is. No change needed. A slightly better solution would put spaces
around the =.

> Source/WebKitLegacy/mac/WebView/WebView.mm:5376
> +    if ([_target respondsToSelector:[invocation selector]]) {

This is ok as-is. No change needed. Dot notation could also be used on this
line and lines below for modernization.


More information about the webkit-reviews mailing list