nsScriptErrorWithStack shadows the refcount in nsScriptError

RESOLVED FIXED in Firefox 42

Status

()

Core
XPConnect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

Trunk
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
Sorry for missing this one too. :-(
Flags: needinfo?(poirot.alex)
(Assignee)

Comment 2

3 years ago
It isn't a big deal. The virtual AddRef/Release ensure we only use the right one. This is also the sort of tedious non-local property that should be caught by static analysis (as we should get with bug 602122).

The fix for this would be something like change nsScriptError into a nsScriptErrorBase class that does not implement refcounting, but is otherwise the same as nsScriptError. nsScriptErrorWithStack stays the same, except that it inherits from nsScriptErrorBase instead. Then you define a new nsScriptError class which inherits from nsScriptErrorBase, but doesn't do anything except implement refcounting. An example of this is nsArrayBase, nsArray, and nsArrayCC.
(Assignee)

Comment 3

3 years ago
I can fix this unless Michael wants to.
Assignee: nobody → continuation
Flags: needinfo?(poirot.alex)
(In reply to Andrew McCreight [:mccr8] from comment #3)
> I can fix this unless Michael wants to.

Go ahead.
(Assignee)

Comment 5

3 years ago
Created attachment 8639107 [details] [diff] [review]
Split out a refcountless base class for nsScriptError.

This avoids shadowing the refcount if nsScriptErrorWithScript inherited from nsScriptError.

Mostly just a lot of search and replace.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=85a79af5458b
Attachment #8639107 - Flags: review?(gkrizsanits)
Comment on attachment 8639107 [details] [diff] [review]
Split out a refcountless base class for nsScriptError.

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

LGTM
Attachment #8639107 - Flags: review?(gkrizsanits) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/20afa46d73c1
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.