Note: There are a few cases of duplicates in user autocompletion which are being worked on.

nsXPConnect::ToParticipant calls js_GetGCThingTraceKind twice

RESOLVED FIXED in mozilla7

Status

()

Core
XPConnect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Other Branch
mozilla7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
It looks to me that in nsXPConnect.cpp, nsXPConnect::ToParticipant calls js_GetGCThingTraceKind twice due to macro'age:

420 #define ADD_TO_CC(_kind)    ((_kind) == JSTRACE_OBJECT || (_kind) == JSTRACE_XML)
[...]
533 nsCycleCollectionParticipant *
534 nsXPConnect::ToParticipant(void *p)
535 {
536     if (!ADD_TO_CC(js_GetGCThingTraceKind(p)))
537         return NULL;
538     return this;
539 }

Not a huge deal as this function has no side effects, but it work that doesn't need to be done.

This can be fixed by creating a new variable and assigning the result of the function to it, or by turning ADD_TO_CC into an inline function called addToCCKind or something.  The latter would be a bit more civilized, if we can rely on the inlining.  I used the former fix in my patch for Bug 650519.
(Assignee)

Comment 1

6 years ago
Created attachment 535223 [details] [diff] [review]
turn CC_KIND into a call
(Assignee)

Comment 2

6 years ago
I ran this on try, and it looks okay, except mobile looks kind of bad.  One of the Maemo builds is burning, which I will retry, and the Android tests are still running, 8 or so hours later.
(Assignee)

Comment 3

6 years ago
Created attachment 538561 [details] [diff] [review]
change CC_KIND macro to a function to avoid double call

fixed bitrot
Assignee: nobody → continuation
Attachment #535223 - Attachment is obsolete: true
(Assignee)

Comment 4

6 years ago
Comment on attachment 538561 [details] [diff] [review]
change CC_KIND macro to a function to avoid double call

Passed a try run.
Attachment #538561 - Flags: review?(mrbkap)
Comment on attachment 538561 [details] [diff] [review]
change CC_KIND macro to a function to avoid double call

Good catch.
Attachment #538561 - Flags: review?(mrbkap) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Updated

6 years ago
Attachment #538561 - Flags: checkin?
Comment on attachment 538561 [details] [diff] [review]
change CC_KIND macro to a function to avoid double call

Landed on inbound.
Attachment #538561 - Flags: checkin? → checkin+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/0321e3569921
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.