1872155 - Avoid crashing in known reentrant paths for PresShell::DoFlushPendingNotifications

Status: NEW

(Core :: Layout, task)

Description

[Yannis Juglaret [:yannis]]

We currently have several bugs open (bug 1851430, bug 1851533, bug 1872154) on the mozilla::PresShell::DoFlushPendingNotifications signature, which are caused by the same MOZ_DIAGNOSTIC_ASSERT(!mForbiddenToFlush, "This is bad!") but through different paths. These bugs are not assigned a high prority when as far as we know these specific reentrancy paths do not have a bad impact in release. However keeping the MOZ_DIAGNOSTIC_ASSERT on these paths is not desirable for at least two reasons.

First, Nightly and Beta users are a precious population, and we don't want these users to move away to Release because we voluntarily keep known crashes around on Nightly and Beta. Keeping the MOZ_DIAGNOSTIC_ASSERT for these paths contradicts the following recommendation from Assertions.h:

/*
 * MOZ_DIAGNOSTIC_ASSERT works like MOZ_RELEASE_ASSERT in Nightly and early beta
 * and MOZ_ASSERT in late Beta and Release - use this when a condition is
 * potentially rare enough to require real user testing to hit, but is not
 * security-sensitive. This can cause user pain, so use it sparingly. If a
 * MOZ_DIAGNOSTIC_ASSERT is firing, it should promptly be converted to a
 * MOZ_ASSERT while the failure is being investigated, rather than letting users
 * suffer.
 */

Second, keeping the MOZ_DIAGNOSTIC_ASSERT on these paths makes us late to notice new volume caused by new reentrancies when it emerges, because the new volume gets mixed with the volume from the existing bugs. For example, we were late to notice bug 1872154.

Therefore I suggest that we remove the MOZ_DIAGNOSTIC_ASSERT on current and future known crashing paths, and we restore it as we fix the reentrancy for each path.

Comment 1

[Yannis Juglaret [:yannis]]

Attachment: Bug 1872155 - Avoid crashing in known reentrant paths for PresShell::DoFlushPendingNotifications. r=#layout-reviewers

We currently have multiple paths that are known to cause a reentrancy in
PresShell::DoFlushPendingNotifications. This patch proposes that once we
have identified a reentrancy path, we stop crashing on this path, until
we are done investigating and removing the reentrancy.

To achieve this, we extend the ChangesToFlush structure so that it
embeds information about whether we are on a path that is known to cause
such reentrancy. This allows us to avoid crashing on these paths.