Closed
Bug 1235737
Opened 8 years ago
Closed 8 years ago
Change MOZ_CRASH in nsFlexContainerFrame switch statements to MOZ_ASSERT_UNREACHABLE
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
Details
Attachments
(1 file)
2.37 KB,
patch
|
MatsPalmgren_bugz
:
review+
|
Details | Diff | Splinter Review |
nsFlexContainerFrame has some MOZ_CRASH statements for handling unexpected (unrecognized) style-system enumerated values, in switch statements. These are bad, because they'll abort in release builds. Really, they should be MOZ_ASSERT_UNREACHABLE. HISTORY: ======== These were originally MOZ_NOT_REACHED(), which were intended as stronger forms of NS_NOTREACHED(), but it turned out that MOZ_NOT_REACHED was a dangerous footgun (bug 820686 comment 0), so it was converted to MOZ_CRASH as part of a mass-conversion here: https://hg.mozilla.org/mozilla-central/diff/55c1f447549d/layout/generic/nsFlexContainerFrame.cpp#l1573 And then I later copypasted the MOZ_CRASH into a new switch statement for "align-content" when adding support for that: https://hg.mozilla.org/mozilla-central/diff/47901ad6d20b/layout/generic/nsFlexContainerFrame.cpp#l1.176 Anyway -- there's no reason we need to abort in release builds if we somehow unexpectedly hit these lines -- these should just be MOZ_ASSERT_UNREACHABLE, which is what they wanted to be originally anyway (though that didn't exist at the time).
Assignee | ||
Comment 1•8 years ago
|
||
This patch does the following: 1) s/MOZ_CRASH/MOZ_ASSERT_UNREACHABLE/ 2) Fixes an incorrect assertion-message in one of the affected lines -- right now, it says "flex-flow" but means to say "flex-direction". (fixed here)
Attachment #8702815 -
Flags: review?(mats)
Comment 2•8 years ago
|
||
Comment on attachment 8702815 [details] [diff] [review] fix v1 r=mats
Attachment #8702815 -
Flags: review?(mats) → review+
Comment 3•8 years ago
|
||
FYI, there is also a NS_NOTREACHED("Unexpected align-self value") that could be fixed.
Assignee | ||
Comment 4•8 years ago
|
||
Thanks -- I'll upgrade that one to MOZ_ASSERT_UNREACHABLE for consistency with the rest of these.
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9059476e8881
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•