Closed Bug 869784 Opened 7 years ago Closed 7 years ago

Fix rooting hazards in the profiler

Categories

(Core :: Gecko Profiler, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch (v1) (obsolete) — Splinter Review
No description provided.
Attachment #746750 - Flags: review?(tschneidereit)
Attachment #746750 - Flags: review?(bgirard)
Attached patch Patch (v1)Splinter Review
Oops, that wouldn't build!
Assignee: nobody → ehsan
Attachment #746750 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #746750 - Flags: review?(tschneidereit)
Attachment #746750 - Flags: review?(bgirard)
Attachment #746751 - Flags: review?(tschneidereit)
Attachment #746751 - Flags: review?(bgirard)
Comment on attachment 746751 [details] [diff] [review]
Patch (v1)

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

::: tools/profiler/JSObjectBuilder.cpp
@@ +6,5 @@
>  #include "jsapi.h"
>  #include "nsStringGlue.h"
>  #include "JSObjectBuilder.h"
>  
> +JSObjectBuilder::JSObjectBuilder(JSContext *aCx) : mCx(aCx), mObj(aCx), mOk(JS_TRUE)

Why not just remove mCx? The rest looks fine.
Attachment #746751 - Flags: review?(bgirard) → review+
Comment on attachment 746751 [details] [diff] [review]
Patch (v1)

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

What BenWa said.
Attachment #746751 - Flags: review?(tschneidereit) → review+
(In reply to Benoit Girard (:BenWa) from comment #2)
> Comment on attachment 746751 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 746751 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: tools/profiler/JSObjectBuilder.cpp
> @@ +6,5 @@
> >  #include "jsapi.h"
> >  #include "nsStringGlue.h"
> >  #include "JSObjectBuilder.h"
> >  
> > +JSObjectBuilder::JSObjectBuilder(JSContext *aCx) : mCx(aCx), mObj(aCx), mOk(JS_TRUE)
> 
> Why not just remove mCx? The rest looks fine.

Remove mCx?  Not sure what you mean, mCx is used all over the place.
Flags: needinfo?(bgirard)
Opps mObj*. I couldn't see it used anywhere last night.
Flags: needinfo?(bgirard)
https://hg.mozilla.org/mozilla-central/rev/202d4048a8b8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.