Mark the members of CSSParsingEnvironment MOZ_UNSAFE_REF

RESOLVED FIXED in Firefox 41

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Nika, Assigned: Nika)

Tracking

(Blocks: 1 bug)

unspecified
mozilla41
Points:
---

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Comment 1

3 years ago
Created attachment 8609363 [details] [diff] [review]
Made the members of CSSParsingEnvironment nsCOMPtrs
Component: Layout → CSS Parsing and Computation
(Assignee)

Updated

3 years ago
Blocks: 1114683
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.
(Assignee)

Comment 4

3 years ago
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
(Assignee)

Updated

3 years ago
Attachment #8610122 - Flags: review?(dbaron)
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+
(Assignee)

Comment 6

3 years ago
Created attachment 8623078 [details] [diff] [review]
Mark the members of CSSParsingEnvironment as MOZ_UNSAFE_REF
Attachment #8610122 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Assignee: nobody → michael
Keywords: checkin-needed
Summary: Made the members of CSSParsingEnvironment nsCOMPtrs → Mark the members of CSSParsingEnvironment MOZ_UNSAFE_REF
https://hg.mozilla.org/mozilla-central/rev/a1558d74b4db
Status: NEW → RESOLVED
Last Resolved: 3 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.