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

RESOLVED FIXED in mozilla23

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

unspecified
mozilla23
x86
Mac OS X
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

We have things here and there that need rooting.
(Assignee)

Updated

6 years ago
Depends on: 861022
See bug 866450.  Maybe we should get that reviewed and landed so we don't duplicate effort.
(Assignee)

Comment 2

6 years ago
Ugh.  We've already duplicated effort.  ;)

I'll go review that right now.
(Assignee)

Updated

6 years ago
Depends on: 866450
(Assignee)

Comment 3

6 years ago
Created attachment 745493 [details] [diff] [review]
part 1.  Fix rooting hazards in LegacyCall.
Attachment #745493 - Flags: review?(bugs)
(Assignee)

Comment 4

6 years ago
Created attachment 745494 [details] [diff] [review]
part 2.  Fix remaining rooting hazards in nsFrameMessageManager.cpp.
Attachment #745494 - Flags: review?(bugs)
(Assignee)

Comment 5

6 years ago
Created attachment 745496 [details] [diff] [review]
part 3.  Fix rooting issues in canvas 2d code.
Attachment #745496 - Flags: review?(bugs)
(Assignee)

Updated

6 years ago
Blocks: 831379
(Assignee)

Updated

6 years ago
Duplicate of this bug: 865475

Comment 7

6 years ago
I filed bug 868776 about fixing rooting hazards in Web Audio.
(Assignee)

Comment 8

6 years ago
Created attachment 745575 [details] [diff] [review]
part 1.  Fix rooting hazards in LegacyCall.
Attachment #745575 - Flags: review?(bugs)
(Assignee)

Updated

6 years ago
Attachment #745493 - Attachment is obsolete: true
Attachment #745493 - Flags: review?(bugs)

Comment 9

6 years ago
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+

Updated

6 years ago
Attachment #745494 - Flags: review?(bugs) → review+

Updated

6 years ago
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
(Assignee)

Updated

6 years ago
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.