Created attachment 8609363 [details] [diff] [review] Made the members of CSSParsingEnvironment nsCOMPtrs
4 years ago
Component: Layout → CSS Parsing and Computation
Why make this change?
These are weak pointers for a very good reason, iirc: this is used in very performance sensitive code and the refcounting was showing up quite noticeably in profiles.
Created attachment 8610122 [details] [diff] [review] Mark the members of CSSParsingEnvironment as MOZ_UNSAFE_REF I've changed the patch to instead mark the members as MOZ_UNSAFE_REF, and reference the original bug which made the change (Bug 649163)
Attachment #8609363 - Attachment is obsolete: true
Comment on attachment 8610122 [details] [diff] [review] Mark the members of CSSParsingEnvironment as MOZ_UNSAFE_REF Now that MOZ_NON_OWNING_REF and MOZ_UNSAFE_REF are basically the same thing underneath, I'm less confused. (Although the commit message of 229b03af6f2b was a little confusing in saying it was changing only documentation.) It's probably worth mentioning in the comment that the user of CSSParsingEnvironment is responsible for maintaining an owning reference for the lifetime of the CSSParsingEnvironment. The comment could probably otherwise be simplified, perhaps to: MOZ_UNSAFE_REF("user of CSSParsingEnviroment must hold an owning " "reference; reference counting here has unacceptable " "performance overhead (see bug 649163)") Also, please don't include the URL for the bug in the commit message; just include the bug number. r=dbaron with that or similar But please change the summary of the bug as well to match what you're doing!
Attachment #8610122 - Flags: review?(dbaron) → review+
Created attachment 8623078 [details] [diff] [review] Mark the members of CSSParsingEnvironment as MOZ_UNSAFE_REF
Attachment #8610122 - Attachment is obsolete: true
Assignee: nobody → michael
Summary: Made the members of CSSParsingEnvironment nsCOMPtrs → Mark the members of CSSParsingEnvironment MOZ_UNSAFE_REF
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.