Open Bug 1723836 Opened 3 years ago Updated 4 months ago

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

Categories

(Core :: Layout, task)

task

Tracking

()

People

(Reporter: dholbert, Unassigned)

References

(Depends on 1 open bug)

Details

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

Flags: needinfo?(emilio)

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

Flags: needinfo?(emilio)
Depends on: 1851430
Depends on: 1714290

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

Flags: needinfo?(emilio)

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

We've seen a couple others recently.

Depends on: 1851533
Flags: needinfo?(emilio)
Depends on: 1857945
Depends on: 1872154
See Also: → 1872155
You need to log in before you can comment on or make changes to this bug.