Bug 1369985 - Look for text run boundary based on transformed text when needed. r?jfkthame draft
authorXidorn Quan <me@upsuper.org>
Sun, 18 Jun 2017 11:18:32 +1000
changeset 596047 84da5cb6bfcdbd86d3c6d8f8d15a1a8a15e54bd8
parent 595489 fe809f57bf2287bb937c3422ed03a63740b3448b
child 597316 21be7fcddad67d0dbf9f5d2f7cd31336920f34a6
push id64515
push userxquan@mozilla.com
push dateSun, 18 Jun 2017 03:22:55 +0000
reviewersjfkthame
bugs1369985
milestone56.0a1
Bug 1369985 - Look for text run boundary based on transformed text when needed. r?jfkthame MozReview-Commit-ID: 9wJXia7LBpO
layout/generic/crashtests/1373586.html
layout/generic/crashtests/crashtests.list
layout/generic/nsTextFrame.cpp
layout/reftests/bugs/1369985-1-ref.html
layout/reftests/bugs/1369985-1.html
layout/reftests/bugs/reftest.list
new file mode 100644
--- /dev/null
+++ b/layout/generic/crashtests/1373586.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<meta charset="UTF-8">
+<script>
+// This is the constant in nsTextFrame.cpp of the number of lines which
+// a text run can be built up to.
+const NUM_LINES_TO_BUILD_TEXT_RUNS = 200;
+// Push the affecting line to be the last line in the text run.
+for (let i = 0; i < NUM_LINES_TO_BUILD_TEXT_RUNS - 1; i++) {
+  document.write('x<br>');
+}
+// The exact number here isn't important. It just needs to be large
+// enough that '\n' would be inside text after a line break.
+for (let i = 0; i < 2000; i++) {
+  document.write('あ');
+}
+document.write('\nあ<br>');
+// Then this ruby would not get its text run.
+document.write('x<ruby>x</ruby>');
+</script>
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@@ -649,8 +649,9 @@ load 1278461-2.html
 load 1281102.html
 load 1297427-non-equal-centers.html
 load 1304441.html
 load 1316649.html
 load 1349650.html
 asserts-if(browserIsRemote,0-5) load 1349816-1.html # bug 1350352
 load 1367413-1.html
 load 1368617-1.html
+load 1373586.html
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -1054,16 +1054,17 @@ public:
   void SetupTextEmphasisForTextRun(gfxTextRun* aTextRun, const void* aTextPtr);
   struct FindBoundaryState {
     nsIFrame*    mStopAtFrame;
     nsTextFrame* mFirstTextFrame;
     nsTextFrame* mLastTextFrame;
     bool mSeenTextRunBoundaryOnLaterLine;
     bool mSeenTextRunBoundaryOnThisLine;
     bool mSeenSpaceForLineBreakingOnThisLine;
+    nsTArray<char16_t>& mBuffer;
   };
   enum FindBoundaryResult {
     FB_CONTINUE,
     FB_STOPPED_AT_STOP_FRAME,
     FB_FOUND_VALID_TEXTRUN_BOUNDARY
   };
   FindBoundaryResult FindBoundaries(nsIFrame* aFrame, FindBoundaryState* aState);
 
@@ -1202,16 +1203,47 @@ TextContainsLineBreakerWhiteSpace(const 
     for (uint32_t i = 0; i < aLength; ++i) {
       if (IsLineBreakingWhiteSpace(chars[i]))
         return true;
     }
     return false;
   }
 }
 
