Bug 1238564 - Don't do another pass over the display list to figure out ancestor scroll clips. r?mattwoodrow draft
authorMarkus Stange <mstange@themasta.com>
Sat, 05 Mar 2016 11:27:54 -0500
changeset 337167 752d726fbc238b783b8e41b7bcd2b6735c4d3739
parent 337031 a23156202612ba4331f167f209eaa5c61d7dd402
child 337168 cd00795753547f31b4d205d0d4b8b82b37d0bb75
push id12285
push usermstange@themasta.com
push dateSat, 05 Mar 2016 16:45:42 +0000
reviewersmattwoodrow
bugs1238564
milestone47.0a1
Bug 1238564 - Don't do another pass over the display list to figure out ancestor scroll clips. r?mattwoodrow MozReview-Commit-ID: BgySqVGG43R
layout/base/DisplayItemScrollClip.h
layout/base/DisplayListClipState.cpp
layout/base/DisplayListClipState.h
layout/generic/nsFrame.cpp
layout/generic/nsSubDocumentFrame.cpp
--- a/layout/base/DisplayItemScrollClip.h
+++ b/layout/base/DisplayItemScrollClip.h
@@ -51,31 +51,41 @@ public:
     , mCrossStackingContextParent(aCrossStackingContextParent)
     , mCrossStackingContextDepth(aCrossStackingContextParent ?
         aCrossStackingContextParent->mCrossStackingContextDepth + 1 : 1)
   {
     MOZ_ASSERT(mScrollableFrame);
   }
 
   /**
-   * "Pick innermost" is analogous to the intersection operation on regular
+   * "Pick descendant" is analogous to the intersection operation on regular
    * clips: In some cases, there are multiple candidate clips that can apply to
    * an item, one of them being the ancestor of the other. This method picks
    * the descendant.
    * Both aClip1 and aClip2 are allowed to be null.
    */
   static const DisplayItemScrollClip*
