[webkit-reviews] review canceled: [Bug 150763] Replace iOS-only WebKitSystemInterface calls with SPI : [Attachment 264498] Patch

[bugzilla-daemon at webkit.org]

Andy Estes <aestes at apple.com> has canceled Andy Estes <aestes at apple.com>'s
request for review:
Bug 150763: Replace iOS-only WebKitSystemInterface calls with SPI
https://bugs.webkit.org/show_bug.cgi?id=150763

Attachment 264498: Patch

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




--- Comment #2 from mitz at webkit.org <mitz at webkit.org> ---
Comment on attachment 264498
  --> https://bugs.webkit.org/attachment.cgi?id=264498
Patch

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

> Source/WebKit2/Shared/AccessibilityIOS.h:1
> +/*

Shouldn’t this file go somewhere inside the platform directory?

> Source/WebKit2/Shared/AccessibilityIOS.h:28
> +NSData *AXRemoteToken(CFUUIDRef);

This doesn’t mirror any SPI. It’s strange to give this function a name that
begins with AX, the prefix of the system Accessibility APIs, even though it’s
in the WebKit namespace. How about “newAccessibilityRemoteToken” (or maybe
change the return type to a RetainPtr and have the name begin with create)?

> Source/WebKit2/UIProcess/ios/WKContentView.mm:433
> +static void AXStoreRemoteConnectionInformation(id element, pid_t pid,
mach_port_t sendPort, CFUUIDRef uuidRef)

Here, too, naming this as if it were an Accessibility API is strange.

> Source/WebKit2/UIProcess/ios/WKContentView.mm:443
>      RetainPtr<CFUUIDRef> uuid = adoptCF(CFUUIDCreate(kCFAllocatorDefault));

It’d make the code a bit cleaner if you changed everything (including the
function in AccessibilityIOS.mm) to use NSUUID instead. There’s no reason to
use the CF type.