Closed Bug 865785 Opened 7 years ago Closed 7 years ago

Root Constructor for JS-implemented WebIDL


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: mccr8, Assigned: bzbarsky)




(2 files)

This method is a big blob of text, so the main trick will be passing in a JSContext* so we can root things.  Note that bug 865544 will add another unrooted thing to the stack.

Most Constructor methods are hopefully not messing around with raw JS objects, so I would think we don't need the cx everywhere.
Blocks: 865969
sfink helpfully added our test codegen to the rooting analysis builds, so now we can see what the actual hazards are.
Ah, nice.  I think most of the uses here are going to be actual hazards, given that we're running arbitrary blobs of chrome JS.
Some of these hazards will need to be fixed in bug 868715.  The rest, patch coming up.
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Attachment #745562 - Flags: review?(continuation) → review+
Comment on attachment 745563 [details] [diff] [review]
part 2.  Fix rooting issues in JS-implemented webidl.

Review of attachment 745563 [details] [diff] [review]:

Thanks for fixing this!

::: dom/bindings/
@@ +8634,5 @@
>    }
>    // Initialize the object, if it implements nsIDOMGlobalPropertyInitializer.
>    nsCOMPtr<nsIDOMGlobalPropertyInitializer> gpi = do_QueryInterface(implISupports);
>    if (gpi) {
> +    JS::Rooted<JS::Value> initReturn(cx, JSVAL_VOID);

If RootedJS<JS::Value> initializes it to something (anything, really) then you can skip the JSVAL_VOID here.
Attachment #745563 - Flags: review?(continuation) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.