Bug 1451384 - Check IsChanged on the old item during merging, since that's the one that might have a deleted frame. r?mstange
This happens when an nsIFrame* that builds an nsDisplayWrapList is deleted, but then the memory is immediately reused for another frame that builds the same type display item, within the same display list.
PreProcessDisplayLists chooses not to descend into the nsDisplayWrapList for the deleted frame, and so mOldItems remains uninitialized for the old sublist.
When adding the new instance, IsChanged returns false, since the pointers are the same, and we're checking HasDeletedFrame on the new instance (where it's never true), instead of the old. We then recurse into MergeDisplayLists, with an uninitialized mOldItems array, and crash.
I haven't added a test because I haven't yet figured out how to create a minimal testcase, and the test would rely on implementation details of the frame allocator to remain unchanged to be useful.
MozReview-Commit-ID: pHimEvfAND
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -122,16 +122,17 @@ RetainedDisplayListBuilder::PreProcessDi
aList->mDAG.mDirectPredecessorList.Length() >
(aList->mDAG.mNodesInfo.Length() * kMaxEdgeRatio)) {
return false;
}
nsDisplayList saved;
aList->mOldItems.SetCapacity(aList->Count());
+ MOZ_ASSERT(aList->mOldItems.IsEmpty());
size_t i = 0;
while (nsDisplayItem* item = aList->RemoveBottom()) {
if (item->HasDeletedFrame() || !item->CanBeReused()) {
// If we haven't yet initialized the DAG, then we can
// just drop this item. Otherwise we need to keep it
// around to preserve indices, and merging will
// get rid of it.
if (initializeDAG) {
@@ -171,16 +172,17 @@ RetainedDisplayListBuilder::PreProcessDi
mBuilder.MarkFrameForDisplayIfVisible(f, mBuilder.RootReferenceFrame());
}
// TODO: This is here because we sometimes reuse the previous display list
// completely. For optimization, we could only restore the state for reused
// display items.
item->RestoreState();
}
+ MOZ_ASSERT(aList->mOldItems.Length() == aList->mDAG.Length());
aList->RestoreState();
return true;
}
void
RetainedDisplayListBuilder::IncrementSubDocPresShellPaintCount(nsDisplayItem* aItem)
{
MOZ_ASSERT(aItem->GetType() == DisplayItemType::TYPE_SUBDOCUMENT);
@@ -252,22 +254,23 @@ public:
{
mOldKeyLookup.SwapElements(aOldList.mKeyLookup);
mMergedDAG.EnsureCapacityFor(mOldDAG);
}
MergedListIndex ProcessItemFromNewList(nsDisplayItem* aNewItem, const Maybe<MergedListIndex>& aPreviousItem) {
OldListIndex oldIndex;
if (mOldKeyLookup.Get({ aNewItem->Frame(), aNewItem->GetPerFrameKey() }, &oldIndex.val)) {
- if (!IsChanged(aNewItem)) {
+ nsDisplayItem* oldItem = mOldItems[oldIndex.val].mItem;
+ if (oldItem && !IsChanged(oldItem)) {
MOZ_ASSERT(!mOldItems[oldIndex.val].IsUsed());
if (aNewItem->GetChildren()) {
Maybe<const ActiveScrolledRoot*> containerASRForChildren;
if (mBuilder->MergeDisplayLists(aNewItem->GetChildren(),
- mOldItems[oldIndex.val].mItem->GetChildren(),
+ oldItem->GetChildren(),
aNewItem->GetChildren(),
containerASRForChildren)) {
mResultIsModified = true;
}
UpdateASR(aNewItem, containerASRForChildren);
aNewItem->UpdateBounds(mBuilder->Builder());
}
--- a/layout/tools/reftest/runreftest.py
+++ b/layout/tools/reftest/runreftest.py
@@ -234,17 +234,17 @@ class RefTest(object):
update_mozinfo()
self.lastTestSeen = None
self.haveDumpedScreen = False
self.resolver = self.resolver_cls()
self.log = None
self.outputHandler = None
self.testDumpFile = os.path.join(tempfile.gettempdir(), 'reftests.json')
- self.run_by_manifest = True
+ self.run_by_manifest = False
if suite in ('crashtest', 'jstestbrowser'):
self.run_by_manifest = False
def _populate_logger(self, options):
if self.log:
return
self.log = getattr(options, 'log', None)