Bug 403426. We should clear whitespace status when we reset the linebreaker. If a final break opportunity exists we should save it and forward it to the line layout. Relanding with a fix so hopefully we won't crash Tp this time. r=smontagu
authorroc+@cs.cmu.edu
Mon, 03 Dec 2007 00:22:07 -0800
changeset 8573 493a92982d75f7194fd351011f3af966f1928364
parent 8572 3efe0cf4f47c7707f4b2ecdc9143a024f7e4bb8a
child 8574 e88e1d0056a5499791eeeaa9f7d0fed50e268ee9
push id1
push userbsmedberg@mozilla.com
push dateThu, 20 Mar 2008 16:49:24 +0000
treeherdermozilla-central@61007906a1f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmontagu
bugs403426
milestone1.9b2pre
Bug 403426. We should clear whitespace status when we reset the linebreaker. If a final break opportunity exists we should save it and forward it to the line layout. Relanding with a fix so hopefully we won't crash Tp this time. r=smontagu
content/base/public/nsLineBreaker.h
content/base/src/nsLineBreaker.cpp
layout/generic/nsTextFrameThebes.cpp
layout/generic/nsTextFrameUtils.h
layout/reftests/bugs/403426-1-ref.html
layout/reftests/bugs/403426-1.html
layout/reftests/bugs/reftest.list
--- a/content/base/public/nsLineBreaker.h
+++ b/content/base/public/nsLineBreaker.h
@@ -179,18 +179,21 @@ public:
                       PRUint32 aFlags, nsILineBreakSink* aSink);
   /**
    * Reset all state. This means the current run has ended; any outstanding
    * calls through nsILineBreakSink are made, and all outstanding references to
    * nsILineBreakSink objects are dropped.
    * After this call, this linebreaker can be reused.
    * This must be called at least once between any call to AppendText() and
    * destroying the object.
+   * @param aTrailingBreak this is set to true when there is a break opportunity
+   * at the end of the text. This will normally only be declared true when there
+   * is breakable whitespace at the end.
    */