-  PickInnermost(const DisplayItemScrollClip* aClip1,
+  PickDescendant(const DisplayItemScrollClip* aClip1,
                 const DisplayItemScrollClip* aClip2)
   {
     MOZ_ASSERT(IsAncestor(aClip1, aClip2) || IsAncestor(aClip2, aClip1),
                "one of the scroll clips must be an ancestor of the other");
     return Depth(aClip1) > Depth(aClip2) ? aClip1 : aClip2;
   }
 
+  static const DisplayItemScrollClip*
+  PickAncestor(const DisplayItemScrollClip* aClip1,
+                const DisplayItemScrollClip* aClip2)
+  {
+    MOZ_ASSERT(IsAncestor(aClip1, aClip2) || IsAncestor(aClip2, aClip1),
+               "one of the scroll clips must be an ancestor of the other");
+    return Depth(aClip1) < Depth(aClip2) ? aClip1 : aClip2;
+  }
+
+
   /**
    * Returns whether aAncestor is an ancestor scroll clip of aDescendant.
    * A scroll clip that's null is considered the root scroll clip.
    */
   static bool IsAncestor(const DisplayItemScrollClip* aAncestor,
                          const DisplayItemScrollClip* aDescendant);
 
   /**
--- a/layout/base/DisplayListClipState.cpp
+++ b/layout/base/DisplayListClipState.cpp
@@ -106,17 +106,17 @@ DisplayListClipState::ClipContainingBloc
   // radius.
   ClipContainingBlockDescendants(clipRect, hasBorderRadius ? radii : nullptr,
                                  aClipOnStack);
 }
 
 const DisplayItemScrollClip*
 DisplayListClipState::GetCurrentInnermostScrollClip()
 {
-  return DisplayItemScrollClip::PickInnermost(
+  return DisplayItemScrollClip::PickDescendant(
     mScrollClipContentDescendants, mScrollClipContainingBlockDescendants);
 }
 
 void
 DisplayListClipState::TurnClipIntoScrollClipForContentDescendants(
     nsDisplayListBuilder* aBuilder, nsIScrollableFrame* aScrollableFrame)
 {
   const DisplayItemScrollClip* parent = GetCurrentInnermostScrollClip();
@@ -201,11 +201,12 @@ DisplayListClipState::InsertInactiveScro
   return scrollClip;
 }
 
 DisplayListClipState::AutoSaveRestore::AutoSaveRestore(nsDisplayListBuilder* aBuilder)
   : mState(aBuilder->ClipState())
   , mSavedState(aBuilder->ClipState())
   , mClipUsed(false)
   , mRestored(false)
+  , mClearedForStackingContextContents(false)
 {}
 
 } // namespace mozilla
--- a/layout/base/DisplayListClipState.h
+++ b/layout/base/DisplayListClipState.h
@@ -2,40 +2,40 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef DISPLAYLISTCLIPSTATE_H_
 #define DISPLAYLISTCLIPSTATE_H_
 
 #include "DisplayItemClip.h"
+#include "DisplayItemScrollClip.h"
 
 #include "mozilla/DebugOnly.h"
 
 class nsIFrame;
 class nsIScrollableFrame;
 class nsDisplayListBuilder;
 
 namespace mozilla {
 
-class DisplayItemScrollClip;
-
 /**
  * All clip coordinates are in appunits relative to the reference frame
  * for the display item we're building.
  */
 class DisplayListClipState {
 public:
   DisplayListClipState()
     : mClipContentDescendants(nullptr)
     , mClipContainingBlockDescendants(nullptr)
     , mCurrentCombinedClip(nullptr)
     , mScrollClipContentDescendants(nullptr)
     , mScrollClipContainingBlockDescendants(nullptr)
     , mCrossStackingContextParentScrollClip(nullptr)
+    , mStackingContextAncestorSC(nullptr)
   {}
 
   /**
    * Returns intersection of mClipContainingBlockDescendants and
    * mClipContentDescendants, allocated on aBuilder's arena.
    */
   const DisplayItemClip* GetCurrentCombinedClip(nsDisplayListBuilder* aBuilder);
 
@@ -45,16 +45,21 @@ public:
   }
   const DisplayItemClip* GetClipForContentDescendants() const
   {
     return mClipContentDescendants;
   }
 
   const DisplayItemScrollClip* GetCurrentInnermostScrollClip();
 
+  const DisplayItemScrollClip* CurrentAncestorScrollClipForStackingContextContents()
+  {
+    return mStackingContextAncestorSC;
+  }
+
   class AutoSaveRestore;
   friend class AutoSaveRestore;
 
   class AutoClipContainingBlockDescendantsToContentBox;
   friend class AutoClipContainingBlockDescendantsToContentBox;
 
   class AutoClipMultiple;
   friend class AutoClipMultiple;
@@ -68,44 +73,40 @@ private:
   {
     mClipContainingBlockDescendants = aClip;
     mCurrentCombinedClip = nullptr;
   }
 
   void SetScrollClipForContainingBlockDescendants(const DisplayItemScrollClip* aScrollClip)
   {
     mScrollClipContainingBlockDescendants = aScrollClip;
+    mStackingContextAncestorSC = DisplayItemScrollClip::PickAncestor(mStackingContextAncestorSC, aScrollClip);
   }
 
   void Clear()
   {
     mClipContentDescendants = nullptr;
     mClipContainingBlockDescendants = nullptr;
     mCurrentCombinedClip = nullptr;
     // We do not clear scroll clips.
   }
 
-  void ClearForStackingContextContents()
+  void EnterStackingContextContents(bool aClear)
   {
-    mClipContentDescendants = nullptr;
-    mClipContainingBlockDescendants = nullptr;
-    mCurrentCombinedClip = nullptr;
-    mCrossStackingContextParentScrollClip = GetCurrentInnermostScrollClip();
-    mScrollClipContentDescendants = nullptr;
-    mScrollClipContainingBlockDescendants = nullptr;
-  }
-
-  void ClearIncludingScrollClip()
-  {
-    mClipContentDescendants = nullptr;
-    mClipContainingBlockDescendants = nullptr;
-    mCurrentCombinedClip = nullptr;
-    mCrossStackingContextParentScrollClip = nullptr;
-    mScrollClipContentDescendants = nullptr;
-    mScrollClipContainingBlockDescendants = nullptr;
+    if (aClear) {
+      mClipContentDescendants = nullptr;
+      mClipContainingBlockDescendants = nullptr;
+      mCurrentCombinedClip = nullptr;
+      mCrossStackingContextParentScrollClip = GetCurrentInnermostScrollClip();
+      mScrollClipContentDescendants = nullptr;
+      mScrollClipContainingBlockDescendants = nullptr;
+      mStackingContextAncestorSC = nullptr;
+    } else {
+      mStackingContextAncestorSC = GetCurrentInnermostScrollClip();
+    }
   }
 
   /**
    * Clear the current clip, and instead add it as a scroll clip to the current
    * scroll clip chain.
    */
   void TurnClipIntoScrollClipForContentDescendants(nsDisplayListBuilder* aBuilder,
                                                    nsIScrollableFrame* aScrollableFrame);
@@ -179,58 +180,97 @@ private:
   const DisplayItemClip* mCurrentCombinedClip;
 
   /**
    * The same for scroll clips.
    */
   const DisplayItemScrollClip* mScrollClipContentDescendants;
   const DisplayItemScrollClip* mScrollClipContainingBlockDescendants;
   const DisplayItemScrollClip* mCrossStackingContextParentScrollClip;
+
+  /**
+   * A scroll clip that is an ancestor of all the scroll clips that were
+   * "current" on this clip state since EnterStackingContextContents was
+   * called.
+   */
+  const DisplayItemScrollClip* mStackingContextAncestorSC;
 };
 
 /**
  * A class to automatically save and restore the current clip state. Also
  * offers methods for modifying the clip state. Only one modification is allowed
  * to be in scope at a time using one of these objects; multiple modifications
  * require nested objects. The interface is written this way to prevent
  * dangling pointers to DisplayItemClips.
  */
 class DisplayListClipState::AutoSaveRestore {
 public:
   explicit AutoSaveRestore(nsDisplayListBuilder* aBuilder);
   void Restore()
   {
+    if (!mClearedForStackingContextContents) {
+      // Forward along the ancestor scroll clip to the original clip state.
+      mSavedState.mStackingContextAncestorSC =
+        DisplayItemScrollClip::PickAncestor(mSavedState.mStackingContextAncestorSC,
+                                            mState.mStackingContextAncestorSC);
+    }
     mState = mSavedState;
     mRestored = true;
   }
   ~AutoSaveRestore()
   {
-    mState = mSavedState;
+    Restore();
   }
 
   void Clear()
   {
     NS_ASSERTION(!mRestored, "Already restored!");
     mState.Clear();
     mClipUsed = false;
   }
 
-  void ClearForStackingContextContents()
+  void EnterStackingContextContents(bool aClear)
   {
     NS_ASSERTION(!mRestored, "Already restored!");
-    mState.ClearForStackingContextContents();
-    mClipUsed = false;
+    mState.EnterStackingContextContents(aClear);
+    mClearedForStackingContextContents = aClear;
   }
 
-
-  void ClearIncludingScrollClip()
+  void ExitStackingContextContents(const DisplayItemScrollClip** aOutContainerSC)
   {
-    NS_ASSERTION(!mRestored, "Already restored!");
-    mState.ClearIncludingScrollClip();
-    mClipUsed = false;
+    if (mClearedForStackingContextContents) {
+      // If we cleared the scroll clip, then the scroll clip that was current
+      // just before we cleared it is the one that needs to be set on the
+      // container item.
+      *aOutContainerSC = mSavedState.GetCurrentInnermostScrollClip();
+    } else {
+      // If we didn't clear the scroll clip, then the container item needs to
+      // get a scroll clip that's an ancestor of all its direct child items'
+      // scroll clips.
+      // The simplest way to satisfy this requirement would be to just take the
+      // root scroll clip (i.e. nullptr). However, this can cause the bounds of
+      // the container items to be enlarged unnecessarily, so instead we try to
+      // take the "deepest" scroll clip that satisfies the requirement.
+      // Usually this is the scroll clip that was current before we entered
+      // the stacking context contents (call that the "initial scroll clip").
+      // There are two cases in which the container scroll clip *won't* be the
+      // initial scroll clip (instead the container scroll clip will be a
+      // proper ancestor of the initial scroll clip):
+      //  (1) If SetScrollClipForContainingBlockDescendants was called with an
+      //      ancestor scroll clip of the initial scroll clip while we were
+      //      building our direct child items. This happens if we entered a
+      //      position:absolute or position:fixed element whose containing
+      //      block is an ancestor of the frame that generated the initial
+      //      scroll clip. Then the "ancestor scroll clip for stacking context
+      //      contents" will be set to that scroll clip.
+      //  (2) If one of our direct child items is a container item for which
+      //      (1) or (2) happened.
+      *aOutContainerSC = mState.CurrentAncestorScrollClipForStackingContextContents();
+    }
+    Restore();
   }
 
   void TurnClipIntoScrollClipForContentDescendants(nsDisplayListBuilder* aBuilder, nsIScrollableFrame* aScrollableFrame)
   {
     NS_ASSERTION(!mRestored, "Already restored!");
     mState.TurnClipIntoScrollClipForContentDescendants(aBuilder, aScrollableFrame);
     mClipUsed = true;
   }
@@ -310,16 +350,17 @@ public:
   }
 
 protected:
   DisplayListClipState& mState;
   DisplayListClipState mSavedState;
   DisplayItemClip mClip;
   DebugOnly<bool> mClipUsed;
   DebugOnly<bool> mRestored;
