[webkit-reviews] review granted: [Bug 188245] [Cocoa] More tweaks and refactoring to prepare for ARC : [Attachment 346630] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Darin Adler <darin at apple.com>'s request for review:
Bug 188245: [Cocoa] More tweaks and refactoring to prepare for ARC
https://bugs.webkit.org/show_bug.cgi?id=188245

Attachment 346630: Patch

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




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

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

> Tools/ChangeLog:53
> +	   (dumpBackForwardListForAllWindows): User a modern for loop instead
of

Typo: “User”

> Tools/DumpRenderTree/mac/MockGeolocationProvider.mm:130
> +	   auto webView = (__bridge WebView*)typelessView;

Missing space before the *.

> Tools/TestRunnerShared/cocoa/LayoutTestSpellChecker.mm:254
> +	       [resultsForWord addObject:[[LayoutTestTextCheckingResult alloc]
initWithType:nsTextCheckingType(WTFMove(typeValue))
range:NSMakeRange(fromValue, toValue - fromValue) replacement:(__bridge
NSString *)replacementText.get() details:details.get()]];

Isn’t this just leaking now when not using ARC?

> Tools/WebKitTestRunner/mac/WebKitTestRunnerPasteboard.mm:175
> +    if (data == nil)

if (!data)

> Tools/WebKitTestRunner/mac/WebKitTestRunnerWindow.h:37
> + at property (nonatomic, assign) WTR::PlatformWebView* platformWebView;

assign is the default for this type, so I’d omit it.

> Tools/WebKitTestRunner/mac/WebKitTestRunnerWindow.mm:34
> +static Vector<WebKitTestRunnerWindow*> allWindows;

Missing space before the *.