829230 - Some rooting for JSCompartment::wrap

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tom S]

Attachment: v1

I am pretty sure there could be done a lot more for this function. But this removes 81 locally for me, down to 116 failures on jit-tests.

Comment 1

[Tom S]

Attachment: clean version

Some stuff slipped in there.

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 700629 [details] [diff] [review]
clean version

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

Awesome! I guess this means we are testing |JSCompartment::wrap| now.

::: js/src/jscompartment.cpp
@@ +352,5 @@
>      }
>  
>      if (vp->isString()) {
>          RootedValue orig(cx, *vp);
> +        Rooted<JSStableString *> str(cx, vp->toString()->ensureStable(cx));

RootedStableString. Add |ForwardDeclareJS(StableString);|, replacing the existing |class JSStableString;| forward declaration at the top of vm/String.h.

Comment 3

[Tom S]

(In reply to Terrence Cole [:terrence] from comment #2)
> Comment on attachment 700629 [details] [diff] [review]
> clean version
> 
> Review of attachment 700629 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Awesome! I guess this means we are testing |JSCompartment::wrap| now.
> 
> ::: js/src/jscompartment.cpp
> @@ +352,5 @@
> >      }
> >  
> >      if (vp->isString()) {
> >          RootedValue orig(cx, *vp);
> > +        Rooted<JSStableString *> str(cx, vp->toString()->ensureStable(cx));
> 
> RootedStableString. Add |ForwardDeclareJS(StableString);|, replacing the
> existing |class JSStableString;| forward declaration at the top of
> vm/String.h.
This didn't really work out as planned, discussed on IRC.

Comment 4

[Tom S]

https://hg.mozilla.org/integration/mozilla-inbound/rev/575c6e6283b6

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/575c6e6283b6