[webkit-reviews] review granted: [Bug 105262] Add SPI to WebKit1 WebFrame for node conversion to JSValueRef : [Attachment 179906] patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has granted Alice Liu
<alice.liu at apple.com>'s request for review:
Bug 105262: Add SPI to WebKit1 WebFrame for node conversion to JSValueRef
https://bugs.webkit.org/show_bug.cgi?id=105262

Attachment 179906: patch
https://bugs.webkit.org/attachment.cgi?id=179906&action=review

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=179906&action=review


> Tools/TestWebKitAPI/Tests/mac/JSWrapperForNodeInWebFrame.mm:54
> +    RetainPtr<WebView> webView(AdoptNS, [[WebView alloc]
initWithFrame:NSMakeRect(0, 0, 120, 200) frameName:nil groupName:nil]);

You can use the adoptNS function instead for clarity.

> Tools/TestWebKitAPI/Tests/mac/JSWrapperForNodeInWebFrame.mm:55
> +    RetainPtr<JSWrapperForNodeFrameLoadDelegate> frameLoadDelegate(AdoptNS,
[JSWrapperForNodeFrameLoadDelegate new]);

Ditto. Also use alloc/init instead of new.

> Tools/TestWebKitAPI/Tests/mac/JSWrapperForNodeInWebFrame.mm:82
> +    JSStringRef isolatedPropertyJSString =
JSStringCreateWithUTF8CString("isolatedProperty");

I think you can use a JSRetainPtr here, then you don't need to use
JSStringRelease.

> Tools/TestWebKitAPI/Tests/mac/JSWrapperForNodeInWebFrame.mm:88
> +    JSStringRef normalPropertyJSString =
JSStringCreateWithUTF8CString("normalProperty");

I think you can use a JSRetainPtr here, then you don't need to use
JSStringRelease.