Bug 504524. Don't reflow the line after an incomplete frame unless we really need to. r=bz
authorRobert O'Callahan <robert@ocallahan.org>
Mon, 19 Oct 2009 01:01:40 -0700
changeset 34246 d3298de30066bb48d1c7babc544dcaaeec3ca7b3
parent 34245 5481246948c65c8fbbda90f4b94ef0cdb5630318
child 34247 872a2ad2af5d10869365085b60a0a98473504c5f
push id115
push userbmcbride@mozilla.com
push dateMon, 19 Oct 2009 23:27:00 +0000
reviewersbz
bugs504524
milestone1.9.3a1pre
Bug 504524. Don't reflow the line after an incomplete frame unless we really need to. r=bz
layout/generic/nsBlockFrame.cpp
layout/generic/nsTextFrame.h
layout/generic/nsTextFrameThebes.cpp
layout/reftests/text/line-editing-1-ref.html
layout/reftests/text/line-editing-1a.html
layout/reftests/text/line-editing-1b.html
layout/reftests/text/line-editing-1c.html
layout/reftests/text/line-editing-1d.html
layout/reftests/text/line-editing-1e.html
layout/reftests/text/reftest.list
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -3846,23 +3846,16 @@ nsBlockFrame::ReflowInlineFrame(nsBlockR
     // If we just ended a first-letter frame or reflowed a placeholder then 
     // don't split the line and don't stop the line reflow...
     if (!(frameReflowStatus & NS_INLINE_BREAK_FIRST_LETTER_COMPLETE) && 
         nsGkAtoms::placeholderFrame != frameType) {
       // Split line after the current frame
       *aLineReflowStatus = LINE_REFLOW_STOP;
       rv = SplitLine(aState, aLineLayout, aLine, aFrame->GetNextSibling(), aLineReflowStatus);
       NS_ENSURE_SUCCESS(rv, rv);
-
-      // Mark next line dirty in case SplitLine didn't end up
-      // pushing any frames.
-      nsLineList_iterator next = aLine.next();
-      if (next != end_lines() && !next->IsBlock()) {
-        next->MarkDirty();
-      }
     }
   }
 
   return NS_OK;
 }
 
 nsresult
 nsBlockFrame::CreateContinuationFor(nsBlockReflowState& aState,
--- a/layout/generic/nsTextFrame.h
+++ b/layout/generic/nsTextFrame.h
@@ -175,17 +175,17 @@ public:
   virtual PRBool PeekOffsetNoAmount(PRBool aForward, PRInt32* aOffset);
   virtual PRBool PeekOffsetCharacter(PRBool aForward, PRInt32* aOffset);
   virtual PRBool PeekOffsetWord(PRBool aForward, PRBool aWordSelectEatSpace, PRBool aIsKeyboardSelect,
                                 PRInt32* aOffset, PeekWordState* aState);
 
   NS_IMETHOD CheckVisibility(nsPresContext* aContext, PRInt32 aStartIndex, PRInt32 aEndIndex, PRBool aRecurse, PRBool *aFinished, PRBool *_retval);
   
   // Update offsets to account for new length. This may clear mTextRun.
-  void SetLength(PRInt32 aLength);
+  void SetLength(PRInt32 aLength, nsLineLayout* aLineLayout);
   
   NS_IMETHOD GetOffsets(PRInt32 &start, PRInt32 &end)const;
   
   virtual void AdjustOffsetsForBidi(PRInt32 start, PRInt32 end);
   
   NS_IMETHOD GetPointFromOffset(PRInt32                 inOffset,
                                 nsPoint*                outPoint);
   
--- a/layout/generic/nsTextFrameThebes.cpp
+++ b/layout/generic/nsTextFrameThebes.cpp
@@ -217,18 +217,18 @@ struct TextRunMappedFlow {
   nsTextFrame* mStartFrame;
   PRInt32      mDOMOffsetToBeforeTransformOffset;
   // The text mapped starts at mStartFrame->GetContentOffset() and is this long
   PRUint32     mContentLength;
 };
 
 /**
  * This is our user data for the textrun, when textRun->GetFlags() does not
- * have TEXT_SIMPLE_FLOW set. When TEXT_SIMPLE_FLOW is set, there is just one
- * flow, the textrun's user data pointer is a pointer to mStartFrame
+ * have TEXT_IS_SIMPLE_FLOW set. When TEXT_IS_SIMPLE_FLOW is set, there is
+ * just one flow, the textrun's user data pointer is a pointer to mStartFrame
  * for that flow, mDOMOffsetToBeforeTransformOffset is zero, and mContentLength
  * is the length of the text node.
  */
 struct TextRunUserData {
   TextRunMappedFlow* mMappedFlows;
   PRInt32            mMappedFlowCount;
 
   PRUint32           mLastFlowIndex;
@@ -3783,62 +3783,82 @@ nsTextFrame::ClearTextRun()
 
   if (!(textRun->GetFlags() & gfxTextRunWordCache::TEXT_IN_CACHE)) {
     // Remove it now because it's not doing anything useful
     gTextRuns->RemoveFromCache(textRun);
     delete textRun;
   }
 }
 
-static void
-ClearTextRunsInFlowChain(nsTextFrame* aFrame)
-{
-  nsTextFrame* f;
-  for (f = aFrame; f; f = static_cast<nsTextFrame*>(f->GetNextContinuation())) {
-    f->ClearTextRun();
-  }
-}
-
 NS_IMETHODIMP
 nsTextFrame::CharacterDataChanged(CharacterDataChangeInfo* aInfo)
 {
-  ClearTextRunsInFlowChain(this);
-
-  nsTextFrame* targetTextFrame;
-  PRInt32 nodeLength = mContent->GetText()->GetLength();
-
-  if (aInfo->mAppend) {
-    targetTextFrame = static_cast<nsTextFrame*>(GetLastContinuation());
-    targetTextFrame->mState &= ~TEXT_WHITESPACE_FLAGS;
-  } else {
-    // Mark all the continuation frames as dirty, and fix up content offsets to
-    // be valid.
-    // Don't set NS_FRAME_IS_DIRTY on |this|, since we call FrameNeedsReflow
-    // below.
-    nsTextFrame* textFrame = this;
-    PRInt32 newLength = nodeLength;
-    do {
-      textFrame->mState &= ~TEXT_WHITESPACE_FLAGS;
-      // If the text node has shrunk, clip the frame contentlength as necessary
-      if (textFrame->mContentOffset > newLength) {
-        textFrame->mContentOffset = newLength;
-      }
+  // Find the first frame whose text has changed. Frames that are entirely
+  // before the text change are completely unaffected.
+  nsTextFrame* next;
+  nsTextFrame* textFrame = this;
+  while (PR_TRUE) {
+    next = static_cast<nsTextFrame*>(textFrame->GetNextContinuation());
+    if (!next || next->GetContentOffset() > PRInt32(aInfo->mChangeStart))
+      break;
+    textFrame = next;
+  }
+
+  PRInt32 endOfChangedText = aInfo->mChangeStart + aInfo->mReplaceLength;
+  nsTextFrame* lastDirtiedFrame = nsnull;
+
+  nsIPresShell* shell = PresContext()->GetPresShell();
+  do {
+    // textFrame contained deleted text (or the insertion point,
+    // if this was a pure insertion).
+    textFrame->mState &= ~TEXT_WHITESPACE_FLAGS;
+    textFrame->ClearTextRun();
+    if (!lastDirtiedFrame ||
+        lastDirtiedFrame->GetParent() != textFrame->GetParent()) {
+      // Ask the parent frame to reflow me.
+      shell->FrameNeedsReflow(textFrame, nsIPresShell::eStyleChange,
+                              NS_FRAME_IS_DIRTY);
+      lastDirtiedFrame = textFrame;
+    } else {
+      // if the parent is a block, we're cheating here because we should
+      // be marking our line dirty, but we're not. nsTextFrame::SetLength
+      // will do that when it gets called during reflow.
+      textFrame->AddStateBits(NS_FRAME_IS_DIRTY);
+    }
+
+    // Below, frames that start after the deleted text will be adjusted so that
+    // their offsets move with the trailing unchanged text. If this change
+    // deletes more text than it inserts, those frame offsets will decrease.
+    // We need to maintain the invariant that mContentOffset is non-decreasing
+    // along the continuation chain. So we need to ensure that frames that
+    // started in the deleted text are all still starting before the
+    // unchanged text.
+    if (textFrame->mContentOffset > endOfChangedText) {
+      textFrame->mContentOffset = endOfChangedText;
+    }
+
+    textFrame = static_cast<nsTextFrame*>(textFrame->GetNextContinuation());
+  } while (textFrame && textFrame->GetContentOffset() < PRInt32(aInfo->mChangeEnd));
+
+  // This is how much the length of the string changed by --- i.e.,
+  // how much the trailing unchanged text moved.
+  PRInt32 sizeChange =
+    aInfo->mChangeStart + aInfo->mReplaceLength - aInfo->mChangeEnd;
+
+  if (sizeChange) {
+    // Fix the offsets of the text frames that start in the trailing
+    // unchanged text.
+    while (textFrame) {
+      textFrame->mContentOffset += sizeChange;
+      // XXX we could rescue some text runs by adjusting their user data
+      // to reflect the change in DOM offsets
+      textFrame->ClearTextRun();
       textFrame = static_cast<nsTextFrame*>(textFrame->GetNextContinuation());
-      if (!textFrame) {
-        break;
-      }
-      textFrame->mState |= NS_FRAME_IS_DIRTY;
-    } while (1);
-    targetTextFrame = this;
-  }
-
-  // Ask the parent frame to reflow me.
-  PresContext()->GetPresShell()->FrameNeedsReflow(targetTextFrame,
-                                                  nsIPresShell::eStyleChange,
-                                                  NS_FRAME_IS_DIRTY);
+    }
+  }
 
   return NS_OK;
 }
 
 /* virtual */ void
 nsTextFrame::DidSetStyleContext(nsStyleContext* aOldStyleContext)
 {
   nsFrame::DidSetStyleContext(aOldStyleContext);
@@ -5945,41 +5965,66 @@ HasSoftHyphenBefore(const nsTextFragment
       break;
     if (aFrag->CharAt(iter.GetOriginalOffset()) == CH_SHY)
       return PR_TRUE;
   }
   return PR_FALSE;
 }
 
 void
-nsTextFrame::SetLength(PRInt32 aLength)
+nsTextFrame::SetLength(PRInt32 aLength, nsLineLayout* aLineLayout)
 {
   mContentLengthHint = aLength;
   PRInt32 end = GetContentOffset() + aLength;
   nsTextFrame* f = static_cast<nsTextFrame*>(GetNextInFlow());
   if (!f)
     return;
+
+  // If our end offset is moving, then even if frames are not being pushed or
+  // pulled, content is moving to or from the next line and the next line
+  // must be reflowed.
+  // If the next-continuation is dirty, then we should dirty the next line now
+  // because we may have skipped doing it if we dirtied it in
+  // CharacterDataChanged. This is ugly but teaching FrameNeedsReflow
+  // and ChildIsDirty to handle a range of frames would be worse.
+  if (aLineLayout &&
+      (end != f->mContentOffset || (f->GetStateBits() & NS_FRAME_IS_DIRTY))) {
+    const nsLineList::iterator* line = aLineLayout->GetLine();
+    nsBlockFrame* block = do_QueryFrame(aLineLayout->GetLineContainerFrame());
+    if (line && block) {
+      nsLineList::iterator next = line->next();
+      if (next != block->end_lines() && !next->IsBlock()) {
+        next->MarkDirty();
+      }
+    }
+  }
+
   if (end < f->mContentOffset) {
     // Our frame is shrinking. Give the text to our next in flow.
     f->mContentOffset = end;
     if (f->GetTextRun() != mTextRun) {
       ClearTextRun();
       f->ClearTextRun();
     }
     return;
   }
+  // Our frame is growing. Take text from our in-flow(s).
+  // We can take text from frames in lines beyond just the next line.
+  // We don't dirty those lines. That's OK, because when we reflow
+  // our empty next-in-flow, it will take text from its next-in-flow and
+  // dirty that line.
   while (f && f->mContentOffset < end) {
-    // Our frame is growing. Take text from our in-flow.
     f->mContentOffset = end;
     if (f->GetTextRun() != mTextRun) {
       ClearTextRun();
       f->ClearTextRun();
     }
     f = static_cast<nsTextFrame*>(f->GetNextInFlow());
   }
+
 #ifdef DEBUG
   f = this;
   PRInt32 iterations = 0;
   while (f && iterations < 10) {
     f->GetContentLength(); // Assert if negative length
     f = static_cast<nsTextFrame*>(f->GetNextContinuation());
     ++iterations;
   }
@@ -6093,17 +6138,17 @@ nsTextFrame::Reflow(nsPresContext*      
     offset += whitespaceCount;
     length -= whitespaceCount;
   }
 
   PRBool completedFirstLetter = PR_FALSE;
   // Layout dependent styles are a problem because we need to reconstruct
   // the gfxTextRun based on our layout.
   if (lineLayout.GetInFirstLetter() || lineLayout.GetInFirstLine()) {
-    SetLength(maxContentLength);
+    SetLength(maxContentLength, &lineLayout);
 
     if (lineLayout.GetInFirstLetter()) {
       // floating first-letter boundaries are significant in textrun
       // construction, so clear the textrun out every time we hit a first-letter
       // and have changed our length (which controls the first-letter boundary)
       ClearTextRun();
       // Find the length of the first-letter. We need a textrun for this.
       gfxSkipCharsIterator iter =
@@ -6135,17 +6180,17 @@ nsTextFrame::Reflow(nsPresContext*      
         }
         length = firstLetterLength;
         if (length) {
           AddStateBits(TEXT_FIRST_LETTER);
         }
         // Change this frame's length to the first-letter length right now
         // so that when we rebuild the textrun it will be built with the
         // right first-letter boundary
-        SetLength(offset + length - GetContentOffset());
+        SetLength(offset + length - GetContentOffset(), &lineLayout);
         // Ensure that the textrun will be rebuilt
         ClearTextRun();
       }
     } 
   }
 
   gfxSkipCharsIterator iter =
     EnsureTextRun(ctx, lineContainer, lineLayout.GetLine(), &flowEndInTextRun);
@@ -6432,20 +6477,19 @@ nsTextFrame::Reflow(nsPresContext*      
       provider.ComputeJustifiableCharacters(offset, charsFit);
 
     NS_ASSERTION(numJustifiableCharacters <= charsFit,
                  "Bad justifiable character count");
     lineLayout.SetTextJustificationWeights(numJustifiableCharacters,
         charsFit - numJustifiableCharacters);
   }
 
-  SetLength(contentLength);
+  SetLength(contentLength, &lineLayout);
 
   if (mContent->HasFlag(NS_TEXT_IN_SELECTION)) {
-    // XXXroc Watch out, this could be slow!!! Speed up GetSelectionDetails?
     SelectionDetails* details = GetSelectionDetails();
     if (details) {
       AddStateBits(NS_FRAME_SELECTED_CONTENT);
       DestroySelectionDetails(details);
     } else {
       RemoveStateBits(NS_FRAME_SELECTED_CONTENT);
     }
   }
@@ -6866,17 +6910,17 @@ nsTextFrame::AdjustOffsetsForBidi(PRInt3
     // let it violate our invariants.
     PRInt32 prevOffset = prev->GetContentOffset();
     aStart = NS_MAX(aStart, prevOffset);
     aEnd = NS_MAX(aEnd, prevOffset);
     prev->ClearTextRun();
   }
 
   mContentOffset = aStart;
-  SetLength(aEnd - aStart);
+  SetLength(aEnd - aStart, nsnull);
 }
 
 /**
  * @return PR_TRUE if this text frame ends with a newline character.  It should return
  * PR_FALSE if it is not a text frame.
  */
 PRBool
 nsTextFrame::HasTerminalNewline() const
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text/line-editing-1-ref.html
@@ -0,0 +1,14 @@
+<!DOCTYPE HTML>
+<html>
+<body style="white-space:pre;">
+Line 1
+Line 2
+Line 3
+Line 4
+Line 5
+Line 6
+Line 7
+Line 8
+Line 9
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text/line-editing-1a.html
@@ -0,0 +1,27 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<head>
+<script>
+function doTest() {
+  var text = document.body.firstChild;
+  // Delete a line, then make a change after it
+  text.deleteData(8, 7);
+  text.replaceData(55, 1, "8");
+  document.documentElement.removeAttribute('class');
+}
+document.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</head>
+<body style="white-space:pre;">
+Line 1
+Line 2
+Line 2
+Line 3
+Line 4
+Line 5
+Line 6
+Line 7
+Line X
+Line 9
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text/line-editing-1b.html
@@ -0,0 +1,25 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<head>
+<script>
+function doTest() {
+  var text = document.body.firstChild;
+  // Insert a line, then make a change after it
+  text.insertData(8, "Line 2\n");
+  text.replaceData(55, 1, "8");
+  document.documentElement.removeAttribute('class');
+}
+document.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</head>
+<body style="white-space:pre;">
+Line 1
+Line 3
+Line 4
+Line 5
+Line 6
+Line 7
+Line X
+Line 9
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text/line-editing-1c.html
@@ -0,0 +1,25 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<head>
+<script>
+function doTest() {
+  var text = document.body.firstChild;
+  // Just make a change
+  text.replaceData(55, 1, "8");
+  document.documentElement.removeAttribute('class');
+}
+document.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</head>
+<body style="white-space:pre;">
+Line 1
+Line 2
+Line 3
+Line 4
+Line 5
+Line 6
+Line 7
+Line X
+Line 9
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text/line-editing-1d.html
@@ -0,0 +1,26 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<head>
+<script>
+function doTest() {
+  var text = document.body.firstChild;
+  // Replace three lines with two lines (but not at line boundaries)
+  text.replaceData(34, 21, "5\nLine 6\nLine ");
+  document.documentElement.removeAttribute('class');
+}
+document.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</head>
+<body style="white-space:pre;">
+Line 1
+Line 2
+Line 3
+Line 4
+Line X
+Line X
+Line X
+Line 7
+Line 8
+Line 9
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/text/line-editing-1e.html
@@ -0,0 +1,24 @@
+<!DOCTYPE HTML>
+<html class="reftest-wait">
+<head>
+<script>
+function doTest() {
+  var text = document.body.firstChild;
+  // Replace two lines with three lines (but not at line boundaries)
+  text.replaceData(34, 7, "5\nLine 6\nLine ");
+  document.documentElement.removeAttribute('class');
+}
+document.addEventListener("MozReftestInvalidate", doTest, false);
+</script>
+</head>
+<body style="white-space:pre;">
+Line 1
+Line 2
+Line 3
+Line 4
+Line X
+Line 7
+Line 8
+Line 9
+</body>
+</html>
--- a/layout/reftests/text/reftest.list
+++ b/layout/reftests/text/reftest.list
@@ -7,16 +7,21 @@
 random-if(MOZ_WIDGET_TOOLKIT!="cocoa") == font-size-adjust-02.html font-size-adjust-02-ref.html
 # This currently fails because line spacing does not respect font-size-adjust
 # in the "obvious" way, but it is unclear what the behavior should really be;
 # see bug #366138 for some (inconclusive) discussion
 # == font-size-adjust-03.html font-size-adjust-03-ref.html
 == justification-1.html justification-1-ref.html
 == justification-2a.html justification-2-ref.html
 == justification-2b.html justification-2-ref.html
+== line-editing-1a.html line-editing-1-ref.html
+== line-editing-1b.html line-editing-1-ref.html
+== line-editing-1c.html line-editing-1-ref.html
+== line-editing-1d.html line-editing-1-ref.html
+== line-editing-1e.html line-editing-1-ref.html
 == long-1.html long-ref.html
 == pre-line-1.html pre-line-1-ref.html
 == pre-line-2.html pre-line-2-ref.html
 == pre-line-3.html pre-line-3-ref.html
 == pre-line-4.html pre-line-4-ref.html
 == soft-hyphens-1a.html soft-hyphens-1-ref.html
 == soft-hyphens-1b.html soft-hyphens-1-ref.html
 == soft-hyphens-1c.html soft-hyphens-1-ref.html