+static_assert(uint8_t(mozilla::StyleWhiteSpace::Normal) == 0, "Convention: StyleWhiteSpace::Normal should be 0");
+static_assert(uint8_t(mozilla::StyleWhiteSpace::Pre) == 1, "Convention: StyleWhiteSpace::Pre should be 1");
+static_assert(uint8_t(mozilla::StyleWhiteSpace::Nowrap) == 2, "Convention: StyleWhiteSpace::NoWrap should be 2");
+static_assert(uint8_t(mozilla::StyleWhiteSpace::PreWrap) == 3, "Convention: StyleWhiteSpace::PreWrap should be 3");
+static_assert(uint8_t(mozilla::StyleWhiteSpace::PreLine) == 4, "Convention: StyleWhiteSpace::PreLine should be 4");
+static_assert(uint8_t(mozilla::StyleWhiteSpace::PreSpace) == 5, "Convention: StyleWhiteSpace::PreSpace should be 5");
+
+static nsTextFrameUtils::CompressionMode
+GetCSSWhitespaceToCompressionMode(nsTextFrame* aFrame,
+                                  const nsStyleText* aStyleText)
+{
+  static const nsTextFrameUtils::CompressionMode sModes[] =
+  {
+    nsTextFrameUtils::COMPRESS_WHITESPACE_NEWLINE,     // normal
+    nsTextFrameUtils::COMPRESS_NONE,                   // pre
+    nsTextFrameUtils::COMPRESS_WHITESPACE_NEWLINE,     // nowrap
+    nsTextFrameUtils::COMPRESS_NONE,                   // pre-wrap
+    nsTextFrameUtils::COMPRESS_WHITESPACE,             // pre-line
+    nsTextFrameUtils::COMPRESS_NONE_TRANSFORM_TO_SPACE // -moz-pre-space
+  };
+
+  auto compression = sModes[uint8_t(aStyleText->mWhiteSpace)];
+  if (compression == nsTextFrameUtils::COMPRESS_NONE &&
+      !aStyleText->NewlineIsSignificant(aFrame)) {
+    // If newline is set to be preserved, but then suppressed,
+    // transform newline to space.
+    compression = nsTextFrameUtils::COMPRESS_NONE_TRANSFORM_TO_SPACE;
+  }
+  return compression;
+}
+
 struct FrameTextTraversal
 {
   FrameTextTraversal()
     : mFrameToScan(nullptr)
     , mOverflowFrameToScan(nullptr)
     , mScanSiblings(false)
     , mLineBreakerCanCrossFrameBoundary(false)
     , mTextRunCanCrossFrameBoundary(false)
@@ -1311,27 +1343,52 @@ BuildTextRunsScanner::FindBoundaries(nsI
     }
     aState->mLastTextFrame = textFrame;
   }
 
   if (aFrame == aState->mStopAtFrame)
     return FB_STOPPED_AT_STOP_FRAME;
 
   if (textFrame) {
-    if (!aState->mSeenSpaceForLineBreakingOnThisLine) {
-      const nsTextFragment* frag = textFrame->GetContent()->GetText();
-      uint32_t start = textFrame->GetContentOffset();
-      const void* text = frag->Is2b()
-          ? static_cast<const void*>(frag->Get2b() + start)
-          : static_cast<const void*>(frag->Get1b() + start);
-      if (TextContainsLineBreakerWhiteSpace(text, textFrame->GetContentLength(),
-                                            frag->Is2b())) {
-        aState->mSeenSpaceForLineBreakingOnThisLine = true;
-        if (aState->mSeenTextRunBoundaryOnLaterLine)
-          return FB_FOUND_VALID_TEXTRUN_BOUNDARY;
+    if (aState->mSeenSpaceForLineBreakingOnThisLine) {
+      return FB_CONTINUE;
+    }
+    const nsTextFragment* frag = textFrame->GetContent()->GetText();
+    uint32_t start = textFrame->GetContentOffset();
+    uint32_t length = textFrame->GetContentLength();
+    const void* text;
+    if (frag->Is2b()) {
+      // It is possible that we may end up removing all whitespace in
+      // a piece of text because of The White Space Processing Rules,
+      // so we need to transform it before we can check existence of
+      // such whitespaces.
+      aState->mBuffer.EnsureLengthAtLeast(length);
+      nsTextFrameUtils::CompressionMode compression =
+        GetCSSWhitespaceToCompressionMode(textFrame, textFrame->StyleText());
+      uint8_t incomingFlags = 0;
+      gfxSkipChars skipChars;
+      nsTextFrameUtils::Flags analysisFlags;
+      char16_t* bufStart = aState->mBuffer.Elements();
+      char16_t* bufEnd = nsTextFrameUtils::TransformText(
+        frag->Get2b() + start, length, bufStart, compression,
+        &incomingFlags, &skipChars, &analysisFlags);
+      text = bufStart;
+      length = bufEnd - bufStart;
+    } else {
+      // If the text only contains ASCII characters, it is currently
+      // impossible that TransformText would remove all whitespaces,
+      // and thus the check below should return the same result for
+      // transformed text and original text. So we don't need to try
+      // transforming it here.
+      text = static_cast<const void*>(frag->Get1b() + start);
+    }
+    if (TextContainsLineBreakerWhiteSpace(text, length, frag->Is2b())) {
+      aState->mSeenSpaceForLineBreakingOnThisLine = true;
+      if (aState->mSeenTextRunBoundaryOnLaterLine) {
+        return FB_FOUND_VALID_TEXTRUN_BOUNDARY;
       }
     }
     return FB_CONTINUE;
   }
 
   FrameTextTraversal traversal = CanTextCrossFrameBoundary(aFrame);
   if (!traversal.mTextRunCanCrossFrameBoundary) {
     aState->mSeenTextRunBoundaryOnThisLine = true;
@@ -1465,28 +1522,29 @@ BuildTextRuns(DrawTarget* aDrawTarget, n
   // are not reconstructed. We construct partial text runs for that text ---
   // for the sake of simplifying the code and feeding the linebreaker ---
   // but we discard them instead of assigning them to frames.
   // This is a little awkward because we traverse lines in the reverse direction
   // but we traverse the frames in each line in the forward direction.
   nsBlockInFlowLineIterator forwardIterator = backIterator;
   nsIFrame* stopAtFrame = lineContainerChild;
   nsTextFrame* nextLineFirstTextFrame = nullptr;
+  AutoTArray<char16_t, BIG_TEXT_NODE_SIZE> buffer;
   bool seenTextRunBoundaryOnLaterLine = false;
   bool mayBeginInTextRun = true;
   while (true) {
     forwardIterator = backIterator;
     nsBlockFrame::LineIterator line = backIterator.GetLine();
     if (!backIterator.Prev() || backIterator.GetLine()->IsBlock()) {
       mayBeginInTextRun = false;
       break;
     }
 
     BuildTextRunsScanner::FindBoundaryState state = { stopAtFrame, nullptr, nullptr,
-      bool(seenTextRunBoundaryOnLaterLine), false, false };
+      bool(seenTextRunBoundaryOnLaterLine), false, false, buffer };
     nsIFrame* child = line->mFirstChild;
     bool foundBoundary = false;
     for (int32_t i = line->GetChildCount() - 1; i >= 0; --i) {
       BuildTextRunsScanner::FindBoundaryResult result =
           scanner.FindBoundaries(child, &state);
       if (result == BuildTextRunsScanner::FB_FOUND_VALID_TEXTRUN_BOUNDARY) {
         foundBoundary = true;
         break;
@@ -1977,47 +2035,16 @@ GetHyphenTextRun(const gfxTextRun* aText
       return nullptr;
     }
   }
 
   return aTextRun->GetFontGroup()->
     MakeHyphenTextRun(dt, aTextRun->GetAppUnitsPerDevUnit());
 }
 
-static_assert(uint8_t(mozilla::StyleWhiteSpace::Normal) == 0, "Convention: StyleWhiteSpace::Normal should be 0");
-static_assert(uint8_t(mozilla::StyleWhiteSpace::Pre) == 1, "Convention: StyleWhiteSpace::Pre should be 1");
-static_assert(uint8_t(mozilla::StyleWhiteSpace::Nowrap) == 2, "Convention: StyleWhiteSpace::NoWrap should be 2");
-static_assert(uint8_t(mozilla::StyleWhiteSpace::PreWrap) == 3, "Convention: StyleWhiteSpace::PreWrap should be 3");
-static_assert(uint8_t(mozilla::StyleWhiteSpace::PreLine) == 4, "Convention: StyleWhiteSpace::PreLine should be 4");
-static_assert(uint8_t(mozilla::StyleWhiteSpace::PreSpace) == 5, "Convention: StyleWhiteSpace::PreSpace should be 5");
-
-static nsTextFrameUtils::CompressionMode
-GetCSSWhitespaceToCompressionMode(nsTextFrame* aFrame,
-                                  const nsStyleText* aStyleText)
-{
-  static const nsTextFrameUtils::CompressionMode sModes[] =
-  {
-    nsTextFrameUtils::COMPRESS_WHITESPACE_NEWLINE,     // normal
-    nsTextFrameUtils::COMPRESS_NONE,                   // pre
-    nsTextFrameUtils::COMPRESS_WHITESPACE_NEWLINE,     // nowrap
-    nsTextFrameUtils::COMPRESS_NONE,                   // pre-wrap
-    nsTextFrameUtils::COMPRESS_WHITESPACE,             // pre-line
-    nsTextFrameUtils::COMPRESS_NONE_TRANSFORM_TO_SPACE // -moz-pre-space
-  };
-
-  auto compression = sModes[uint8_t(aStyleText->mWhiteSpace)];
-  if (compression == nsTextFrameUtils::COMPRESS_NONE &&
-      !aStyleText->NewlineIsSignificant(aFrame)) {
-    // If newline is set to be preserved, but then suppressed,
-    // transform newline to space.
-    compression = nsTextFrameUtils::COMPRESS_NONE_TRANSFORM_TO_SPACE;
-  }
-  return compression;
-}
-
 already_AddRefed<gfxTextRun>
 BuildTextRunsScanner::BuildTextRunForFrames(void* aTextBuffer)
 {
   gfxSkipChars skipChars;
 
   const void* textPtr = aTextBuffer;
   bool anyTextTransformStyle = false;
   bool anyMathMLStyling = false;
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1369985-1-ref.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<meta charset="UTF-8">
+<style>
+body {
+  padding: 0;
+  margin: 0;
+  font-size: 2px;
+  line-height: 1;
+}
+p {
+  padding: 0;
+  margin: 0;
+}
+</style>
+<body>
+<script>
+  let x = '<p>' + '\u2588'.repeat(100) + '</p>';
+  for (let i = 0; i < 99; i++) {
+    document.write(x)
+  }
+  document.write('<p>あ\nあ</p>');
+  for (let i = 0; i < 300; i++) {
+    document.write(x)
+  }
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1369985-1.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<meta charset="UTF-8">
+<style>
+body {
+  padding: 0;
+  margin: 0;
+  font-size: 2px;
+  line-height: 1;
+}
+</style>
+<body>
+<script>
+  // This is the constant in nsTextFrame.cpp of the number of lines
+  // which a text run can be built up to.
+  const NUM_LINES_TO_BUILD_TEXT_RUNS = 200;
+  // Since the font-size is small, we need lots of blocks so that it can
+  // be seen clearer whether difference is fuzzy or real failure.
+  let x = '\u2588'.repeat(100) + '<br>';
+  for (let i = 0; i < NUM_LINES_TO_BUILD_TEXT_RUNS / 2 - 1; i++) {
+    document.write(x)
+  }
+  // We want this piece to be in whatever place inside the first
+  // NUM_LINES_TO_BUILD_TEXT_RUNS lines.
+  document.write('あ\nあ<br>');
+  // And we want another NUM_LINES_TO_BUILD_TEXT_RUNS lines to check.
+  for (let i = 0; i < NUM_LINES_TO_BUILD_TEXT_RUNS * 1.5; i++) {
+    document.write(x)
+  }
+</script>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -2007,8 +2007,9 @@ fails-if(styloVsGecko) == 1322512-1.html
 == 1365159-1.html 1365159-1-ref.html
 fails-if(!stylo||styloVsGecko) == 1365162-1.html 1365162-1-ref.html
 == 1366144.html 1366144-ref.html
 == 1367592-1.html 1367592-1-ref.html
 == 1368113-1.html 1368113-1-ref.html
 == 1369584-1a.html 1369584-1-ref.html
 == 1369584-1b.html 1369584-1-ref.html
 == 1369954-1.xhtml 1369954-1-ref.xhtml
+== 1369985-1.html 1369985-1-ref.html