984780 - mozilla::css::URLValue and ImageValue and have iffy refcounting

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

From code inspection, noticed several problems with URLValue and ImageValue:
 http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSValue.h#72

* URLValue defines AddRef/Release and has a non-virtual destructor, which means that it's a bit sketchy to have a class that inherits from it. (since Release() on a nsRefPtr<URLValue> could in theory be invoking the wrong destructor, if the underlying variable is actually an instance of the subclass)

* ImageValue has its own NS_INLINE_DECL_REFCOUNTING() invocation, meaning it gets its own  nsAutoRefCnt mRefCnt variable (in addition to the one it gets from URLValue) So it has two reference counts.

Comment 1

[Daniel Holbert [:dholbert]]

I believe neither of these issues are *actually* causing problems right now, since (from a brief search) we don't seem to ever stick an ImageValue in a nsRefPtr<URLValue>.

But they are footguns and should be fixed.

Comment 2

[Daniel Holbert [:dholbert]]

Bug 1158540 seems to be improving things here, and may end up fixing this (perhaps by using a new non-refcounted base class per bug 1158540 comment 7).