Bug 219723

Summary: Remove unnecessary locking from JSValue API functions
Product: WebKit Reporter: Tadeu Zagallo <tzagallo>
Component: JavaScriptCoreAssignee: Tadeu Zagallo <tzagallo>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, mark.lam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=219787
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Tadeu Zagallo 2020-12-09 18:41:35 PST
...
Comment 1 Tadeu Zagallo 2020-12-09 18:47:16 PST
Created attachment 415819 [details]
Patch

The patch won't apply since it was written on top of b219663
Comment 2 Mark Lam 2020-12-10 14:05:48 PST
Comment on attachment 415819 [details]
Patch

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

> Source/JavaScriptCore/API/JSValue.mm:404
> +    return bitwise_cast<JSC::JSValue>(m_value).isUndefined();

nit: Why not encapsulate this in a toJS(JSValueRef)  inline function (complement to the toRef(JSCJSValue) you have above? Currently, you're doing bitwise_cast in many places.  I think such an inline function would express the intent a little better.
Comment 3 Tadeu Zagallo 2020-12-10 14:14:19 PST
Comment on attachment 415819 [details]
Patch

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

>> Source/JavaScriptCore/API/JSValue.mm:404
>> +    return bitwise_cast<JSC::JSValue>(m_value).isUndefined();
> 
> nit: Why not encapsulate this in a toJS(JSValueRef)  inline function (complement to the toRef(JSCJSValue) you have above? Currently, you're doing bitwise_cast in many places.  I think such an inline function would express the intent a little better.

That sounds fair, I'll refactor it before landing.
Comment 4 Tadeu Zagallo 2020-12-10 19:16:25 PST
Created attachment 415959 [details]
Patch for landing
Comment 5 EWS 2020-12-10 20:20:45 PST
Committed r270665: <https://trac.webkit.org/changeset/270665>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 415959 [details].
Comment 6 Radar WebKit Bug Importer 2020-12-10 20:21:18 PST
<rdar://problem/72209082>