1723836 - use release assertion for PresShell.cpp's MOZ_DIAGNOSTIC_ASSERT(!mForbiddenToFlush, "This is bad!")

Status: NEW

(Core :: Layout, task)

Description

[Daniel Holbert [:dholbert]]

Noticed this while triaging bug 1719483 (which is a case where this assertion fails):

void PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush aFlush) {
  // FIXME(emilio, bug 1530177): Turn into a release assert when bug 1530188 and
  // bug 1530190 are fixed.
  MOZ_DIAGNOSTIC_ASSERT(!mForbiddenToFlush, "This is bad!");

All three bugs mentioned there are closed. Perhaps we can/should take action on the FIXME now and upgrade the assertion to a release assert?

(We do know of at least one way to make it fail -- bug 1719483 -- but in that case, release builds crash anyway, so there's no harm in crashing slightly sooner via this hypothetically-promoted release assertion.)

Searchfox link: https://searchfox.org/mozilla-central/rev/064a1e6a2a6f6aa30be8bf4edea2f8408f779d4d/layout/base/PresShell.cpp#4079-4082

Comment 1

[Emilio Cobos Álvarez [:emilio]]

There's also bug 1714290 which happens only with a11y enabled.

Comment 2

[Andrew McCreight [:mccr8]]

Has this been fairly stable for a few years? Maybe it is time to consider upgrading this assert again?

Comment 3

[Andrew McCreight [:mccr8]]

If not, maybe the comment should be updated to point to this bug and remove the reference to the three closed bugs.

Comment 4

[Emilio Cobos Álvarez [:emilio]]

We've seen a couple others recently.

Comment 5

[Alexandre Poirot [:ochameau]]

FYI, I've hit this assertion consistantly when modifying some devtools logic involving a reflow listener (bug 2085888 comment 3).

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Well, yeah, running random things from those reflow observers is not great. Let me look at that.