Bug 1404181 - Part 22: Make sure we mark frames as modified any time they change position or style data and make sure we don't accidentally mark the root as being modified when we don't need to. r?mstange draft
authorMatt Woodrow <mwoodrow@mozilla.com>, Miko Mynttinen <mikokm@gmail.com>, Timothy Nikkel <tnikkel@gmail.com>
Fri, 29 Sep 2017 10:51:49 +1300
changeset 684533 87b05d8c0b258eba2b31b37590d4708e6ec87041
parent 684532 05e49a0a883e798b4388e05d9722c0d3703c8c1b
child 684534 5bb9062e8603968387f2dfe3cb65eeee032989ab
push id85633
push usermwoodrow@mozilla.com
push dateSun, 22 Oct 2017 23:03:02 +0000
reviewersmstange
bugs1404181
milestone58.0a1
Bug 1404181 - Part 22: Make sure we mark frames as modified any time they change position or style data and make sure we don't accidentally mark the root as being modified when we don't need to. r?mstange MozReview-Commit-ID: J5ov5cwvvrE
dom/animation/KeyframeEffectReadOnly.cpp
gfx/layers/apz/util/APZCCallbackHelper.cpp
layout/base/PresShell.cpp
layout/base/RestyleManager.cpp
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
layout/mathml/nsMathMLmactionFrame.cpp
layout/painting/FrameLayerBuilder.cpp
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -970,16 +970,21 @@ KeyframeEffectReadOnly::UpdateTargetRegi
              "Out of date Animation::IsRelevant value");
 
   if (isRelevant && !mInEffectSet) {
     EffectSet* effectSet =
       EffectSet::GetOrCreateEffectSet(mTarget->mElement, mTarget->mPseudoType);
     effectSet->AddEffect(*this);
     mInEffectSet = true;
     UpdateEffectSet(effectSet);
+    nsIFrame* f = mTarget->mElement->GetPrimaryFrame();
+    while (f) {
+      f->MarkNeedsDisplayItemRebuild();
+      f = f->GetNextContinuation();
+    }
   } else if (!isRelevant && mInEffectSet) {
     UnregisterTarget();
   }
 }
 
 void
 KeyframeEffectReadOnly::UnregisterTarget()
 {
@@ -994,16 +999,21 @@ KeyframeEffectReadOnly::UnregisterTarget
   mInEffectSet = false;
   if (effectSet) {
     effectSet->RemoveEffect(*this);
 
     if (effectSet->IsEmpty()) {
       EffectSet::DestroyEffectSet(mTarget->mElement, mTarget->mPseudoType);
     }
   }
+  nsIFrame* f = mTarget->mElement->GetPrimaryFrame();
+  while (f) {
+    f->MarkNeedsDisplayItemRebuild();
+    f = f->GetNextContinuation();
+  }
 }
 
 void
 KeyframeEffectReadOnly::RequestRestyle(
   EffectCompositor::RestyleType aRestyleType)
 {
    if (!mTarget) {
     return;
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ -892,17 +892,17 @@ APZCCallbackHelper::NotifyFlushComplete(
   MOZ_ASSERT(NS_IsMainThread());
   // In some cases, flushing the APZ state to the main thread doesn't actually
   // trigger a flush and repaint (this is an intentional optimization - the stuff
   // visible to the user is still correct). However, reftests update their
   // snapshot based on invalidation events that are emitted during paints,
   // so we ensure that we kick off a paint when an APZ flush is done. Note that
   // only chrome/testing code can trigger this behaviour.
   if (aShell && aShell->GetRootFrame()) {
-    aShell->GetRootFrame()->SchedulePaint();
+    aShell->GetRootFrame()->SchedulePaint(nsIFrame::PAINT_DEFAULT, false);
   }
 
   nsCOMPtr<nsIObserverService> observerService = mozilla::services::GetObserverService();
   MOZ_ASSERT(observerService);
   observerService->NotifyObservers(nullptr, "apz-repaints-flushed", nullptr);
 }
 
 static int32_t sActiveSuppressDisplayport = 0;
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -6784,17 +6784,17 @@ PresShell::HandleEvent(nsIFrame* aFrame,
 
   NS_ASSERTION(aFrame, "aFrame should be not null");
 
   // Update the latest focus sequence number with this new sequence number
   if (mAPZFocusSequenceNumber < aEvent->mFocusSequenceNumber) {
     mAPZFocusSequenceNumber = aEvent->mFocusSequenceNumber;
 
     // Schedule an empty transaction to transmit this focus update
-    aFrame->SchedulePaint(nsIFrame::PAINT_COMPOSITE_ONLY);
+    aFrame->SchedulePaint(nsIFrame::PAINT_COMPOSITE_ONLY, false);
   }
 
   if (PointerEventHandler::IsPointerEventEnabled()) {
     AutoWeakFrame weakFrame(aFrame);
     nsCOMPtr<nsIContent> targetContent;
     PointerEventHandler::DispatchPointerFromMouseOrTouch(
                            this, aFrame, aEvent, aDontRetargetEvents,
                            aEventStatus, getter_AddRefs(targetContent));
@@ -8832,22 +8832,21 @@ PresShell::DoReflow(nsIFrame* target, bo
   gfxTextPerfMetrics* tp = mPresContext->GetTextPerfMetrics();
   TimeStamp timeStart;
   if (tp) {
     tp->Accumulate();
     tp->reflowCount++;
     timeStart = TimeStamp::Now();
   }
 
-  target->SchedulePaint();
-  nsIFrame *parent = nsLayoutUtils::GetCrossDocParentFrame(target);
-  while (parent) {
-    SVGObserverUtils::InvalidateDirectRenderingObservers(parent);
-    parent = nsLayoutUtils::GetCrossDocParentFrame(parent);
-  }
+  // Schedule a paint, but don't actually mark this frame as changed for
+  // retained DL building purposes. If any child frames get moved, then
+  // they will schedule paint again. We could probaby skip this, and just
+  // schedule a similar paint when a frame is deleted.
+  target->SchedulePaint(nsIFrame::PAINT_DEFAULT, false);
 
   nsIURI* uri = mDocument->GetDocumentURI();
   nsCString uriString = uri ? uri->GetSpecOrDefault() : NS_LITERAL_CSTRING("N/A");
   AUTO_PROFILER_LABEL_DYNAMIC("PresShell::DoReflow", GRAPHICS, uriString.get());
 
   nsDocShell* docShell = static_cast<nsDocShell*>(GetPresContext()->GetDocShell());
   RefPtr<TimelineConsumers> timelines = TimelineConsumers::Get();
   bool isTimelineRecording = timelines && timelines->HasConsumer(docShell);
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1556,16 +1556,17 @@ RestyleManager::ProcessRestyledFrames(ns
           SVGObserverUtils::UpdateEffects(cont);
         }
       }
       if ((hint & nsChangeHint_InvalidateRenderingObservers) ||
           ((hint & nsChangeHint_UpdateOpacityLayer) &&
            frame->IsFrameOfType(nsIFrame::eSVG) &&
            !(frame->GetStateBits() & NS_STATE_IS_OUTER_SVG))) {
         SVGObserverUtils::InvalidateRenderingObservers(frame);
+        frame->SchedulePaint();
       }
       if (hint & nsChangeHint_NeedReflow) {
         StyleChangeReflow(frame, hint);
         didReflowThisFrame = true;
       }
 
       // Here we need to propagate repaint frame change hint instead of update
       // opacity layer change hint when we do opacity optimization for SVG.
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1124,16 +1124,17 @@ nsFrame::DidSetStyleContext(nsStyleConte
   // bidi algorithm, we need to call |SetBidiEnabled| on the pres
   // context before reflow starts.  See bug 115921.
   if (StyleVisibility()->mDirection == NS_STYLE_DIRECTION_RTL) {
     PresContext()->SetBidiEnabled();
   }
 
   RemoveStateBits(NS_FRAME_SIMPLE_EVENT_REGIONS |
                   NS_FRAME_SIMPLE_DISPLAYLIST);
+  this->MarkNeedsDisplayItemRebuild();
 
   mMayHaveRoundedCorners = true;
 }
 
 void
 nsIFrame::ReparentFrameViewTo(nsViewManager* aViewManager,
                               nsView*        aNewParentView,
                               nsView*        aOldParentView)
@@ -6569,26 +6570,32 @@ nsIFrame::GetTransformMatrix(const nsIFr
   nsPoint delta = GetOffsetToCrossDoc(*aOutAncestor);
   int32_t scaleFactor = ((aFlags & IN_CSS_UNITS) ? PresContext()->AppUnitsPerCSSPixel()
                                                  : PresContext()->AppUnitsPerDevPixel());
   return Matrix4x4::Translation(NSAppUnitsToFloatPixels(delta.x, scaleFactor),
                                 NSAppUnitsToFloatPixels(delta.y, scaleFactor),
                                 0.0f);
 }
 
-static void InvalidateRenderingObservers(nsIFrame* aDisplayRoot, nsIFrame* aFrame)
+static void InvalidateRenderingObservers(nsIFrame* aDisplayRoot, nsIFrame* aFrame, bool aFrameChanged = true)
 {
   MOZ_ASSERT(aDisplayRoot == nsLayoutUtils::GetDisplayRootFrame(aFrame));
   SVGObserverUtils::InvalidateDirectRenderingObservers(aFrame);
   nsIFrame* parent = aFrame;
   while (parent != aDisplayRoot &&
          (parent = nsLayoutUtils::GetCrossDocParentFrame(parent)) &&
          !parent->HasAnyStateBits(NS_FRAME_DESCENDANT_NEEDS_PAINT)) {
     SVGObserverUtils::InvalidateDirectRenderingObservers(parent);
   }
+
+  if (!aFrameChanged) {
+    return;
+  }
+
+  aFrame->MarkNeedsDisplayItemRebuild();
 }
 
 void
 SchedulePaintInternal(nsIFrame* aDisplayRoot, nsIFrame* aFrame,
                       nsIFrame::PaintType aType = nsIFrame::PAINT_DEFAULT)
 {
   MOZ_ASSERT(aDisplayRoot == nsLayoutUtils::GetDisplayRootFrame(aFrame));
   nsPresContext* pres = aDisplayRoot->PresContext()->GetRootPresContext();
@@ -6616,16 +6623,18 @@ SchedulePaintInternal(nsIFrame* aDisplay
   }
 }
 
 static void InvalidateFrameInternal(nsIFrame *aFrame, bool aHasDisplayItem = true)
 {
   if (aHasDisplayItem) {
     aFrame->AddStateBits(NS_FRAME_NEEDS_PAINT);
   }
+
+  aFrame->MarkNeedsDisplayItemRebuild();
   SVGObserverUtils::InvalidateDirectRenderingObservers(aFrame);
   bool needsSchedulePaint = false;
   if (nsLayoutUtils::IsPopup(aFrame)) {
     needsSchedulePaint = true;
   } else {
     nsIFrame *parent = nsLayoutUtils::GetCrossDocParentFrame(aFrame);
     while (parent && !parent->HasAnyStateBits(NS_FRAME_DESCENDANT_NEEDS_PAINT)) {
       if (aHasDisplayItem && !parent->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY)) {
@@ -6844,20 +6853,20 @@ nsIFrame::IsInvalid(nsRect& aRect)
     aRect = *rect;
   } else {
     aRect.SetEmpty();
   }
   return true;
 }
 
 void
-nsIFrame::SchedulePaint(PaintType aType)
+nsIFrame::SchedulePaint(PaintType aType, bool aFrameChanged)
 {
   nsIFrame* displayRoot = nsLayoutUtils::GetDisplayRootFrame(this);
-  InvalidateRenderingObservers(displayRoot, this);
+  InvalidateRenderingObservers(displayRoot, this, aFrameChanged);
   SchedulePaintInternal(displayRoot, this, aType);
 }
 
 void
 nsIFrame::SchedulePaintWithoutInvalidatingObservers(PaintType aType)
 {
   nsIFrame* displayRoot = nsLayoutUtils::GetDisplayRootFrame(this);
   SchedulePaintInternal(displayRoot, this, aType);
@@ -10556,16 +10565,18 @@ nsIFrame::SetParent(nsContainerFrame* aP
     RemoveInPopupStateBitFromDescendants(this);
   }
 
   // If our new parent only has invalid children, then we just invalidate
   // ourselves too. This is probably faster than clearing the flag all
   // the way up the frame tree.
   if (aParent->HasAnyStateBits(NS_FRAME_ALL_DESCENDANTS_NEED_PAINT)) {
     InvalidateFrame();
+  } else {
+    SchedulePaint();
   }
 }
 
 void
 nsIFrame::CreateOwnLayerIfNeeded(nsDisplayListBuilder* aBuilder,
                                  nsDisplayList* aList)
 {
   if (GetContent() &&
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -1004,24 +1004,28 @@ public:
 
   /**
    * When we change the size of the frame's border-box rect, we may need to
    * reset the overflow rect if it was previously stored as deltas.
    * (If it is currently a "large" overflow and could be re-packed as deltas,
    * we don't bother as the cost of the allocation has already been paid.)
    */
   void SetRect(const nsRect& aRect) {
+    if (aRect == mRect) {
+      return;
+    }
     if (mOverflow.mType != NS_FRAME_OVERFLOW_LARGE &&
         mOverflow.mType != NS_FRAME_OVERFLOW_NONE) {
       nsOverflowAreas overflow = GetOverflowAreas();
       mRect = aRect;
       SetOverflowAreas(overflow);
     } else {
       mRect = aRect;
     }
+    MarkNeedsDisplayItemRebuild();
   }
   /**
    * Set this frame's rect from a logical rect in its own writing direction
    */
   void SetRect(const mozilla::LogicalRect& aRect,
                const nsSize& aContainerSize) {
     SetRect(GetWritingMode(), aRect, aContainerSize);
   }
@@ -1064,25 +1068,31 @@ public:
   /**
    * Set this frame's physical size. This leaves the frame's physical position
    * (topLeft) unchanged.
    */
   void SetSize(const nsSize& aSize) {
     SetRect(nsRect(mRect.TopLeft(), aSize));
   }
 
-  void SetPosition(const nsPoint& aPt) { mRect.MoveTo(aPt); }
+  void SetPosition(const nsPoint& aPt) {
+    if (mRect.TopLeft() == aPt) {
+      return;
+    }
+    mRect.MoveTo(aPt);
+    MarkNeedsDisplayItemRebuild();
+  }
   void SetPosition(mozilla::WritingMode aWritingMode,
                    const mozilla::LogicalPoint& aPt,
                    const nsSize& aContainerSize) {
     // We subtract mRect.Size() from the container size to account for
     // the fact that logical origins in RTL coordinate systems are at
     // the top right of the frame instead of the top left.
-    mRect.MoveTo(aPt.GetPhysicalPoint(aWritingMode,
-                                      aContainerSize - mRect.Size()));
+    SetPosition(aPt.GetPhysicalPoint(aWritingMode,
+                                    aContainerSize - mRect.Size()));
   }
 
   /**
    * Move the frame, accounting for relative positioning. Use this when
    * adjusting the frame's position by a known amount, to properly update its
    * saved normal position (see GetNormalPosition below).
    *
    * This must be used only when moving a frame *after*
@@ -3036,17 +3046,17 @@ public:
    * PAINT_DELAYED_COMPRESS : Schedule a paint to be executed after a delay, and
    * put FrameLayerBuilder in 'compressed' mode that avoids short cut optimizations.
    */
   enum PaintType {
     PAINT_DEFAULT = 0,
     PAINT_COMPOSITE_ONLY,
     PAINT_DELAYED_COMPRESS
   };
-  void SchedulePaint(PaintType aType = PAINT_DEFAULT);
+  void SchedulePaint(PaintType aType = PAINT_DEFAULT, bool aFrameChanged = true);
 
   // Similar to SchedulePaint() but without calling
   // InvalidateRenderingObservers() for SVG.
   void SchedulePaintWithoutInvalidatingObservers(
     PaintType aType = PAINT_DEFAULT);
 
   /**
    * Checks if the layer tree includes a dedicated layer for this
--- a/layout/mathml/nsMathMLmactionFrame.cpp
+++ b/layout/mathml/nsMathMLmactionFrame.cpp
@@ -195,16 +195,18 @@ nsMathMLmactionFrame::SetInitialChildLis
 
 nsresult
 nsMathMLmactionFrame::AttributeChanged(int32_t  aNameSpaceID,
                                        nsAtom* aAttribute,
                                        int32_t  aModType)
 {
   bool needsReflow = false;
 
+  InvalidateFrame();
+
   if (aAttribute == nsGkAtoms::actiontype_) {
     // updating mActionType ...
     int32_t oldActionType = mActionType;
     mActionType = GetActionType(mContent);
 
     // Initiate a reflow when actiontype classes are different.
     if ((oldActionType & NS_MATHML_ACTION_TYPE_CLASS_BITMASK) !=
           (mActionType & NS_MATHML_ACTION_TYPE_CLASS_BITMASK)) {
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -4904,17 +4904,17 @@ bool
 FrameLayerBuilder::CheckInLayerTreeCompressionMode()
 {
   if (mInLayerTreeCompressionMode) {
     return true;
   }
 
   // If we wanted to be in layer tree compression mode, but weren't, then scheduled
   // a delayed repaint where we will be.
-  mRootPresContext->PresShell()->GetRootFrame()->SchedulePaint(nsIFrame::PAINT_DELAYED_COMPRESS);
+  mRootPresContext->PresShell()->GetRootFrame()->SchedulePaint(nsIFrame::PAINT_DELAYED_COMPRESS, false);
 
   return false;
 }
 
 void
 ContainerState::CollectOldLayers()
 {
   for (Layer* layer = mContainerLayer->GetFirstChild(); layer;