-  nsresult Reset() { return FlushCurrentWord(); }
+  nsresult Reset(PRBool* aTrailingBreak);
 
 private:
   // This is a list of text sources that make up the "current word" (i.e.,
   // run of text which does not contain any whitespace). All the mLengths
   // are are nonzero, these cannot overlap.
   struct TextItem {
     TextItem(nsILineBreakSink* aSink, PRUint32 aSinkOffset, PRUint32 aLength,
              PRUint32 aFlags)
--- a/content/base/src/nsLineBreaker.cpp
+++ b/content/base/src/nsLineBreaker.cpp
@@ -386,20 +386,34 @@ nsLineBreaker::AppendText(nsIAtom* aLang
 
   if (!noBreaksNeeded) {
     aSink->SetBreaks(start, offset - start, breakState.Elements() + start);
   }
   return NS_OK;
 }
 
 nsresult
-nsLineBreaker::AppendInvisibleWhitespace(PRUint32 aFlags) {
+nsLineBreaker::AppendInvisibleWhitespace(PRUint32 aFlags)
+{
   nsresult rv = FlushCurrentWord();
   if (NS_FAILED(rv))
     return rv;
 
   PRBool isBreakableSpace = !(aFlags & BREAK_SUPPRESS_INSIDE);
   if (mAfterBreakableSpace && !isBreakableSpace) {
     mBreakHere = PR_TRUE;
   }
   mAfterBreakableSpace = isBreakableSpace;
-  return NS_OK;  
+  return NS_OK;
 }
+
+nsresult
+nsLineBreaker::Reset(PRBool* aTrailingBreak)
+{
+  nsresult rv = FlushCurrentWord();
+  if (NS_FAILED(rv))
+    return rv;
+
+  *aTrailingBreak = mBreakHere || mAfterBreakableSpace;
+  mBreakHere = PR_FALSE;
+  mAfterBreakableSpace = PR_FALSE;
+  return NS_OK;
+}
--- a/layout/generic/nsTextFrameThebes.cpp
+++ b/layout/generic/nsTextFrameThebes.cpp
@@ -598,20 +598,26 @@ public:
   void ResetRunInfo() {
     mLastFrame = nsnull;
     mMappedFlows.Clear();
     mLineBreakBeforeFrames.Clear();
     mMaxTextLength = 0;
     mDoubleByteText = PR_FALSE;
   }
   void ResetLineBreaker() {
-    mLineBreaker.Reset();
+    PRBool trailingBreak;
+    mLineBreaker.Reset(&trailingBreak);
   }
   void AccumulateRunInfo(nsTextFrame* aFrame);
-  void BuildTextRunForFrames(void* aTextBuffer);
+  /**
+   * @return null to indicate either textrun construction failed or
+   * we constructed just a partial textrun to set up linebreaker and other
+   * state for following textruns.
+   */
+  gfxTextRun* BuildTextRunForFrames(void* aTextBuffer);
   void AssignTextRun(gfxTextRun* aTextRun);
   nsTextFrame* GetNextBreakBeforeFrame(PRUint32* aIndex);
   void SetupBreakSinksForTextRun(gfxTextRun* aTextRun, PRBool aIsExistingTextRun,
                                  PRBool aSuppressSink);
   struct FindBoundaryState {
     nsIFrame*    mStopAtFrame;
     nsTextFrame* mFirstTextFrame;
     nsTextFrame* mLastTextFrame;
@@ -686,19 +692,18 @@ private:
   nsAutoTArray<MappedFlow,10>   mMappedFlows;
   nsAutoTArray<nsTextFrame*,50> mLineBreakBeforeFrames;
   nsAutoTArray<nsAutoPtr<BreakSink>,10> mBreakSinks;
   nsLineBreaker                 mLineBreaker;
   gfxTextRun*                   mCurrentFramesAllSameTextRun;
   gfxContext*                   mContext;
   nsIFrame*                     mLineContainer;
   nsTextFrame*                  mLastFrame;
-  // The common ancestor of the current frame and the previous text frame
-  // on the line, if there's no non-text frame boundaries in between. Otherwise
-  // null.
+  // The common ancestor of the current frame and the previous leaf frame
+  // on the line, or null if there was no previous leaf frame.
   nsIFrame*                     mCommonAncestorWithLastFrame;
   // mMaxTextLength is an upper bound on the size of the text in all mapped frames
   PRUint32                      mMaxTextLength;
   PRPackedBool                  mDoubleByteText;
   PRPackedBool                  mBidiEnabled;
   PRPackedBool                  mStartOfLine;
   PRPackedBool                  mTrimNextRunLeadingWhitespace;
   PRPackedBool                  mCurrentRunTrimLeadingWhitespace;
@@ -1023,36 +1028,45 @@ PRBool BuildTextRunsScanner::IsTextRunVa
  * This gets called when we need to make a text run for the current list of
  * frames.
  */
 void BuildTextRunsScanner::FlushFrames(PRBool aFlushLineBreaks)
 {
   if (mMappedFlows.Length() == 0)
     return;
 
+  gfxTextRun* textRun;
   if (!mSkipIncompleteTextRuns && mCurrentFramesAllSameTextRun &&
       ((mCurrentFramesAllSameTextRun->GetFlags() & nsTextFrameUtils::TEXT_INCOMING_WHITESPACE) != 0) ==
       mCurrentRunTrimLeadingWhitespace &&
       IsTextRunValidForMappedFlows(mCurrentFramesAllSameTextRun)) {
     // Optimization: We do not need to (re)build the textrun.
+    textRun = mCurrentFramesAllSameTextRun;
 
     // Feed this run's text into the linebreaker to provide context. This also
     // updates mTrimNextRunLeadingWhitespace appropriately.
-    SetupBreakSinksForTextRun(mCurrentFramesAllSameTextRun, PR_TRUE, PR_FALSE);
+    SetupBreakSinksForTextRun(textRun, PR_TRUE, PR_FALSE);
     mTrimNextRunLeadingWhitespace =
-      (mCurrentFramesAllSameTextRun->GetFlags() & nsTextFrameUtils::TEXT_TRAILING_WHITESPACE) != 0;
+      (textRun->GetFlags() & nsTextFrameUtils::TEXT_TRAILING_WHITESPACE) != 0;
   } else {
     nsAutoTArray<PRUint8,BIG_TEXT_NODE_SIZE> buffer;
     if (!buffer.AppendElements(mMaxTextLength*(mDoubleByteText ? 2 : 1)))
       return;
-    BuildTextRunForFrames(buffer.Elements());
+    textRun = BuildTextRunForFrames(buffer.Elements());
   }
 
   if (aFlushLineBreaks) {
-    mLineBreaker.Reset();
+    PRBool trailingLineBreak;
+    nsresult rv = mLineBreaker.Reset(&trailingLineBreak);
+    // textRun may be null for various reasons, including because we constructed
+    // a partial textrun just to get the linebreaker and other state set up
+    // to build the next textrun.
+    if (NS_SUCCEEDED(rv) && trailingLineBreak && textRun) {
+      textRun->SetFlagBits(nsTextFrameUtils::TEXT_HAS_TRAILING_BREAK);
+    }
     PRUint32 i;
     for (i = 0; i < mBreakSinks.Length(); ++i) {
       if (!mBreakSinks[i]->mExistingTextRun || mBreakSinks[i]->mChangedBreaks) {
         // TODO cause frames associated with the textrun to be reflowed, if they
         // aren't being reflowed already!
       }
     }
     mBreakSinks.Clear();
@@ -1062,17 +1076,17 @@ void BuildTextRunsScanner::FlushFrames(P
   ResetRunInfo();
 }
 
 void BuildTextRunsScanner::AccumulateRunInfo(nsTextFrame* aFrame)
 {
   mMaxTextLength += aFrame->GetContentLength();
   mDoubleByteText |= aFrame->GetContent()->GetText()->Is2b();
   mLastFrame = aFrame;
-  mCommonAncestorWithLastFrame = aFrame;
+  mCommonAncestorWithLastFrame = aFrame->GetParent();
 
   MappedFlow* mappedFlow = &mMappedFlows[mMappedFlows.Length() - 1];
   NS_ASSERTION(mappedFlow->mStartFrame == aFrame ||
                mappedFlow->GetContentEnd() == aFrame->GetContentOffset(),
                "Overlapping or discontiguous frames => BAD");
   mappedFlow->mEndFrame = static_cast<nsTextFrame*>(aFrame->GetNextContinuation());
   if (mCurrentFramesAllSameTextRun != aFrame->GetTextRun()) {
     mCurrentFramesAllSameTextRun = nsnull;
@@ -1193,34 +1207,34 @@ void BuildTextRunsScanner::ScanFrame(nsI
     }
     return;
   }
 
   PRBool continueTextRun = CanTextRunCrossFrameBoundary(aFrame);
   PRBool descendInto = PR_TRUE;
   if (!continueTextRun) {
     FlushFrames(PR_TRUE);
-    mCommonAncestorWithLastFrame = nsnull;
+    mCommonAncestorWithLastFrame = aFrame;
+    mTrimNextRunLeadingWhitespace = PR_FALSE;
     // XXX do we need this? are there frames we need to descend into that aren't
     // float-containing-blocks?
     descendInto = !aFrame->IsFloatContainingBlock();
     mStartOfLine = PR_FALSE;
-    mTrimNextRunLeadingWhitespace = PR_FALSE;
   }
 
   if (descendInto) {
     nsIFrame* f;
     for (f = aFrame->GetFirstChild(nsnull); f; f = f->GetNextSibling()) {
       ScanFrame(f);
     }
   }
 
   if (!continueTextRun) {
     FlushFrames(PR_TRUE);
-    mCommonAncestorWithLastFrame = nsnull;
+    mCommonAncestorWithLastFrame = aFrame;
     mTrimNextRunLeadingWhitespace = PR_FALSE;
   }
 
   LiftCommonAncestorWithLastFrameToParent(aFrame->GetParent());
 }
 
 nsTextFrame*
 BuildTextRunsScanner::GetNextBreakBeforeFrame(PRUint32* aIndex)
@@ -1317,17 +1331,17 @@ GetFontMetrics(gfxFontGroup* aFontGroup)
   if (!aFontGroup)
     return gfxFont::Metrics();
   gfxFont* font = aFontGroup->GetFontAt(0);
   if (!font)
     return gfxFont::Metrics();
   return font->GetMetrics();
 }
 
-void
+gfxTextRun*
 BuildTextRunsScanner::BuildTextRunForFrames(void* aTextBuffer)
 {
   gfxSkipCharsBuilder builder;
 
   const void* textPtr = aTextBuffer;
   PRBool anySmallcapsStyle = PR_FALSE;
   PRBool anyTextTransformStyle = PR_FALSE;
   nsIContent* lastContent = nsnull;
@@ -1416,17 +1430,17 @@ BuildTextRunsScanner::BuildTextRunForFra
       aTextBuffer = bufEnd;
     } else {
       if (mDoubleByteText) {
         // Need to expand the text. First transform it into a temporary buffer,
         // then expand.
         nsAutoTArray<PRUint8,BIG_TEXT_NODE_SIZE> tempBuf;
         if (!tempBuf.AppendElements(contentLength)) {
           DestroyUserData(userData);
-          return;
+          return nsnull;
         }
         PRUint8* bufStart = tempBuf.Elements();
         PRUint8* end = nsTextFrameUtils::TransformText(
             reinterpret_cast<const PRUint8*>(frag->Get1b()) + contentStart, contentLength,
             bufStart, compressWhitespace, &mTrimNextRunLeadingWhitespace,
             &builder, &analysisFlags);
         aTextBuffer = ExpandBuffer(static_cast<PRUnichar*>(aTextBuffer),
                                    tempBuf.Elements(), end - tempBuf.Elements());
@@ -1451,17 +1465,17 @@ BuildTextRunsScanner::BuildTextRunForFra
 
     lastContent = content;
     endOfLastContent = contentEnd;
   }
 
   // Check for out-of-memory in gfxSkipCharsBuilder
   if (!builder.IsOK()) {
     DestroyUserData(userData);
-    return;
+    return nsnull;
   }
 
   void* finalUserData;
   if (userData == &dummyData) {
     textFlags |= nsTextFrameUtils::TEXT_IS_SIMPLE_FLOW;
     userData = nsnull;
     finalUserData = mMappedFlows[0].mStartFrame;
   } else {
@@ -1486,17 +1500,17 @@ BuildTextRunsScanner::BuildTextRunForFra
 //    textFlags |= gfxTextRunFactory::TEXT_IS_PERSISTENT;
 //  }
 //
   // Now build the textrun
   nsTextFrame* firstFrame = mMappedFlows[0].mStartFrame;
   gfxFontGroup* fontGroup = GetFontGroupForFrame(firstFrame);
   if (!fontGroup) {
     DestroyUserData(userData);
-    return;
+    return nsnull;
   }
 
   if (textFlags & nsTextFrameUtils::TEXT_HAS_TAB) {
     textFlags |= gfxTextRunFactory::TEXT_ENABLE_SPACING;
   }
   if (textFlags & nsTextFrameUtils::TEXT_HAS_SHY) {
     textFlags |= gfxTextRunFactory::TEXT_ENABLE_HYPHEN_BREAKS;
   }
@@ -1593,39 +1607,40 @@ BuildTextRunsScanner::BuildTextRunForFra
         transformingFactory.forget();
       }
     } else {
       textRun = MakeTextRun(text, transformedLength, fontGroup, &params, textFlags);
     }
   }
   if (!textRun) {
     DestroyUserData(userData);
-    return;
+    return nsnull;
   }
 
   // We have to set these up after we've created the textrun, because
   // the breaks may be stored in the textrun during this very call.
   // This is a bit annoying because it requires another loop over the frames
   // making up the textrun, but I don't see a way to avoid this.
   SetupBreakSinksForTextRun(textRun, PR_FALSE, mSkipIncompleteTextRuns);
 
   if (mSkipIncompleteTextRuns) {
     mSkipIncompleteTextRuns = !TextContainsLineBreakerWhiteSpace(textPtr,
         transformedLength, mDoubleByteText);
     
     // Nuke the textrun
     gTextRuns->RemoveFromCache(textRun);
     delete textRun;
     DestroyUserData(userData);
-    return;
+    return nsnull;
   }
 
   // Actually wipe out the textruns associated with the mapped frames and associate
   // those frames with this text run.
   AssignTextRun(textRun);
+  return textRun;
 }
 
 static PRBool
 HasCompressedLeadingWhitespace(nsTextFrame* aFrame, PRInt32 aContentEndOffset,
                                const gfxSkipCharsIterator& aIterator)
 {
   if (!aIterator.IsOriginalCharSkipped())
     return PR_FALSE;
@@ -5503,31 +5518,46 @@ nsTextFrame::Reflow(nsPresContext*      
     lineLayout.SetTrimmableWidth(NSToCoordFloor(trimmableWidth));
   }
   if (charsFit > 0 && charsFit == length &&
       HasSoftHyphenBefore(frag, mTextRun, offset, end)) {
     // Record a potential break after final soft hyphen
     lineLayout.NotifyOptionalBreakPosition(mContent, offset + length,
         textMetrics.mAdvanceWidth + provider.GetHyphenWidth() <= availWidth);
   }
+  PRBool breakAfter = PR_FALSE;
+  if ((charsFit == length && transformedOffset + transformedLength == mTextRun->GetLength() &&
+       (mTextRun->GetFlags() & nsTextFrameUtils::TEXT_HAS_TRAILING_BREAK))) {
+    // Note that because we didn't break, we can be sure that (thanks to the
+    // code up above) textMetrics.mAdvanceWidth includes the width of any
+    // trailing whitespace. So we need to subtract trimmableWidth here
+    // because if we did break at this point, that much width would be trimmed.
+    if (textMetrics.mAdvanceWidth - trimmableWidth > availWidth) {
+      breakAfter = PR_TRUE;
+    } else {
+      lineLayout.NotifyOptionalBreakPosition(mContent, offset + length, PR_TRUE);
+    }
+  }
   if (completedFirstLetter) {
     lineLayout.SetFirstLetterStyleOK(PR_FALSE);
   }
 
   // Compute reflow status
   aStatus = contentLength == maxContentLength
     ? NS_FRAME_COMPLETE : NS_FRAME_NOT_COMPLETE;
 
   if (charsFit == 0 && length > 0) {
     // Couldn't place any text
     aStatus = NS_INLINE_LINE_BREAK_BEFORE();
   } else if (contentLength > 0 && contentLength - 1 == newLineOffset) {
     // Ends in \n
     aStatus = NS_INLINE_LINE_BREAK_AFTER(aStatus);
     lineLayout.SetLineEndsInBR(PR_TRUE);
+  } else if (breakAfter) {
+    aStatus = NS_INLINE_LINE_BREAK_AFTER(aStatus);
   }
 
   // Compute space and letter counts for justification, if required
   if (NS_STYLE_TEXT_ALIGN_JUSTIFY == textStyle->mTextAlign &&
       !textStyle->WhiteSpaceIsSignificant()) {
     // This will include a space for trailing whitespace, if any is present.
     // This is corrected for in nsLineLayout::TrimWhiteSpaceIn.
     PRInt32 numJustifiableCharacters =
--- a/layout/generic/nsTextFrameUtils.h
+++ b/layout/generic/nsTextFrameUtils.h
@@ -68,17 +68,22 @@ public:
 
     // The following flags are set by nsTextFrame
 
     TEXT_IS_SIMPLE_FLOW      = 0x100000,
     TEXT_INCOMING_WHITESPACE = 0x200000,
     TEXT_TRAILING_WHITESPACE = 0x400000,
     TEXT_COMPRESSED_LEADING_WHITESPACE = 0x800000,
     TEXT_NO_BREAKS           = 0x1000000,
-    TEXT_IS_TRANSFORMED      = 0x2000000
+    TEXT_IS_TRANSFORMED      = 0x2000000,
+    // This gets set if there's a break opportunity at the end of the textrun.
+    // We normally don't use this break opportunity because the following text
+    // will have a break opportunity at the start, but it's useful for line
+    // layout to know about it in case the following content is not text
+    TEXT_HAS_TRAILING_BREAK  = 0x4000000
   };
 
   /**
    * Returns PR_TRUE if aChars/aLength are something that make a space
    * character not be whitespace when they follow the space character.
    * For now, this is true if and only if aChars starts with a ZWJ. (This
    * is what Uniscribe assumes.)
    */
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/403426-1-ref.html
@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<p><span style="white-space:nowrap;"><input type="checkbox">hello1</span>
+<p>Hello2<br><span style="white-space:nowrap;"><input type="checkbox"></span>
+<p>Hello3<br><input type="checkbox">
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/403426-1.html
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+<body style="width:0;">
+<p><span style="white-space:nowrap;"><input type="checkbox">hello1</span>
+<p>Hello2 <span style="white-space:nowrap;"><input type="checkbox"></span>
+<p style="white-space:nowrap"><span style="white-space:normal">Hello3
+</span><span style="white-space:nowrap;"><input type="checkbox"></span>
+</body>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -468,16 +468,17 @@ fails == 386310-1d.html 386310-1-ref.htm
 == 403129-2.html 403129-2-ref.html
 == 403129-3.html 403129-3-ref.html
 == 403129-4.html 403129-4-ref.html
 random == 403134-1.html 403134-1-ref.html # bug 405377
 == 403249-1a.html 403249-1-ref.html
 == 403249-1b.html 403249-1-ref.html
 == 403249-2a.html 403249-2-ref.html
 == 403249-2b.html 403249-2-ref.html
+== 403426-1.html 403426-1-ref.html
 == 403455-1.html 403455-1-ref.html
 == 403505-1.xml 403505-1-ref.xul
 == 403519-1.html 403519-1-ref.html
 == 403656-1.html 403656-1-ref.html
 == 403656-2.html 403656-2-ref.html
 == 403656-3.html 403656-3-ref.html
 == 403656-4.html 403656-4-ref.html
 == 403656-5.html 403656-5-ref.html