Bug 189652 – Use OpaqueJSString rather than JSRetainPtr inside WebKit

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Darin Adler]

Use OpaqueJSString rather than JSRetainPtr inside WebKit

[Darin Adler]

Created attachment 349873 [details]
Patch

[Darin Adler]

Keith, this patch is a response to a comment you made in my last one about using smart pointers more consistently. I realized that inside WebKit we should use RefPtr and Ref, not JSRetainPtr, for JSStringRef.

[Saam Barati]

Comment on attachment 349873 [details]
Patch

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

r=me

> Source/JavaScriptCore/API/JSContext.mm:104
> +    auto sourceURLJS = OpaqueJSString::create([sourceURL absoluteString]);

This removes a null check on sourceURL. Is that intended?

> Source/JavaScriptCore/API/JSContext.mm:105
> +    JSValueRef result = JSEvaluateScript(m_context, scriptJS.get(), nullptr, sourceURLJS.get(), 0, &exceptionValue);

Ignore the above comment (I’m on my phone and can’t edit it). I knew that ObjC message send is safe on null, but I didn’t know it’s guaranteed to also return null

[WebKit Commit Bot]

Comment on attachment 349873 [details]
Patch

Clearing flags on attachment: 349873

Committed r236066: <https://trac.webkit.org/changeset/236066>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44523492>

[Darin Adler]

Looks like I missed the JavaScriptCore/API/glib directory when making this change. The code in there still uses JSRetainPtr, not OpaqueJSString.