1168154 - Mark refcounted members of nsCSSValue::mValue as MOZ_OWNING_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=761267c360be

Comment 1

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

Attachment: Mark refcounted members of nsCSSValue::mValue as MOZ_OWNING_REF

Comment 2

[Cameron McCormack (:heycam)]

Comment on attachment 8610181 [details] [diff] [review]
Mark refcounted members of nsCSSValue::mValue as MOZ_OWNING_REF

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

Does it make sense to put MOZ_NON_OWNING_REF on m{,Pair}ListDependent, since they're owned by other objects outside nsCSSValue?

Comment 3

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

I don't think that it makes sense to do that, as, unless I'm wrong, m{,Pair}ListDependent aren't reference counted values, and MOZ_NON_OWNING_REF is meant to talk about a pattern of dealing with weak references to reference counted values, rather than just references to things which we don't have direct ownership over.

Comment 4

[Cameron McCormack (:heycam)]

(In reply to Michael Layzell [:mystor] from comment #3)
> I don't think that it makes sense to do that, as, unless I'm wrong,
> m{,Pair}ListDependent aren't reference counted values, and
> MOZ_NON_OWNING_REF is meant to talk about a pattern of dealing with weak
> references to reference counted values, rather than just references to
> things which we don't have direct ownership over.

OK.  (The comments in mfbt/Attributes.h about these attributes don't mention anything about them being for pointers to refcounted things.)

Comment 5

[Pulsebot]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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