From 75d8c0a95066e07ccb6d425cfad21502f855d1eb Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Sat, 20 Apr 2013 00:01:41 +1200 Subject: [PATCH] Bug 862180. Part 1: Remove null checks on the result of nsDisplayItem::GetUnderlyingFrame(). r=mattwoodrow --- layout/base/FrameLayerBuilder.cpp | 39 +++++------ layout/base/nsDisplayList.cpp | 14 ++-- layout/base/nsLayoutUtils.cpp | 3 - layout/base/nsPresShell.cpp | 108 ++++++++++++++---------------- layout/generic/TextOverflow.cpp | 2 +- layout/generic/nsFrame.cpp | 4 +- layout/generic/nsPageFrame.cpp | 2 +- 7 files changed, 76 insertions(+), 96 deletions(-) diff --git a/layout/base/FrameLayerBuilder.cpp b/layout/base/FrameLayerBuilder.cpp index 29505dba5fdb..97fa51e323a3 100644 --- a/layout/base/FrameLayerBuilder.cpp +++ b/layout/base/FrameLayerBuilder.cpp @@ -1128,23 +1128,21 @@ FrameLayerBuilder::GetOldLayerFor(nsDisplayItem* aItem, uint32_t key = aItem->GetPerFrameKey(); nsIFrame* frame = aItem->GetUnderlyingFrame(); - if (frame) { - DisplayItemData* oldData = GetOldLayerForFrame(frame, key); - if (oldData) { - if (aOldGeometry) { - *aOldGeometry = oldData->mGeometry.get(); - } - if (aOldClip) { - *aOldClip = &oldData->mClip; - } - if (aChangedFrames) { - oldData->GetFrameListChanges(aItem, *aChangedFrames); - } - if (aIsInvalid) { - *aIsInvalid = oldData->mIsInvalid; - } - return oldData->mLayer; + DisplayItemData* oldData = GetOldLayerForFrame(frame, key); + if (oldData) { + if (aOldGeometry) { + *aOldGeometry = oldData->mGeometry.get(); } + if (aOldClip) { + *aOldClip = &oldData->mClip; + } + if (aChangedFrames) { + oldData->GetFrameListChanges(aItem, *aChangedFrames); + } + if (aIsInvalid) { + *aIsInvalid = oldData->mIsInvalid; + } + return oldData->mLayer; } return nullptr; @@ -2233,8 +2231,6 @@ ContainerState::InvalidateForLayerChange(nsDisplayItem* aItem, const nsPoint& aTopLeft, nsDisplayItemGeometry *aGeometry) { - NS_ASSERTION(aItem->GetUnderlyingFrame(), - "Display items that render using Thebes must have a frame"); NS_ASSERTION(aItem->GetPerFrameKey(), "Display items that render using Thebes must have a key"); nsDisplayItemGeometry *oldGeometry = NULL; @@ -2383,7 +2379,6 @@ FrameLayerBuilder::AddThebesDisplayItem(ThebesLayer* aLayer, if (entry->mContainerLayerGeneration == 0) { entry->mContainerLayerGeneration = mContainerLayerGeneration; } - NS_ASSERTION(aItem->GetUnderlyingFrame(), "Must have frame"); if (tempManager) { FrameLayerBuilder* layerBuilder = new FrameLayerBuilder(); layerBuilder->Init(mDisplayListBuilder, tempManager); @@ -2960,8 +2955,6 @@ Layer* FrameLayerBuilder::GetLeafLayerFor(nsDisplayListBuilder* aBuilder, nsDisplayItem* aItem) { - NS_ASSERTION(aItem->GetUnderlyingFrame(), - "Can only call GetLeafLayerFor on items that have a frame"); Layer* layer = GetOldLayerFor(aItem); if (!layer) return nullptr; @@ -3293,9 +3286,7 @@ FrameLayerBuilder::DrawThebesLayer(ThebesLayer* aLayer, PaintInactiveLayer(builder, cdi->mInactiveLayerManager, cdi->mItem, aContext, rc); } else { nsIFrame* frame = cdi->mItem->GetUnderlyingFrame(); - if (frame) { - frame->AddStateBits(NS_FRAME_PAINTED_THEBES); - } + frame->AddStateBits(NS_FRAME_PAINTED_THEBES); #ifdef MOZ_DUMP_PAINTING if (gfxUtils::sDumpPainting) { diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index 44a97a6bc450..9704cab29215 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -939,7 +939,7 @@ TreatAsOpaque(nsDisplayItem* aItem, nsDisplayListBuilder* aBuilder) // be treated as opaque (and their bounds is just the union of their // children, which might be a large area their contents don't really cover). nsIFrame* f = aItem->GetUnderlyingFrame(); - if (f && f->PresContext()->IsChrome() && !aItem->GetChildren() && + if (f->PresContext()->IsChrome() && !aItem->GetChildren() && f->StyleDisplay()->mOpacity != 0.0) { opaque = aItem->GetBounds(aBuilder, &snap); } @@ -1440,8 +1440,7 @@ static bool IsContentLEQ(nsDisplayItem* aItem1, nsDisplayItem* aItem2, static bool IsZOrderLEQ(nsDisplayItem* aItem1, nsDisplayItem* aItem2, void* aClosure) { - // These GetUnderlyingFrame calls return non-null because we're only used - // in sorting. Note that we can't just take the difference of the two + // Note that we can't just take the difference of the two // z-indices here, because that might overflow a 32-bit int. int32_t index1 = nsLayoutUtils::GetZIndex(aItem1->GetUnderlyingFrame()); int32_t index2 = nsLayoutUtils::GetZIndex(aItem2->GetUnderlyingFrame()); @@ -2649,11 +2648,10 @@ nsDisplayWrapList::RequiredLayerStateForChildren(nsDisplayListBuilder* aBuilder, LayerState result = LAYER_INACTIVE; for (nsDisplayItem* i = aList.GetBottom(); i; i = i->GetAbove()) { nsIFrame* f = i->GetUnderlyingFrame(); - if (f) { - nsIFrame* activeScrolledRoot = - nsLayoutUtils::GetActiveScrolledRootFor(f, nullptr); - if (activeScrolledRoot != aActiveScrolledRoot && result == LAYER_INACTIVE) - result = LAYER_ACTIVE; + nsIFrame* activeScrolledRoot = + nsLayoutUtils::GetActiveScrolledRootFor(f, nullptr); + if (activeScrolledRoot != aActiveScrolledRoot && result == LAYER_INACTIVE) { + result = LAYER_ACTIVE; } LayerState state = i->GetLayerState(aBuilder, aManager, aParameters); diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index c1127c775c50..c77e822505fc 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -1081,9 +1081,6 @@ nsLayoutUtils::GetActiveScrolledRootFor(nsDisplayItem* aItem, if (aShouldFixToViewport) { *aShouldFixToViewport = false; } - if (!f) { - return nullptr; - } if (aItem->ShouldFixToViewport(aBuilder)) { if (aShouldFixToViewport) { *aShouldFixToViewport = true; diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index bfbce3512efe..ce66f2f817b9 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -4493,56 +4493,54 @@ PresShell::ClipListToRange(nsDisplayListBuilder *aBuilder, // temporary list. If null, no item should be inserted. nsDisplayItem* itemToInsert = nullptr; nsIFrame* frame = i->GetUnderlyingFrame(); - if (frame) { - nsIContent* content = frame->GetContent(); - if (content) { - bool atStart = (content == aRange->GetStartParent()); - bool atEnd = (content == aRange->GetEndParent()); - if ((atStart || atEnd) && frame->GetType() == nsGkAtoms::textFrame) { - int32_t frameStartOffset, frameEndOffset; - frame->GetOffsets(frameStartOffset, frameEndOffset); + nsIContent* content = frame->GetContent(); + if (content) { + bool atStart = (content == aRange->GetStartParent()); + bool atEnd = (content == aRange->GetEndParent()); + if ((atStart || atEnd) && frame->GetType() == nsGkAtoms::textFrame) { + int32_t frameStartOffset, frameEndOffset; + frame->GetOffsets(frameStartOffset, frameEndOffset); - int32_t hilightStart = - atStart ? std::max(aRange->StartOffset(), frameStartOffset) : frameStartOffset; - int32_t hilightEnd = - atEnd ? std::min(aRange->EndOffset(), frameEndOffset) : frameEndOffset; - if (hilightStart < hilightEnd) { - // determine the location of the start and end edges of the range. - nsPoint startPoint, endPoint; - frame->GetPointFromOffset(hilightStart, &startPoint); - frame->GetPointFromOffset(hilightEnd, &endPoint); + int32_t hilightStart = + atStart ? std::max(aRange->StartOffset(), frameStartOffset) : frameStartOffset; + int32_t hilightEnd = + atEnd ? std::min(aRange->EndOffset(), frameEndOffset) : frameEndOffset; + if (hilightStart < hilightEnd) { + // determine the location of the start and end edges of the range. + nsPoint startPoint, endPoint; + frame->GetPointFromOffset(hilightStart, &startPoint); + frame->GetPointFromOffset(hilightEnd, &endPoint); - // the clip rectangle is determined by taking the the start and - // end points of the range, offset from the reference frame. - // Because of rtl, the end point may be to the left of the - // start point, so x is set to the lowest value - nsRect textRect(aBuilder->ToReferenceFrame(frame), frame->GetSize()); - nscoord x = std::min(startPoint.x, endPoint.x); - textRect.x += x; - textRect.width = std::max(startPoint.x, endPoint.x) - x; - surfaceRect.UnionRect(surfaceRect, textRect); + // the clip rectangle is determined by taking the the start and + // end points of the range, offset from the reference frame. + // Because of rtl, the end point may be to the left of the + // start point, so x is set to the lowest value + nsRect textRect(aBuilder->ToReferenceFrame(frame), frame->GetSize()); + nscoord x = std::min(startPoint.x, endPoint.x); + textRect.x += x; + textRect.width = std::max(startPoint.x, endPoint.x) - x; + surfaceRect.UnionRect(surfaceRect, textRect); - DisplayItemClip newClip; - newClip.SetTo(textRect); - newClip.IntersectWith(i->GetClip()); - i->SetClip(aBuilder, newClip); - itemToInsert = i; - } + DisplayItemClip newClip; + newClip.SetTo(textRect); + newClip.IntersectWith(i->GetClip()); + i->SetClip(aBuilder, newClip); + itemToInsert = i; } - // Don't try to descend into subdocuments. - // If this ever changes we'd need to add handling for subdocuments with - // different zoom levels. - else if (content->GetCurrentDoc() == - aRange->GetStartParent()->GetCurrentDoc()) { - // if the node is within the range, append it to the temporary list - bool before, after; - nsresult rv = - nsRange::CompareNodeToRange(content, aRange, &before, &after); - if (NS_SUCCEEDED(rv) && !before && !after) { - itemToInsert = i; - bool snap; - surfaceRect.UnionRect(surfaceRect, i->GetBounds(aBuilder, &snap)); - } + } + // Don't try to descend into subdocuments. + // If this ever changes we'd need to add handling for subdocuments with + // different zoom levels. + else if (content->GetCurrentDoc() == + aRange->GetStartParent()->GetCurrentDoc()) { + // if the node is within the range, append it to the temporary list + bool before, after; + nsresult rv = + nsRange::CompareNodeToRange(content, aRange, &before, &after); + if (NS_SUCCEEDED(rv) && !before && !after) { + itemToInsert = i; + bool snap; + surfaceRect.UnionRect(surfaceRect, i->GetBounds(aBuilder, &snap)); } } } @@ -5282,16 +5280,14 @@ PresShell::MarkImagesInListVisible(const nsDisplayList& aList) nsIFrame* f = item->GetUnderlyingFrame(); // We could check the type of the display item, only a handful can hold an // image loading content. - if (f) { - // dont bother nscomptr here, it is wasteful - nsCOMPtr content(do_QueryInterface(f->GetContent())); - if (content) { - // use the presshell containing the image - PresShell* presShell = static_cast(f->PresContext()->PresShell()); - if (presShell) { - content->IncrementVisibleCount(); - presShell->mVisibleImages.AppendElement(content); - } + // dont bother nscomptr here, it is wasteful + nsCOMPtr content(do_QueryInterface(f->GetContent())); + if (content) { + // use the presshell containing the image + PresShell* presShell = static_cast(f->PresContext()->PresShell()); + if (presShell) { + content->IncrementVisibleCount(); + presShell->mVisibleImages.AppendElement(content); } } } diff --git a/layout/generic/TextOverflow.cpp b/layout/generic/TextOverflow.cpp index 45dd53d926a3..f6f87890091b 100644 --- a/layout/generic/TextOverflow.cpp +++ b/layout/generic/TextOverflow.cpp @@ -627,7 +627,7 @@ TextOverflow::PruneDisplayListContents(nsDisplayList* aList, nsDisplayItem* item; while ((item = aList->RemoveBottom())) { nsIFrame* itemFrame = item->GetUnderlyingFrame(); - if (itemFrame && IsFrameDescendantOfAny(itemFrame, aFramesToHide, mBlock)) { + if (IsFrameDescendantOfAny(itemFrame, aFramesToHide, mBlock)) { item->~nsDisplayItem(); continue; } diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index e458d7d430ac..4ef0718dab76 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -1673,8 +1673,7 @@ WrapPreserve3DListInternal(nsIFrame* aFrame, nsDisplayListBuilder *aBuilder, nsD // and then flush this list into aOutput by wrapping the whole lot with a single // nsDisplayTransform. - if (childFrame && - childFrame->GetParent() && + if (childFrame->GetParent() && (childFrame->GetParent()->Preserves3DChildren() || childFrame == aFrame)) { switch (item->GetType()) { case nsDisplayItem::TYPE_TRANSFORM: { @@ -1869,7 +1868,6 @@ nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder* aBuilder, nsDisplayItem* item = set.PositionedDescendants()->GetBottom(); if (item) { nsIFrame* f = item->GetUnderlyingFrame(); - NS_ASSERTION(f, "After sorting, every item in the list should have an underlying frame"); if (nsLayoutUtils::GetZIndex(f) < 0) { set.PositionedDescendants()->RemoveBottom(); resultList.AppendToTop(item); diff --git a/layout/generic/nsPageFrame.cpp b/layout/generic/nsPageFrame.cpp index 5f0d43da928e..389db37e2da7 100644 --- a/layout/generic/nsPageFrame.cpp +++ b/layout/generic/nsPageFrame.cpp @@ -421,7 +421,7 @@ PruneDisplayListForExtraPage(nsDisplayListBuilder* aBuilder, i->UpdateBounds(aBuilder); } else { nsIFrame* f = i->GetUnderlyingFrame(); - if (!f || !nsLayoutUtils::IsProperAncestorFrameCrossDoc(aPage, f)) { + if (!nsLayoutUtils::IsProperAncestorFrameCrossDoc(aPage, f)) { // We're throwing this away so call its destructor now. The memory // is owned by aBuilder which destroys all items at once. i->~nsDisplayItem();