Bug 114841 – Objective-C API: Update public header documentation

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

[Mark Hahnenberg]

There has been a lot of changes/additions to the Objective-C API during development, so we need to make sure all of the public headers have sufficient and correct documentation prior to final release.

[Mark Hahnenberg]

<rdar://problem/13351539>

[Mark Hahnenberg]

Created attachment 198783 [details]
Patch

[Geoffrey Garen]

Comment on attachment 198783 [details]
Patch

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

> Source/JavaScriptCore/API/JSManagedValue.h:36
> +// JSManagedValue represents a "conditionally strong" JSValue. "Conditionally strong" means that 

Maybe we should say "conditionally retained" to match typical ObjC language.

> Source/JavaScriptCore/API/JSManagedValue.h:38
> +// as long as the JSManagedValue object is reachable either through the JavaScript object graph
> +// or through the external Objective-C object graph as reported to the JSVirtualMachine using 

We should be a little clearer here: The JSManagedValue's JSValue can be reachable through the JS object graph; the JSManageValue itself can be reachable through addManagedReference:withOwner:.

> Source/JavaScriptCore/API/JSManagedValue.h:40
> +// addManagedReference:withOwner:, the corresponding JavaScript value will remain alive inside 
> +// of the JSVirtualMachine. However, if none of these conditions are true, the corresponding 

Let's say "be retained" here as well.

> Source/JavaScriptCore/API/JSManagedValue.h:42
> +// JavaScript value becomes a weak value, meaning it can be collected by the JavaScript garbage 
> +// collector at any time.

Let's say "be release" here.

> Source/JavaScriptCore/API/JSVirtualMachine.h:41
> +// addManagedReference:withOwner and removeManagedReference:withOwner allow clients of to make the 

Typo: "clients of the JSVirtualMachine".

> Source/JavaScriptCore/API/JSVirtualMachine.h:49
> +// There is one caveat to this API: the root of any particular Objective-C object graph must be reachable from
> +// within the JavaScript runtime. For example, a client could install a wrapped Objective-C object 
> +// as a property on the global object within the JSContext. If the external object graph is not 
> +// reachable from within the JSContext then that object graph will not be scanned during garbage
> +// collection, which will cause objects referenced from Objective-C to be prematurely collected.

I think this would read more clearly if you phrased it in terms of what's required to get correct behavior:

If an Objective-C object is reachable from within the JavaScript runtime, all managed references transitively reachable from it will be scanned by the garbage collector....

[Mark Hahnenberg]

Created attachment 198896 [details]
Patch

[Geoffrey Garen]

Comment on attachment 198896 [details]
Patch

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

r=me

> Source/JavaScriptCore/API/JSManagedValue.h:42
> +// be retained inside of the JSVirtualMachine. However, if neither of these 

Let's drop "inside the JSVirtualMachine" here and say, "the corresponding JSValue will be retained". That puts things more in terms of classical Cocoa programming.

> Source/JavaScriptCore/API/JSManagedValue.h:44
> +// conditions are true, the corresponding JavaScript value becomes a weak value,
> +// meaning it can be released by the JavaScript garbage collector at any time.

Here, let's say, "the corresponding JSValue will be released and set to nil". We can hide the detail that it only becomes nil after the GC notices.

[Mark Hahnenberg]

Committed r148974: <http://trac.webkit.org/changeset/148974>