867844 - Root some random stuff in content/ that has lots of GC hazards

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

We have things here and there that need rooting.

Comment 1

[David Zbarsky (:dzbarsky)]

See bug 866450.  Maybe we should get that reviewed and landed so we don't duplicate effort.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Ugh.  We've already duplicated effort.  ;)

I'll go review that right now.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Fix rooting hazards in LegacyCall.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Fix remaining rooting hazards in nsFrameMessageManager.cpp.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. Fix rooting issues in canvas 2d code.

Comment 7

[(no longer active)]

I filed bug 868776 about fixing rooting hazards in Web Audio.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Fix rooting hazards in LegacyCall.

Comment 9

[(no longer active)]

Also see bug 868782 and 868783.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 745575 [details] [diff] [review]
part 1.  Fix rooting hazards in LegacyCall.

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

::: dom/bindings/Codegen.py
@@ +4795,5 @@
>      function to do the rest of the work.  This function should return a
>      CGThing which is already properly indented.
>      """
>      def __init__(self, descriptor, name, args, unwrapFailureCode=None,
> +                 getThisObj="&args.computeThis(cx).toObject()",

Could you explain this.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 745575 [details] [diff] [review]
part 1.  Fix rooting hazards in LegacyCall.


>     def __init__(self, descriptor, name, args, unwrapFailureCode=None,
>-                 getThisObj="JS_THIS_OBJECT(cx, vp)"):
>+                 getThisObj="&args.computeThis(cx).toObject()",
er, this one.
(don't understand how review tool works)

Comment 12

[Boris Zbarsky [:bzbarsky]]

> Could you explain this.

Sure.  It's functionally equivalent to JS_THIS_OBJECT(cx, vp), but actually uses the 'args' which I introduced to be able to pass in a handle to legacycaller via args.thisv().  This change is not, strictly speaking, needed for legacycaller, but without it we have an unused args in lots of methods triggering tons of compiler warnings.  Since our long-term plan is to move to CallArgs in this code anyway, I figured I'd take this first step and silence the warnings.

Comment 13

[Boris Zbarsky [:bzbarsky]]

   https://hg.mozilla.org/integration/mozilla-inbound/rev/aed0d9db6a9c
   https://hg.mozilla.org/integration/mozilla-inbound/rev/f3881fe6a5b8
   https://hg.mozilla.org/integration/mozilla-inbound/rev/07dc100e6fbb

I'll file other bugs for more rooting.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aed0d9db6a9c
https://hg.mozilla.org/mozilla-central/rev/f3881fe6a5b8
https://hg.mozilla.org/mozilla-central/rev/07dc100e6fbb