Closed
Bug 655089
Opened 14 years ago
Closed 14 years ago
nsXPConnect::ToParticipant calls js_GetGCThingTraceKind twice
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla7
People
(Reporter: mccr8, Assigned: mccr8)
Details
Attachments
(1 file, 1 obsolete file)
3.90 KB,
patch
|
mrbkap
:
review+
ehsan.akhgari
:
checkin+
|
Details | Diff | Splinter Review |
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•14 years ago
|
||
Assignee | ||
Comment 2•14 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•14 years ago
|
||
fixed bitrot
Assignee: nobody → continuation
Attachment #535223 -
Attachment is obsolete: true
Assignee | ||
Comment 4•14 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 5•14 years ago
|
||
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•14 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•14 years ago
|
Attachment #538561 -
Flags: checkin?
Comment 6•14 years ago
|
||
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+
Updated•14 years ago
|
Keywords: checkin-needed
Comment 7•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in
before you can comment on or make changes to this bug.
Description
•