[webkit-reviews] review granted: [Bug 189228] [Cocoa] Turn on ARC for WebKitTestRunner : [Attachment 348777] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Darin Adler <darin at apple.com>'s request
for review:
Bug 189228: [Cocoa] Turn on ARC for WebKitTestRunner
https://bugs.webkit.org/show_bug.cgi?id=189228

Attachment 348777: Patch

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




--- Comment #4 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 348777
  --> https://bugs.webkit.org/attachment.cgi?id=348777
Patch

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

> Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityTextMarkerIOS.mm:32
> +    return [(__bridge id)platformTextMarker() isEqual:(__bridge
id)other->platformTextMarker()];

Is there any way to make platformTextMarker() return (or rather make
PlatformTextMarker) an id rather than CFTypeRef? I know in WebCore we use the
trick of doing something like:

#ifndef __OBJC__
typedef struct objc_object *id;
#endif

in the C++ class. Though I am not sure what the implications for ARC are of
that.

> Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:1066
> +    return AccessibilityTextMarkerRange::create((__bridge
PlatformTextMarkerRange)textMarkerRange);

Same question as above, but for PlatformTextMarkerRange.

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:1825
> -    id mutableString = [[[NSMutableString alloc] init] autorelease];
> +    id mutableString = [[NSMutableString alloc] init];

Seems like this could be NSMutableString * rather than id (as long as you are
changing this line).

> Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:138
> +    return JSStringCreateWithCFString((__bridge
CFStringRef)webView.accessibilitySpeakSelectionContent);

There is a category in WebKitTestRunner that adds a  -[NSString
createJSStringRef] (and a +[NSString stringWithJSStringRef:]). It is currently
in AccessibilityCommonMac.h, but seems more generally useful (could probably
add ones for WKStringRef as well if we were going wild).