Mark nsPresContext::mMedium as 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, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8610206 [details] [diff] [review]
Mark nsPresContext::mMedium as MOZ_UNSAFE_REF
(Assignee)

Updated

3 years ago
Attachment #8610206 - Flags: review?(dbaron)
(Assignee)

Updated

3 years ago
Blocks: 1114683
I guess I don't really understand (from the comments in Attributes.h) how MOZ_UNSAFE_REF differs from MOZ_NON_OWNING_REF (or, for that matter, MOZ_OWNING_REF, which it appears to actually be more similar to).
Flags: needinfo?(michael)
(Assignee)

Comment 3

3 years ago
MOZ_NON_OWNING_REF is referring to the pattern where a raw pointer is set up such that before the value it is pointing to is invalidated, the pointer has been nulled out. This is often done in the destructor of the pointed-at value.

MOZ_OWNING_REF is referring to the pattern where manual AddRefs and Releases are used to manage the lifetime of an object. Something which is MOZ_OWNING_REF would usually be a smart pointer, except that there are reasons why it cannot be one (for example, a MOZ_OWNING_REF could exist inside of a union, or in another place where a POD object is required).

MOZ_UNSAFE_REF is for everything else which needs to be handled. For example, in this situation mMedium is MOZ_UNSAFE_REF because it is never AddRef/Released, nor is it nulled out before it's target becomes invalid, but rather it's target is guaranteed to always be valid because it is a static atom.
Flags: needinfo?(michael)
(Assignee)

Updated

3 years ago
Assignee: nobody → michael
Comment on attachment 8610206 [details] [diff] [review]
Mark nsPresContext::mMedium as MOZ_UNSAFE_REF

Please leave the comment "//initialized by subclass ctors".

You could also shorten it to:
  MOZ_UNSAFE_REF("always a static atom")
("mMedium is only ever assigned" doesn't really add anything, and
the example might not remain true in the future)

Please include the bug number in the commit message.

r=dbaron with that
Attachment #8610206 - Flags: review?(dbaron) → review+
(Assignee)

Comment 5

3 years ago
Created attachment 8623076 [details] [diff] [review]
Mark nsPresContext::mMedium as MOZ_UNSAFE_REF
Attachment #8610206 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/57f67506a354
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.