Bug 1433056: Account for pres shell destruction. r?smaug
The underlying issue here is that the pres shell was already being torn down
when we flush.
The less risky change here is to just change the assertions instead of
IsSafeToFlush too, but the !isSafeToFlush || mViewManager assertion was also
firing already, because we null out the view manager from Destroy.
So this change is effectively idempotent behavior-wise (we tighten a bit the
view manager assertion, but that should be fine).
Just let me know if you want me to just loosen the assertions, but I think the
IsSafeToFlush change is nicer too.
MozReview-Commit-ID: 240qLlLi7RE
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -4057,18 +4057,19 @@ PresShell::HandlePostedReflowCallbacks(b
if (shouldFlush && !mIsDestroying) {
FlushPendingNotifications(flushType);
}
}
bool
nsIPresShell::IsSafeToFlush() const
{
- // Not safe if we are reflowing or in the middle of frame construction
- if (mIsReflowing || mChangeNestCount) {
+ // Not safe if we are getting torn down, reflowing, or in the middle of frame
+ // construction.
+ if (mIsReflowing || mChangeNestCount || mIsDestroying) {
return false;
}
// Not safe if we are painting
if (nsViewManager* viewManager = GetViewManager()) {
bool isPainting = false;
viewManager->IsPainting(isPainting);
if (isPainting) {
@@ -4097,18 +4098,16 @@ PresShell::DoFlushPendingNotifications(m
/**
* VERY IMPORTANT: If you add some sort of new flushing to this
* method, make sure to add the relevant SetNeedLayoutFlush or
* SetNeedStyleFlush calls on the shell.
*/
FlushType flushType = aFlush.mFlushType;
MOZ_ASSERT(NeedFlush(flushType), "Why did we get called?");
- MOZ_DIAGNOSTIC_ASSERT(mDocument->HasShellOrBFCacheEntry());
- MOZ_DIAGNOSTIC_ASSERT(mDocument->GetShell() == this);
#ifdef MOZ_GECKO_PROFILER
static const EnumeratedArray<FlushType,
FlushType::Count,
const char*> flushTypeNames = {
"",
"Event",
"Content",
@@ -4148,22 +4147,26 @@ PresShell::DoFlushPendingNotifications(m
// If layout could possibly trigger scripts, then it's only safe to flush if
// it's safe to run script.
bool hasHadScriptObject;
if (mDocument->GetScriptHandlingObject(hasHadScriptObject) ||
hasHadScriptObject) {
isSafeToFlush = isSafeToFlush && nsContentUtils::IsSafeToRunScript();
}
- NS_ASSERTION(!isSafeToFlush || mViewManager, "Must have view manager");
+ MOZ_DIAGNOSTIC_ASSERT(!mIsDestroying || !isSafeToFlush);
+ MOZ_DIAGNOSTIC_ASSERT(mIsDestroying || mViewManager);
+ MOZ_DIAGNOSTIC_ASSERT(mIsDestroying || mDocument->HasShellOrBFCacheEntry());
+ MOZ_DIAGNOSTIC_ASSERT(mIsDestroying || mDocument->GetShell() == this);
+
// Make sure the view manager stays alive.
RefPtr<nsViewManager> viewManager = mViewManager;
bool didStyleFlush = false;
bool didLayoutFlush = false;
- if (isSafeToFlush && viewManager) {
+ if (isSafeToFlush) {
// Record that we are in a flush, so that our optimization in
// nsDocument::FlushPendingNotifications doesn't skip any re-entrant
// calls to us. Otherwise, we might miss some needed flushes, since
// we clear mNeedStyleFlush / mNeedLayoutFlush here at the top of
// the function but we might not have done the work yet.
AutoRestore<bool> guard(mInFlush);
mInFlush = true;