Bug 944291 - Part 2: Mark parent frames whose child frames' overflow have changed as always needing UpdateOverflow called. r=matspal, a=lsblakk
authorRobert O'Callahan <robert@ocallahan.org>
Sat, 14 Dec 2013 02:21:03 +1300
changeset 175608 89680c49db3b77ff338a1fccd93b8f1e6a067394
parent 175607 1c16411289e4bc4202c5786fc463ed7a978f8afc
child 175609 f0607d5d0e071fa8efd6d8059d781c741c7486f1
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmatspal, lsblakk
bugs944291
milestone28.0a2
Bug 944291 - Part 2: Mark parent frames whose child frames' overflow have changed as always needing UpdateOverflow called. r=matspal, a=lsblakk
layout/base/RestyleTracker.h
layout/reftests/bugs/944291-1-ref.html
layout/reftests/bugs/944291-1.html
layout/reftests/bugs/reftest.list
--- a/layout/base/RestyleTracker.h
+++ b/layout/base/RestyleTracker.h
@@ -47,32 +47,35 @@ public:
    * areas instead of UpdateOverflow().
    *
    * If the overflow area changes, then UpdateOverflow will also
    * be called on the parent.
    */
   void AddFrame(nsIFrame* aFrame) {
     uint32_t depth = aFrame->GetDepthInFrameTree();
     if (mEntryList.empty() ||
-        !mEntryList.find(Entry(aFrame, depth, true))) {
-      mEntryList.insert(new Entry(aFrame, depth, true));
+        !mEntryList.find(Entry(aFrame, depth))) {
+      // All frames in mEntryList at this stage have STYLE_CHANGED so we don't
+      // need to worry about setting the STYLE_CHANGED flag if 'find'
+      // returns true.
+      mEntryList.insert(new Entry(aFrame, depth, STYLE_CHANGED));
     }
   }
 
   /**
    * Remove a frame.
    */
   void RemoveFrame(nsIFrame* aFrame) {
     if (mEntryList.empty()) {
       return;
     }
 
     uint32_t depth = aFrame->GetDepthInFrameTree();
-    if (mEntryList.find(Entry(aFrame, depth, false))) {
-      delete mEntryList.remove(Entry(aFrame, depth, false));
+    if (mEntryList.find(Entry(aFrame, depth))) {
+      delete mEntryList.remove(Entry(aFrame, depth));
     }
   }
 
   /**
    * Set the subtree root to limit overflow updates. This must be set if and
    * only if currently reflowing aSubtreeRoot, to ensure overflow changes will
    * still propagate correctly.
    */
@@ -84,63 +87,80 @@ public:
    * Update the overflow of all added frames, and clear the entry list.
    *
    * Start from those deepest in the frame tree and works upwards. This stops 
    * us from processing the same frame twice.
    */
   void Flush() {
     while (!mEntryList.empty()) {
       Entry *entry = mEntryList.removeMin();
-
       nsIFrame *frame = entry->mFrame;
 
-      bool updateParent = false;
-      if (entry->mInitial) {
+      bool overflowChanged;
+      if (entry->mFlags & CHILDREN_CHANGED) {
+        // Need to union the overflow areas of the children.
+        overflowChanged = frame->UpdateOverflow();
+      } else {
         nsOverflowAreas* pre = static_cast<nsOverflowAreas*>
           (frame->Properties().Get(frame->PreTransformOverflowAreasProperty()));
         if (pre) {
+          // Since we have the pre-transform-overflow-areas, we can take a
+          // faster path that doesn't require unioning the overflow areas
+          // of our children.
           // FinishAndStoreOverflow will change the overflow areas passed in,
           // so make a copy.
           nsOverflowAreas overflowAreas = *pre;
           frame->FinishAndStoreOverflow(overflowAreas, frame->GetSize());
-          // We can't tell if the overflow changed, so update the parent regardless
-          updateParent = true;
+          // We can't tell if the overflow changed, so be conservative
+          overflowChanged = true;
+        } else {
+          // We can't take the faster path here. Do it the hard way.
+          overflowChanged = frame->UpdateOverflow();
         }
       }
 
-      // If the overflow changed, then we want to also update the parent's
-      // overflow. We always update the parent for initial frames.
-      if (!updateParent) {
-        updateParent = frame->UpdateOverflow() || entry->mInitial;
-      }
-      if (updateParent) {
+      // If the frame style changed (e.g. positioning offsets)
+      // then we need to update the parent with the overflow areas of its
+      // children.
+      if (overflowChanged || (entry->mFlags & STYLE_CHANGED)) {
         nsIFrame *parent = frame->GetParent();
         if (parent && parent != mSubtreeRoot) {
-          if (!mEntryList.find(Entry(parent, entry->mDepth - 1, false))) {
-            mEntryList.insert(new Entry(parent, entry->mDepth - 1, false));
+          Entry* parentEntry = mEntryList.find(Entry(parent, entry->mDepth - 1));
+          if (parentEntry) {
+            parentEntry->mFlags |= CHILDREN_CHANGED;
+          } else {
+            mEntryList.insert(new Entry(parent, entry->mDepth - 1, CHILDREN_CHANGED));
           }
         }
       }
       delete entry;
     }
   }
   
 private:
+  enum {
+    /**
+     * Set if the overflow areas of children have changed so we need to call
+     * UpdateOverflow on the frame.
+     */
+    CHILDREN_CHANGED = 0x01,
+    /**
+     * True if the frame was explicitly added and hence may have had a style
+     * change that changes its geometry relative to parent, without reflowing.
+     * In this case we must update overflow on the frame's parent even if
+     * this frame's overflow did not change.
+     */
+    STYLE_CHANGED = 0x02
+  };
   struct Entry : SplayTreeNode<Entry>
   {
-    Entry(nsIFrame* aFrame, bool aInitial)
-      : mFrame(aFrame)
-      , mDepth(aFrame->GetDepthInFrameTree())
-      , mInitial(aInitial)
-    {}
-    
-    Entry(nsIFrame* aFrame, uint32_t aDepth, bool aInitial)
+    Entry(nsIFrame* aFrame, uint32_t aDepth, uint8_t aFlags = 0)
       : mFrame(aFrame)
       , mDepth(aDepth)
-      , mInitial(aInitial)
+      , mFlags(aFlags)
     {}
 
     bool operator==(const Entry& aOther) const
     {
       return mFrame == aOther.mFrame;
     }
  
     /**
@@ -164,21 +184,17 @@ private:
       } else {
         return 1;
       }
     }
 
     nsIFrame* mFrame;
     /* Depth in the frame tree */
     uint32_t mDepth;
-    /**
-     * True if the frame had the actual style change, and we
-     * want to check for pre-transform overflow areas.
-     */
-    bool mInitial;
+    uint8_t mFlags;
   };
 
   /* A list of frames to process, sorted by their depth in the frame tree */
   SplayTree<Entry, Entry> mEntryList;
 
   /* Don't update overflow of this frame or its ancestors. */
   const nsIFrame* mSubtreeRoot;
 };
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/944291-1-ref.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#d1 {
+  width: 250px;
+  height: 200px;
+  margin: 200px;
+  transform: translateY(10px);
+}
+#d1.hidden {
+  transform: translateY(20px);
+}
+#d2 {
+  background-color: yellow;
+  position: relative;
+  width: 100px;
+  height: 100px;
+  left: 0;
+}
+.hidden > #d2 {
+  left: -100px;
+}
+</style>
+</head>
+<body>
+<div id="d1" class="hidden">
+  <div id="d2"></div>
+</div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/944291-1.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<style>
+#d1 {
+  width: 250px;
+  height: 200px;
+  margin: 200px;
+  transform: translateY(10px);
+}
+#d1.hidden {
+  transform: translateY(20px);
+}
+#d2 {
+  background-color: yellow;
+  position: relative;
+  width: 100px;
+  height: 100px;
+  left: 0;
+}
+.hidden > #d2 {
+  left: -100px;
+}
+</style>
+</head>
+<body>
+<div id="d1">
+  <div id="d2"></div>
+</div>
+<script>
+window.addEventListener("MozReftestInvalidate", function() {
+  d1.classList.toggle("hidden");
+  document.documentElement.removeAttribute("class");
+});
+</script>
+</body>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1783,8 +1783,9 @@ fuzzy-if(cocoaWidget,1,40) == 928607-1.h
 == 931464-1.html 931464-1-ref.html
 == 931853.html 931853-ref.html
 == 931853-quirks.html 931853-quirks-ref.html
 fuzzy-if(OSX==10.6,2,30) == 933264-1.html 933264-1-ref.html
 == 936670-1.svg 936670-1-ref.svg
 == 941940-1.html 941940-1-ref.html
 == 942017.html 942017-ref.html
 == 942672-1.html 942672-1-ref.html
+== 944291-1.html 944291-1-ref.html