Closed Bug 899973 Opened 6 years ago Closed 6 years ago

GC: Convert the rest of the JS property API to use MutableHandleValue for out params

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: jonco, Assigned: jonco)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

Attached patch property-api-mhvSplinter Review
Following on from bug 897484 and bug 896540, let's convert the rest of the JS property API to use MutableHandleValue for out parameters.
Attachment #783707 - Flags: review?(sphink)
Attachment #783710 - Flags: review?(bzbarsky)
Comment on attachment 783707 [details] [diff] [review]
property-api-mhv

Review of attachment 783707 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsapi.cpp
@@ +3144,2 @@
>  JS_PUBLIC_API(JSBool)
>  JS_LookupElement(JSContext *cx, JSObject *objArg, uint32_t index, jsval *vp)

How about this one?

@@ +3973,5 @@
>      JSBool succeeded;
>      if (!JSObject::deleteByValue(cx, obj, StringValue(atom), &succeeded))
>          return false;
>  
> +    rval.setBoolean(succeeded);

These really should become bool*s
(In reply to :Ms2ger from comment #2)

> How about this one?

I'm leaving the JS_*Element() ones for the next bug.

> These really should become bool*s

Good idea - I'll do that as a followup bug too.
Comment on attachment 783707 [details] [diff] [review]
property-api-mhv

Review of attachment 783707 [details] [diff] [review]:
-----------------------------------------------------------------

Yay! It's really great to see these changing.
Attachment #783707 - Flags: review?(sphink) → review+
Comment on attachment 783710 [details] [diff] [review]
property-api-mhv-browser

>+    JS::RootedValue prop(cx);

(Making rooting hazards not compile)++.

r=me
Attachment #783710 - Flags: review?(bzbarsky) → review+
No longer depends on: ExactRootingBrowser
https://hg.mozilla.org/mozilla-central/rev/e291816b49be
https://hg.mozilla.org/mozilla-central/rev/5ee8be4e4815
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Blocks: 773686
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.