Closed Bug 867844 Opened 7 years ago Closed 7 years ago

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

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(3 files, 1 obsolete file)

We have things here and there that need rooting.
Depends on: 861022
See bug 866450.  Maybe we should get that reviewed and landed so we don't duplicate effort.
Ugh.  We've already duplicated effort.  ;)

I'll go review that right now.
Depends on: 866450
Attachment #745493 - Flags: review?(bugs)
Duplicate of this bug: 865475
I filed bug 868776 about fixing rooting hazards in Web Audio.
Attachment #745493 - Attachment is obsolete: true
Attachment #745493 - Flags: review?(bugs)
Also see bug 868782 and 868783.
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 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)
Attachment #745575 - Flags: review?(bugs) → review+
Attachment #745494 - Flags: review?(bugs) → review+
Attachment #745496 - Flags: review?(bugs) → review+
> 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.
   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.
Summary: Root random stuff in content/ → Root some random stuff in content/ that has lots of GC hazards
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla23
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.