+  bool mClearedForStackingContextContents;
 };
 
 class DisplayListClipState::AutoClipContainingBlockDescendantsToContentBox : public AutoSaveRestore {
 public:
   AutoClipContainingBlockDescendantsToContentBox(nsDisplayListBuilder* aBuilder,
                                                  nsIFrame* aFrame,
                                                  uint32_t aFlags = 0)
     : AutoSaveRestore(aBuilder)
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1916,30 +1916,16 @@ CreateOpacityItem(nsDisplayListBuilder* 
   // all descendant content, but some should not be clipped.
   DisplayListClipState::AutoSaveRestore opacityClipState(aBuilder);
   opacityClipState.Clear();
   aList.AppendNewToTop(
       new (aBuilder) nsDisplayOpacity(aBuilder, aFrame, &aList,
                                       aScrollClip, aItemForEventsOnly));
 }
 
-static const DisplayItemScrollClip*
-FindCommonAncestorScrollClip(nsDisplayList& aList, const DisplayItemScrollClip* aInitial)
-{
-  const DisplayItemScrollClip* ancestorScrollClip = aInitial;
-  for (nsDisplayItem* i = aList.GetBottom(); i; i = i->GetAbove()) {
-    const DisplayItemScrollClip* itemScrollClip = i->ScrollClip();
-    if (!DisplayItemScrollClip::IsAncestor(ancestorScrollClip, itemScrollClip)) {
-      MOZ_ASSERT(DisplayItemScrollClip::IsAncestor(itemScrollClip, ancestorScrollClip));
-      ancestorScrollClip = itemScrollClip;
-    }
-  }
-  return ancestorScrollClip;
-}
-
 void
 nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder* aBuilder,
                                              const nsRect&         aDirtyRect,
                                              nsDisplayList*        aList) {
   if (GetStateBits() & NS_FRAME_TOO_DEEP_IN_FRAME_TREE)
     return;
 
   // Replaced elements have their visibility handled here, because
@@ -2047,39 +2033,27 @@ nsIFrame::BuildDisplayListForStackingCon
 
   mozilla::gfx::VRDeviceProxy* vrHMDInfo = nullptr;
   if ((GetStateBits() & NS_FRAME_HAS_VR_CONTENT)) {
     vrHMDInfo = static_cast<mozilla::gfx::VRDeviceProxy*>(mContent->GetProperty(nsGkAtoms::vr_state));
   }
 
   DisplayListClipState::AutoSaveRestore clipState(aBuilder);
 
-  // The scroll clip to use for the container items that we create here.
-  // We can create multiple different container items in this function, and not
-  // all of them will use the same scroll clip depending on whether we reset
-  // the clip, so the value of containerItemScrollClip will be updated whenever
-  // we clear or restore the clip.
-  const DisplayItemScrollClip* containerItemScrollClip =
-    aBuilder->ClipState().GetCurrentInnermostScrollClip();
-  bool didResetClip = false;
-
+  bool clearClip = false;
   if (isTransformed || usingSVGEffects || useFixedPosition || useStickyPosition) {
     // We don't need to pass ancestor clipping down to our children;
     // everything goes inside a display item's child list, and the display
     // item itself will be clipped.
     // For transforms we also need to clear ancestor clipping because it's
     // relative to the wrong display item reference frame anyway.
-    // We clear both regular and scroll clips here. Our content needs to be
-    // able to walk up the complete cross stacking context scroll clip chain,
-    // so we call a special method on the clip state that keeps the ancestor
-    // scroll clip around.
-    clipState.ClearForStackingContextContents();
-    didResetClip = true;
-    containerItemScrollClip = nullptr;
-  }
+    clearClip = true;
+  }
+
+  clipState.EnterStackingContextContents(clearClip);
 
   nsDisplayListCollection set;
   {
     DisplayListClipState::AutoSaveRestore nestedClipState(aBuilder);
     nsDisplayListBuilder::AutoInTransformSetter
       inTransformSetter(aBuilder, inTransform);
     nsDisplayListBuilder::AutoSaveRestorePerspectiveIndex
       perspectiveIndex(aBuilder, this);
@@ -2162,28 +2136,22 @@ nsIFrame::BuildDisplayListForStackingCon
   }
 #ifdef DEBUG
   DisplayDebugBorders(aBuilder, this, set);
 #endif
   resultList.AppendToTop(set.Outlines());
   // 8, 9: non-negative z-index children
   resultList.AppendToTop(set.PositionedDescendants());
 
