1167589 - Mark the members of CSSParsingEnvironment MOZ_UNSAFE_REF

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Nika Layzell [:nika] (ni? for response)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae29b2b77ed8

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Made the members of CSSParsingEnvironment nsCOMPtrs

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Why make this change?

Comment 3

[Boris Zbarsky [:bzbarsky]]

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.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: 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)

Comment 5

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

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!

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Mark the members of CSSParsingEnvironment as MOZ_UNSAFE_REF

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a1558d74b4db

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a1558d74b4db