[webkit-reviews] review granted: [Bug 221780] Reduce explicit usage of [objC release] in WebKit : [Attachment 420183] Follow-up

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 221780: Reduce explicit usage of [objC release] in WebKit
https://bugs.webkit.org/show_bug.cgi?id=221780

Attachment 420183: Follow-up

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




--- Comment #19 from Darin Adler <darin at apple.com> ---
Comment on attachment 420183
  --> https://bugs.webkit.org/attachment.cgi?id=420183
Follow-up

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

> Source/WebKitLegacy/mac/History/WebHistoryItem.mm:352
>      NSArray *childDicts = [dict objectForKey:childrenKey];

Could get rid of this local and merge it into the next line.

> Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm:656
> +							   windowFeatures:@{
}];

Existing indenting here is lining up the ":" characters. I prefer just putting
things on one line, but this mix is messy.

> Source/WebKitLegacy/mac/WebCoreSupport/WebFrameLoaderClient.mm:1612
> +    auto *newFrame = kit(result.ptr());

I don’t think this should be:

    auto *xxx =

Either of these makes sense to me:

    auto xxx =
    WebFrame *xxx =

And possibly if you want to emphasize it’s a pointer:

    auto* xxx =

But to put the * in the "Objective-C" place and use auto, doesn’t seem right.

> Source/WebKitLegacy/mac/WebView/WebScriptDebugger.mm:100
> +	   if (NSString* nsErrorMessage = nsStringNilIfEmpty(errorMsg)) {

Seems like we want to move that "*" before nsErrorMessage. Also, could just
name it errorMessage. Or could reverse the logic, write:

    if (errorMsg.isEmpty()) {

And then we would not even have to use nsStringNilIfEmpty at all.

> Source/WebKitLegacy/mac/WebView/WebView.mm:2583
> +					       windowFeatures:@{ }];

Existing indenting here is lining up the ":" characters. I prefer just putting
things on one line, but this mix is messy.