-  if (!didResetClip) {
-    // The current scroll clip for this frame was containerItemScrollClip, but
-    // some of our children might have received a scroll clip from outside this
-    // frame, and containerItemScrollClip would not be an ancestor of that
-    // scroll clip. So we need to find a scroll clip that is an ancestor of all
-    // containened scroll clips.
-    // We don't need to do this if we reset the clip - in that case, the
-    // pre-clear scroll clip and and the scroll clips of our contents are in
-    // different scroll clip trees. (This is very confusing and I apologize.)
-    containerItemScrollClip = FindCommonAncestorScrollClip(resultList,
-      containerItemScrollClip);
-  }
+  // Get the scroll clip to use for the container items that we create here.
+  // If we cleared the clip, and we create multiple container items, then the
+  // items we create before we restore the clip will have a different scroll
+  // clip from the items we create after we restore the clip.
+  const DisplayItemScrollClip* containerItemScrollClip =
+    aBuilder->ClipState().CurrentAncestorScrollClipForStackingContextContents();
 
   /* If adding both a nsDisplayBlendContainer and a nsDisplayMixBlendMode to the
    * same list, the nsDisplayBlendContainer should be added first. This only
    * happens when the element creating this stacking context has mix-blend-mode
    * and also contains a child which has mix-blend-mode.
    * The nsDisplayBlendContainer must be added to the list first, so it does not
    * isolate the containing element blending as well.
    */
