Closed
Bug 1222754
Opened 9 years ago
Closed 8 years ago
Replace nsSecureBrowserUIImpl::mOnStateLocationChangeReentranceDetection and related classes
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: Cykesiopka, Assigned: Cykesiopka)
References
Details
(Whiteboard: [psm-assigned])
Attachments
(1 file)
(David Keeler [:keeler] (use needinfo?) from Bug 1110935 comment #6) > ::: security/manager/ssl/nsSecureBrowserUIImpl.h:81 > (Diff revision 1) > > mozilla::Atomic<int32_t> mOnStateLocationChangeReentranceDetection; > > I believe this doesn't need to be atomic any more (since it's not > multi-threaded any more). It's also unclear to me that this is necessary at > all. These are probably changes/investigations for another bug, though.
Whiteboard: [psm-cleanup]
Assignee | ||
Comment 1•8 years ago
|
||
nsSecureBrowserUIImpl is simultaneously critical to security UI, and a mess. So, maybe the safer option here is to just replace mOnStateLocationChangeReentranceDetection with mozilla::ReentrancyGuard for now, and remove the guard if/when nsSecureBrowserUIImpl isn't a mess.
Assignee: nobody → cykesiopka.bmo
Status: NEW → ASSIGNED
Summary: Investigate whether mOnStateLocationChangeReentranceDetection is still necessary → Replace nsSecureBrowserUIImpl::mOnStateLocationChangeReentranceDetection and related classes
Whiteboard: [psm-cleanup] → [psm-assigned]
Assignee | ||
Comment 2•8 years ago
|
||
mOnStateLocationChangeReentranceDetection and nsAutoAtomic form an unnecessarily threadsafe reentrance prevention mechanism that can be replaced by mozilla::ReentrancyGuard. Review commit: https://reviewboard.mozilla.org/r/54160/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54160/
Attachment #8754669 -
Flags: review?(dkeeler)
Comment on attachment 8754669 [details] MozReview Request: Bug 1222754 - Replace nsSecureBrowserUIImpl::mOnStateLocationChangeReentranceDetection and nsAutoAtomic. https://reviewboard.mozilla.org/r/54160/#review51026 Neat - r=me.
Attachment #8754669 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 4•8 years ago
|
||
Thanks! https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d0b230605dada321e90bb2f0678c5f0bbd71374 (AFAICT, all that orange is due to Bug 1270962, and other misc intermittent failures.)
Keywords: checkin-needed
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2250f9d1fa6d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•