Bug 114841 - Objective-C API: Update public header documentation
Summary: Objective-C API: Update public header documentation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-18 18:02 PDT by Mark Hahnenberg
Modified: 2013-04-23 10:59 PDT (History)
0 users

See Also:


Attachments
Patch (3.62 KB, patch)
2013-04-18 18:03 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (3.56 KB, patch)
2013-04-19 10:25 PDT, Mark Hahnenberg
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2013-04-18 18:02:02 PDT
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.
Comment 1 Mark Hahnenberg 2013-04-18 18:02:12 PDT
<rdar://problem/13351539>
Comment 2 Mark Hahnenberg 2013-04-18 18:03:39 PDT
Created attachment 198783 [details]
Patch
Comment 3 Geoffrey Garen 2013-04-18 23:02:42 PDT
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....
Comment 4 Mark Hahnenberg 2013-04-19 10:25:19 PDT
Created attachment 198896 [details]
Patch
Comment 5 Geoffrey Garen 2013-04-22 13:27:14 PDT
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.
Comment 6 Mark Hahnenberg 2013-04-23 10:59:29 PDT
Committed r148974: <http://trac.webkit.org/changeset/148974>