@@ -2194,20 +2162,17 @@ nsIFrame::BuildDisplayListForStackingCon
     resultList.AppendNewToTop(
       new (aBuilder) nsDisplayBlendContainer(aBuilder, this, &resultList,
                                              containerItemScrollClip));
   }
 
   if (!isTransformed && !useFixedPosition && !useStickyPosition) {
     // Restore saved clip state now so that any display items we create below
     // are clipped properly.
-    clipState.Restore();
-    if (didResetClip) {
-      containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
-    }
+    clipState.ExitStackingContextContents(&containerItemScrollClip);
   }
 
   bool is3DContextRoot = Extend3DContext() && !Combines3DTransformWithAncestors();
 
   /* If there are any SVG effects, wrap the list up in an SVG effects item
    * (which also handles CSS group opacity). Note that we create an SVG effects
    * item even if resultList is empty, since a filter can produce graphical
    * output even if the element being filtered wouldn't otherwise do so.
@@ -2274,20 +2239,17 @@ nsIFrame::BuildDisplayListForStackingCon
       }
       WrapSeparatorTransform(aBuilder, this, dirtyRect,
                              &nonparticipants, &participants, index++);
       resultList.AppendToTop(&participants);
     }
 
     // Restore clip state now so nsDisplayTransform is clipped properly.
     if (!HasPerspective() && !useFixedPosition && !useStickyPosition) {
-      clipState.Restore();
-      if (didResetClip) {
-        containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
-      }
+      clipState.ExitStackingContextContents(&containerItemScrollClip);
     }
     // Revert to the dirtyrect coming in from the parent, without our transform
     // taken into account.
     buildingDisplayList.SetDirtyRect(dirtyRectOutsideTransform);
     // Revert to the outer reference frame and offset because all display
     // items we create from now on are outside the transform.
     nsPoint toOuterReferenceFrame;
     const nsIFrame* outerReferenceFrame = this;
@@ -2299,20 +2261,17 @@ nsIFrame::BuildDisplayListForStackingCon
       GetOffsetToCrossDoc(outerReferenceFrame));
 
     nsDisplayTransform *transformItem =
       new (aBuilder) nsDisplayTransform(aBuilder, this, &resultList, dirtyRect);
     resultList.AppendNewToTop(transformItem);
 
     if (HasPerspective()) {
       if (!useFixedPosition && !useStickyPosition) {
-        clipState.Restore();
-        if (didResetClip) {
-          containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
-        }
+        clipState.ExitStackingContextContents(&containerItemScrollClip);
       }
       resultList.AppendNewToTop(
         new (aBuilder) nsDisplayPerspective(
           aBuilder, this,
           GetContainingBlock()->GetContent()->GetPrimaryFrame(), &resultList));
     }
 
     /* If we need an opacity item, but didn't do it earlier, add it now on the
@@ -2320,20 +2279,17 @@ nsIFrame::BuildDisplayListForStackingCon
      */
     if (useOpacity && !usingSVGEffects) {
       CreateOpacityItem(aBuilder, this, resultList, opacityItemForEventsOnly,
                         containerItemScrollClip);
     }
   }
 
   if (useFixedPosition || useStickyPosition) {
-    clipState.Restore();
-    if (didResetClip) {
-      containerItemScrollClip = aBuilder->ClipState().GetCurrentInnermostScrollClip();
-    }
+    clipState.ExitStackingContextContents(&containerItemScrollClip);
   }
 
   /* If we have sticky positioning, wrap it in a sticky position item.
    */
   if (useFixedPosition) {
     resultList.AppendNewToTop(
         new (aBuilder) nsDisplayFixedPosition(aBuilder, this, &resultList));
   } else if (useStickyPosition) {
--- a/layout/generic/nsSubDocumentFrame.cpp
+++ b/layout/generic/nsSubDocumentFrame.cpp
@@ -468,17 +468,17 @@ nsSubDocumentFrame::BuildDisplayList(nsD
 
   {
     DisplayListClipState::AutoSaveRestore nestedClipState(aBuilder);
     if (needsOwnLayer) {
       // Clear current clip. There's no point in propagating it down, since
       // the layer we will construct will be clipped by the current clip.
       // In fact for nsDisplayZoom propagating it down would be incorrect since
       // nsDisplayZoom changes the meaning of appunits.
-      nestedClipState.ClearForStackingContextContents();
+      nestedClipState.EnterStackingContextContents(true);
     }
 
     if (subdocRootFrame) {
       nsIFrame* rootScrollFrame = presShell->GetRootScrollFrame();
       nsDisplayListBuilder::AutoCurrentScrollParentIdSetter idSetter(
           aBuilder,
           ignoreViewportScrolling && rootScrollFrame && rootScrollFrame->GetContent()
               ? nsLayoutUtils::FindOrCreateIDFor(rootScrollFrame->GetContent())