Bug 851847 - Optimize nsBlockFrame::StealFrame when removing a normal flow child. r=bzbarsky

layout/generic/nsBlockFrame.cpp

@@ -5607,12 +5607,66 @@ nsBlockFrame::DoRemoveFrame(nsIFrame* aDeletedFrame, uint32_t aFlags)
   return RemoveBlockChild(aDeletedFrame, !(aFlags & REMOVE_FIXED_CONTINUATIONS));
 }
 
+static bool
+FindBlockLineFor(nsIFrame*             aChild,
+                 nsLineList::iterator  aBegin,
+                 nsLineList::iterator  aEnd,
+                 nsLineList::iterator* aResult)
+{
+  MOZ_ASSERT(aChild->IsBlockOutside());
+  for (nsLineList::iterator line = aBegin; line != aEnd; ++line) {
+    MOZ_ASSERT(line->GetChildCount() > 0);
+    if (line->IsBlock() && line->mFirstChild == aChild) {
+      MOZ_ASSERT(line->GetChildCount() == 1);
+      *aResult = line;
+      return true;
+    }
+  }
+  return false;
+}
+
+static bool
+FindInlineLineFor(nsIFrame*             aChild,
+                  const nsFrameList&    aFrameList,
+                  nsLineList::iterator  aBegin,
+                  nsLineList::iterator  aEnd,
+                  nsLineList::iterator* aResult)
+{
+  MOZ_ASSERT(!aChild->IsBlockOutside());
+  for (nsLineList::iterator line = aBegin; line != aEnd; ++line) {
+    MOZ_ASSERT(line->GetChildCount() > 0);
+    if (!line->IsBlock()) {
+      // Optimize by comparing the line's last child first.
+      nsLineList::iterator next = line.next();
+      if (aChild == (next == aEnd ? aFrameList.LastChild()
+                                  : next->mFirstChild->GetPrevSibling()) ||
+          line->Contains(aChild)) {
+        *aResult = line;
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
+static bool
+FindLineFor(nsIFrame*             aChild,
+            const nsFrameList&    aFrameList,
+            nsLineList::iterator  aBegin,
+            nsLineList::iterator  aEnd,
+            nsLineList::iterator* aResult)
+{
+  return aChild->IsBlockOutside() ?
+    FindBlockLineFor(aChild, aBegin, aEnd, aResult) :
+    FindInlineLineFor(aChild, aFrameList, aBegin, aEnd, aResult);
+}
+
 nsresult
 nsBlockFrame::StealFrame(nsPresContext* aPresContext,
                          nsIFrame*      aChild,
                          bool           aForceNormal)
 {
-  NS_PRECONDITION(aPresContext && aChild, "null pointer");
+  MOZ_ASSERT(aChild->GetParent() == this);
 
   if ((aChild->GetStateBits() & NS_FRAME_OUT_OF_FLOW) &&
       aChild->IsFloating()) {
@@ -5621,81 +5675,52 @@ nsBlockFrame::StealFrame(nsPresContext* aPresContext,
   }
 
   if ((aChild->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)
-      && !aForceNormal)
+      && !aForceNormal) {
     return nsContainerFrame::StealFrame(aPresContext, aChild);
+  }
 
-  // Find the line and the previous sibling that contains
+  MOZ_ASSERT(!(aChild->GetStateBits() & NS_FRAME_OUT_OF_FLOW));
-  // aChild; we also find the pointer to the line.
-  nsLineList::iterator line = mLines.begin(),
-                       line_start = line,
-                       line_end = mLines.end();
-  bool searchingOverflowList = false;
-  FrameLines* overflowLines = nullptr;
-  nsIFrame* prevSibling = nullptr;
-  // Make sure we look in the overflow lines even if the normal line
-  // list is empty
-  TryAllLines(&line, &line_start, &line_end, &searchingOverflowList,
-              &overflowLines);
-  while (line != line_end) {
-    nsIFrame* frame = line->mFirstChild;
-    int32_t n = line->GetChildCount();
-    while (--n >= 0) {
-      if (frame == aChild) {
-        if (frame == line->mFirstChild) {
-          line->mFirstChild = frame->GetNextSibling();
-        }
-        if (searchingOverflowList) {
-          overflowLines->mFrames.RemoveFrame(frame);
-        } else {
-          mFrames.RemoveFrame(frame);
-        }
 
-        // Register removal with the line boxes
+  nsLineList::iterator line;
-        line->NoteFrameRemoved(frame);
+  if (FindLineFor(aChild, mFrames, mLines.begin(), mLines.end(), &line)) {
-        if (line->GetChildCount() > 0) {
+    RemoveFrameFromLine(aChild, line, mFrames, mLines);
-           line->MarkDirty();
+  } else {
-        } else {
+    FrameLines* overflowLines = GetOverflowLines();
-          // Remove the line box
+    DebugOnly<bool> found;
-          nsLineBox* lineBox = line;
+    found = FindLineFor(aChild, overflowLines->mFrames,
-          if (searchingOverflowList) {
+                        overflowLines->mLines.begin(),
-            // Erase the line, destroy the property if it was the last one.
+                        overflowLines->mLines.end(), &line);
-            line = overflowLines->mLines.erase(line);
+    MOZ_ASSERT(found);
-            if (overflowLines->mLines.empty()) {
+    RemoveFrameFromLine(aChild, line, overflowLines->mFrames,
-              DestroyOverflowLines();
+                        overflowLines->mLines);
-              overflowLines = nullptr;
+    if (overflowLines->mLines.empty()) {
-              // We just invalidated our iterators.  Since we were in
+      DestroyOverflowLines();
-              // the overflow lines list, which is now empty, set them
-              // so we're at the end of the regular line list.
-              line_start = mLines.begin();
-              line_end = mLines.end();
-              line = line_end;
-            }
-          } else {
-            line = mLines.erase(line);
-          }
-          FreeLineBox(lineBox);
-          if (line != line_end) {
-            // Line disappeared, so tell next line it may have to change position
-            line->MarkPreviousMarginDirty();
-          }
-        }
-
-        // Ok, we're done
-        return NS_OK;
-      }
-      prevSibling = frame;
-      frame = frame->GetNextSibling();
-    }
-    ++line;
-    TryAllLines(&line, &line_start, &line_end, &searchingOverflowList,
-                &overflowLines);
-    if (prevSibling && !prevSibling->GetNextSibling()) {
-      // We just switched to the overflow list.  Null out prevSibling
-      prevSibling = nullptr;
     }
   }
-  MOZ_ASSERT(false, "StealFrame failed to remove the frame");
+
-  return NS_ERROR_UNEXPECTED;
+  return NS_OK;
+}
+
+void
+nsBlockFrame::RemoveFrameFromLine(nsIFrame* aChild, nsLineList::iterator aLine,
+                                  nsFrameList& aFrameList, nsLineList& aLineList)
+{
+  aFrameList.RemoveFrame(aChild);
+  if (aChild == aLine->mFirstChild) {
+    aLine->mFirstChild = aChild->GetNextSibling();
+  }
+  aLine->NoteFrameRemoved(aChild);
+  if (aLine->GetChildCount() > 0) {
+    aLine->MarkDirty();
+  } else {
+    // The line became empty - destroy it.
+    nsLineBox* lineBox = aLine;
+    aLine = aLineList.erase(aLine);
+    if (aLine != aLineList.end()) {
+      aLine->MarkPreviousMarginDirty();
+    }
+    FreeLineBox(lineBox);
+  }
 }
 
 void

layout/generic/nsBlockFrame.h

@@ -384,6 +384,11 @@ protected:
     }
     aLine->Destroy(PresContext()->PresShell());
   }
+  /**
+   * Helper method for StealFrame.
+   */
+  void RemoveFrameFromLine(nsIFrame* aChild, nsLineList::iterator aLine,
+                           nsFrameList& aFrameList, nsLineList& aLineList);
 
   void TryAllLines(nsLineList::iterator* aIterator,
                    nsLineList::iterator* aStartIterator,

layout/generic/nsFrameSetFrame.cpp

@@ -747,7 +747,9 @@ nsHTMLFramesetFrame::ReflowPlaceChild(nsIFrame*                aChild,
                                       nsIntPoint*              aCellIndex)
 {
   // reflow the child
-  nsHTMLReflowState  reflowState(aPresContext, aReflowState, aChild, aSize);
+  nsHTMLReflowState reflowState(aPresContext, aReflowState, aChild, aSize);
+  reflowState.SetComputedWidth(std::max(0, aSize.width - reflowState.mComputedBorderPadding.LeftRight()));
+  reflowState.SetComputedHeight(std::max(0, aSize.height - reflowState.mComputedBorderPadding.TopBottom()));
   nsHTMLReflowMetrics metrics;
   metrics.width = aSize.width;
   metrics.height= aSize.height;

layout/generic/nsSubDocumentFrame.cpp

@@ -677,28 +677,19 @@ nsSubDocumentFrame::Reflow(nsPresContext*           aPresContext,
   NS_ASSERTION(mContent->GetPrimaryFrame() == this,
                "Shouldn't happen");
 
+  // XUL <iframe> or <browser>, or HTML <iframe>, <object> or <embed>
+  nsresult rv = nsLeafFrame::DoReflow(aPresContext, aDesiredSize, aReflowState,
+                                      aStatus);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // "offset" is the offset of our content area from our frame's
   // top-left corner.
-  nsPoint offset(0, 0);
+  nsPoint offset = nsPoint(aReflowState.mComputedBorderPadding.left,
-  
+                           aReflowState.mComputedBorderPadding.top);
-  if (IsInline()) {
-    // XUL <iframe> or <browser>, or HTML <iframe>, <object> or <embed>
-    nsresult rv = nsLeafFrame::DoReflow(aPresContext, aDesiredSize, aReflowState,
-                                        aStatus);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    offset = nsPoint(aReflowState.mComputedBorderPadding.left,
-                     aReflowState.mComputedBorderPadding.top);
-  } else {
-    // HTML <frame>
-    SizeToAvailSize(aReflowState, aDesiredSize);
-  }
 
   nsSize innerSize(aDesiredSize.width, aDesiredSize.height);
-  if (IsInline()) {
+  innerSize.width  -= aReflowState.mComputedBorderPadding.LeftRight();
-    innerSize.width  -= aReflowState.mComputedBorderPadding.LeftRight();
+  innerSize.height -= aReflowState.mComputedBorderPadding.TopBottom();
-    innerSize.height -= aReflowState.mComputedBorderPadding.TopBottom();
-  }
 
   if (mInnerView) {
     nsViewManager* vm = mInnerView->GetViewManager();

layout/reftests/bugs/reftest.list

@@ -1745,7 +1745,8 @@ skip-if(B2G) == 818276-1.html 818276-1-ref.html
 == 827577-1b.html 827577-1-ref.html
 == 827799-1.html about:blank
 == 836844-1.html 836844-1-ref.html
-== 848421-1.html 848421-1-ref.html
+== 841192-1.html 841192-1-ref.html
-== 847850-1.html 847850-1-ref.html
 == 846144-1.html 846144-1-ref.html
+== 847850-1.html 847850-1-ref.html
+== 848421-1.html 848421-1-ref.html
 test-pref(layout.css.flexbox.enabled,true) == 849407-1.html 849407-1-ref.html

layout/style/html.css

@@ -659,7 +659,7 @@ frameset {
 }
 
 frame {
-  border: none ! important;
+  border-radius: 0 ! important;
 }
 
 iframe {