Backed out 3 changesets (bug 1573249) for chrome failures at layout/inspector/tests/chrome/test_fontFaceGeneric.xul on a CLOSED TREE
authorCoroiu Cristina <ccoroiu@mozilla.com>
Wed, 21 Aug 2019 14:01:11 +0300
changeset 489160 d7d41033f5615723ac638929850221c1d683d34c
parent 489159 c34ef6625ca789761d667572fc347039bdb8c08c
child 489161 76a17f3d411d2ae05ba9e73c597325708cb171cc
push id36465
push userdvarga@mozilla.com
push dateWed, 21 Aug 2019 16:47:43 +0000
treeherdermozilla-central@4ab60925635c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1573249
milestone70.0a1
backs outbdba80dae6e2e2ae62405f8d9a723d3a2834ef9e
21306e3f8d31af3d1957415acbe5ca1425cddb1a
3576d5b9f2db1053ca2f32181ac2a30b98eaeb7d
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out 3 changesets (bug 1573249) for chrome failures at layout/inspector/tests/chrome/test_fontFaceGeneric.xul on a CLOSED TREE Backed out changeset bdba80dae6e2 (bug 1573249) Backed out changeset 21306e3f8d31 (bug 1573249) Backed out changeset 3576d5b9f2db (bug 1573249)
gfx/thebes/gfxFont.cpp
gfx/thebes/gfxTextRun.cpp
gfx/thebes/gfxTextRun.h
gfx/thebes/gfxTypes.h
layout/generic/nsTextRunTransformations.cpp
layout/mathml/nsMathMLChar.cpp
layout/painting/nsCSSRendering.cpp
layout/reftests/text-decoration/reftest.list
layout/reftests/text-decoration/skip-ink-cjk-1-ref.html
layout/reftests/text-decoration/skip-ink-cjk-1.html
layout/reftests/text-decoration/skip-ink-cjk-2-ref.html
layout/reftests/text-decoration/skip-ink-cjk-2.html
layout/reftests/text-decoration/skip-ink-cjk-3-ref.html
layout/reftests/text-decoration/skip-ink-cjk-3.html
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -3197,18 +3197,16 @@ bool gfxFont::InitFakeSmallCapsRun(DrawT
   bool ok = true;
 
   RefPtr<gfxFont> smallCapsFont = GetSmallCapsFont();
   if (!smallCapsFont) {
     NS_WARNING("failed to get reduced-size font for smallcaps!");
     smallCapsFont = this;
   }
 
-  bool isCJK = gfxTextRun::IsCJKScript(aScript);
-
   enum RunCaseAction { kNoChange, kUppercaseReduce, kUppercase };
 
   RunCaseAction runAction = kNoChange;
   uint32_t runStart = 0;
 
   for (uint32_t i = 0; i <= aLength; ++i) {
     uint32_t extraCodeUnits = 0;  // Will be set to 1 if we need to consume
                                   // a trailing surrogate as well as the
@@ -3259,17 +3257,17 @@ bool gfxFont::InitFakeSmallCapsRun(DrawT
     // as that would be invalid in the case where i==length.
     if ((i == aLength || runAction != chAction) && runStart < i) {
       uint32_t runLength = i - runStart;
       gfxFont* f = this;
       switch (runAction) {
         case kNoChange:
           // just use the current font and the existing string
           aTextRun->AddGlyphRun(f, aMatchType, aOffset + runStart, true,
-                                aOrientation, isCJK);
+                                aOrientation);
           if (!f->SplitAndInitTextRun(aDrawTarget, aTextRun, aText + runStart,
                                       aOffset + runStart, runLength, aScript,
                                       aOrientation)) {
             ok = false;
           }
           break;
 
         case kUppercaseReduce:
@@ -3296,17 +3294,17 @@ bool gfxFont::InitFakeSmallCapsRun(DrawT
             // into the destination textrun but have to handle the
             // mismatch of character positions.
             gfxTextRunFactory::Parameters params = {
                 aDrawTarget, nullptr, nullptr,
                 nullptr,     0,       aTextRun->GetAppUnitsPerDevUnit()};
             RefPtr<gfxTextRun> tempRun(gfxTextRun::Create(
                 &params, convertedString.Length(), aTextRun->GetFontGroup(),
                 gfx::ShapedTextFlags(), nsTextFrameUtils::Flags()));
-            tempRun->AddGlyphRun(f, aMatchType, 0, true, aOrientation, isCJK);
+            tempRun->AddGlyphRun(f, aMatchType, 0, true, aOrientation);
             if (!f->SplitAndInitTextRun(
                     aDrawTarget, tempRun.get(), convertedString.BeginReading(),
                     0, convertedString.Length(), aScript, aOrientation)) {
               ok = false;
             } else {
               RefPtr<gfxTextRun> mergedRun(gfxTextRun::Create(
                   &params, runLength, aTextRun->GetFontGroup(),
                   gfx::ShapedTextFlags(), nsTextFrameUtils::Flags()));
@@ -3314,17 +3312,17 @@ bool gfxFont::InitFakeSmallCapsRun(DrawT
                                        charsToMergeArray.Elements(),
                                        deletedCharsArray.Elements());
               gfxTextRun::Range runRange(0, runLength);
               aTextRun->CopyGlyphDataFrom(mergedRun.get(), runRange,
                                           aOffset + runStart);
             }
           } else {
             aTextRun->AddGlyphRun(f, aMatchType, aOffset + runStart, true,
-                                  aOrientation, isCJK);
+                                  aOrientation);
             if (!f->SplitAndInitTextRun(
                     aDrawTarget, aTextRun, convertedString.BeginReading(),
                     aOffset + runStart, runLength, aScript, aOrientation)) {
               ok = false;
             }
           }
           break;
       }
--- a/gfx/thebes/gfxTextRun.cpp
+++ b/gfx/thebes/gfxTextRun.cpp
@@ -1240,111 +1240,135 @@ uint32_t gfxTextRun::FindFirstGlyphRunCo
       end = mid;
     }
   }
   NS_ASSERTION(mGlyphRunArray[start].mCharacterOffset <= aOffset,
                "Hmm, something went wrong, aOffset should have been found");
   return start;
 }
 
-void gfxTextRun::AddGlyphRun(gfxFont* aFont, FontMatchType aMatchType,
-                             uint32_t aUTF16Offset, bool aForceNewRun,
-                             gfx::ShapedTextFlags aOrientation, bool aIsCJK) {
+nsresult gfxTextRun::AddGlyphRun(gfxFont* aFont, FontMatchType aMatchType,
+                                 uint32_t aUTF16Offset, bool aForceNewRun,
+                                 gfx::ShapedTextFlags aOrientation) {
   NS_ASSERTION(aFont, "adding glyph run for null font!");
   NS_ASSERTION(aOrientation != gfx::ShapedTextFlags::TEXT_ORIENT_VERTICAL_MIXED,
                "mixed orientation should have been resolved");
   if (!aFont) {
-    return;
+    return NS_OK;
   }
   if (!mHasGlyphRunArray) {
     // We don't currently have an array.
     if (!mSingleGlyphRun.mFont) {
       // This is the first glyph run: just store it directly.
-      mSingleGlyphRun.SetProperties(aFont, aOrientation, aIsCJK, aMatchType);
+      mSingleGlyphRun.mFont = aFont;
+      mSingleGlyphRun.mMatchType = aMatchType;
+      mSingleGlyphRun.mOrientation = aOrientation;
       mSingleGlyphRun.mCharacterOffset = aUTF16Offset;
-      return;
+      return NS_OK;
     }
   }
   uint32_t numGlyphRuns = mHasGlyphRunArray ? mGlyphRunArray.Length() : 1;
   if (!aForceNewRun && numGlyphRuns > 0) {
     GlyphRun* lastGlyphRun = mHasGlyphRunArray
                                  ? &mGlyphRunArray[numGlyphRuns - 1]
                                  : &mSingleGlyphRun;
 
     NS_ASSERTION(lastGlyphRun->mCharacterOffset <= aUTF16Offset,
                  "Glyph runs out of order (and run not forced)");
 
     // Don't append a run if the font is already the one we want
-    if (lastGlyphRun->Matches(aFont, aOrientation, aIsCJK, aMatchType)) {
-      return;
+    if (lastGlyphRun->mFont == aFont &&
+        lastGlyphRun->mMatchType == aMatchType &&
+        lastGlyphRun->mOrientation == aOrientation) {
+      return NS_OK;
     }
 
     // If the offset has not changed, avoid leaving a zero-length run
     // by overwriting the last entry instead of appending...
     if (lastGlyphRun->mCharacterOffset == aUTF16Offset) {
       // ...except that if the run before the last entry had the same
       // font as the new one wants, merge with it instead of creating
       // adjacent runs with the same font
-      if (numGlyphRuns > 1 && mGlyphRunArray[numGlyphRuns - 2].Matches(
-                                  aFont, aOrientation, aIsCJK, aMatchType)) {
+      if (numGlyphRuns > 1 && mGlyphRunArray[numGlyphRuns - 2].mFont == aFont &&
+          mGlyphRunArray[numGlyphRuns - 2].mMatchType == aMatchType &&
+          mGlyphRunArray[numGlyphRuns - 2].mOrientation == aOrientation) {
         mGlyphRunArray.TruncateLength(numGlyphRuns - 1);
         if (mGlyphRunArray.Length() == 1) {
           ConvertFromGlyphRunArray();
         }
-        return;
+        return NS_OK;
       }
 
-      lastGlyphRun->SetProperties(aFont, aOrientation, aIsCJK, aMatchType);
-      return;
+      lastGlyphRun->mFont = aFont;
+      lastGlyphRun->mMatchType = aMatchType;
+      lastGlyphRun->mOrientation = aOrientation;
+      return NS_OK;
     }
   }
 
   NS_ASSERTION(
       aForceNewRun || numGlyphRuns > 0 || aUTF16Offset == 0,
       "First run doesn't cover the first character (and run not forced)?");
 
   if (!mHasGlyphRunArray) {
     ConvertToGlyphRunArray();
   }
 
   GlyphRun* glyphRun = mGlyphRunArray.AppendElement();
-  glyphRun->SetProperties(aFont, aOrientation, aIsCJK, aMatchType);
+  if (!glyphRun) {
+    if (mGlyphRunArray.Length() == 1) {
+      ConvertFromGlyphRunArray();
+    }
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  glyphRun->mFont = aFont;
   glyphRun->mCharacterOffset = aUTF16Offset;
+  glyphRun->mMatchType = aMatchType;
+  glyphRun->mOrientation = aOrientation;
+
+  return NS_OK;
 }
 
 void gfxTextRun::SortGlyphRuns() {
   if (!mHasGlyphRunArray) {
     return;
   }
 
   // We should never have an empty or one-element array here; if there's only
   // one glyphrun, it should be stored directly in the textrun without using
   // an array at all.
   MOZ_ASSERT(mGlyphRunArray.Length() > 1);
 
   AutoTArray<GlyphRun, 16> runs(std::move(mGlyphRunArray));
   GlyphRunOffsetComparator comp;
   runs.Sort(comp);
 
-  // Now copy back, coalescing adjacent glyph runs that have the same
-  // properties.
+  // Now copy back, coalescing adjacent glyph runs that have the same font
   mGlyphRunArray.Clear();
-  GlyphRun* prevRun = nullptr;
+  gfxFont* prevFont = nullptr;
+  gfx::ShapedTextFlags prevOrient = gfx::ShapedTextFlags();
+  DebugOnly<uint32_t> prevOffset = 0;
   for (auto& run : runs) {
-    // A GlyphRun with the same font and orientation as the previous can
+    // a GlyphRun with the same font and orientation as the previous can
     // just be skipped; the last GlyphRun will cover its character range.
     MOZ_ASSERT(run.mFont != nullptr);
-    if (!prevRun || !prevRun->Matches(run.mFont, run.mOrientation, run.mIsCJK,
-                                      run.mMatchType)) {
+    if (prevFont == nullptr || run.mFont != prevFont ||
+        run.mOrientation != prevOrient) {
       // If two fonts have the same character offset, Sort() will have
       // randomized the order.
-      MOZ_ASSERT(prevRun == nullptr ||
-                     prevRun->mCharacterOffset < run.mCharacterOffset,
+      MOZ_ASSERT(prevFont == nullptr || run.mCharacterOffset != prevOffset,
                  "Two fonts for the same run, glyph indices unreliable");
-      prevRun = mGlyphRunArray.AppendElement(std::move(run));
+      prevFont = run.mFont;
+      prevOrient = run.mOrientation;
+#ifdef DEBUG
+      prevOffset = run.mCharacterOffset;
+#endif
+      if (!mGlyphRunArray.AppendElement(std::move(run))) {
+        NS_WARNING("Failed to append glyph run!");
+      }
     }
   }
 
   MOZ_ASSERT(mGlyphRunArray.Length() > 0);
   if (mGlyphRunArray.Length() == 1) {
     ConvertFromGlyphRunArray();
   }
 }
@@ -1458,27 +1482,26 @@ void gfxTextRun::CopyGlyphDataFrom(gfxTe
       }
     }
     dstGlyphs[i] = g;
   }
 
   // Copy glyph runs
   GlyphRunIterator iter(aSource, aRange);
 #ifdef DEBUG
-  GlyphRun* prevRun = nullptr;
+  const GlyphRun* prevRun = nullptr;
 #endif
   while (iter.NextRun()) {
     gfxFont* font = iter.GetGlyphRun()->mFont;
-    MOZ_ASSERT(!prevRun || !prevRun->Matches(iter.GetGlyphRun()->mFont,
-                                             iter.GetGlyphRun()->mOrientation,
-                                             iter.GetGlyphRun()->mIsCJK,
-                                             FontMatchType::Kind::kUnspecified),
-               "Glyphruns not coalesced?");
+    NS_ASSERTION(!prevRun || prevRun->mFont != iter.GetGlyphRun()->mFont ||
+                     prevRun->mMatchType != iter.GetGlyphRun()->mMatchType ||
+                     prevRun->mOrientation != iter.GetGlyphRun()->mOrientation,
+                 "Glyphruns not coalesced?");
 #ifdef DEBUG
-    prevRun = const_cast<GlyphRun*>(iter.GetGlyphRun());
+    prevRun = iter.GetGlyphRun();
     uint32_t end = iter.GetStringEnd();
 #endif
     uint32_t start = iter.GetStringStart();
 
     // These used to be NS_ASSERTION()s, but WARNING is more appropriate.
     // Although it's unusual (and not desirable), it's possible for us to assign
     // different fonts to a base character and a following diacritic.
     // Example on OSX 10.5/10.6 with default fonts installed:
@@ -1488,19 +1511,20 @@ void gfxTextRun::CopyGlyphDataFrom(gfxTe
     // but it's the best we can do for now if the specified font only covered
     // the initial base character and not its applied marks.
     NS_WARNING_ASSERTION(aSource->IsClusterStart(start),
                          "Started font run in the middle of a cluster");
     NS_WARNING_ASSERTION(
         end == aSource->GetLength() || aSource->IsClusterStart(end),
         "Ended font run in the middle of a cluster");
 
-    AddGlyphRun(font, iter.GetGlyphRun()->mMatchType,
-                start - aRange.start + aDest, false,
-                iter.GetGlyphRun()->mOrientation, iter.GetGlyphRun()->mIsCJK);
+    nsresult rv = AddGlyphRun(font, iter.GetGlyphRun()->mMatchType,
+                              start - aRange.start + aDest, false,
+                              iter.GetGlyphRun()->mOrientation);
+    if (NS_FAILED(rv)) return;
   }
 }
 
 void gfxTextRun::ClearGlyphsAndCharacters() {
   ResetGlyphRuns();
   memset(reinterpret_cast<char*>(mCharacterGlyphs), 0,
          mLength * sizeof(CompressedGlyph));
   mDetailedGlyphs = nullptr;
@@ -1520,23 +1544,18 @@ void gfxTextRun::SetSpaceGlyph(gfxFont* 
   bool vertical =
       !!(GetFlags() & gfx::ShapedTextFlags::TEXT_ORIENT_VERTICAL_UPRIGHT);
   gfxFontShaper::RoundingFlags roundingFlags =
       aFont->GetRoundOffsetsToPixels(aDrawTarget);
   gfxShapedWord* sw = aFont->GetShapedWord(
       aDrawTarget, &space, 1, gfxShapedWord::HashMix(0, ' '), Script::LATIN,
       vertical, mAppUnitsPerDevUnit, flags, roundingFlags, nullptr);
   if (sw) {
-    const GlyphRun* prevRun = TrailingGlyphRun();
-    bool isCJK = prevRun && prevRun->mFont == aFont &&
-                         prevRun->mOrientation == aOrientation
-                     ? prevRun->mIsCJK
-                     : false;
-    AddGlyphRun(aFont, FontMatchType::Kind::kUnspecified, aCharIndex, false,
-                aOrientation, isCJK);
+    AddGlyphRun(aFont, FontMatchType::Kind::kFontGroup, aCharIndex, false,
+                aOrientation);
     CopyGlyphDataFrom(sw, aCharIndex);
     GetCharacterGlyphs()[aCharIndex].SetIsSpace();
   }
 }
 
 bool gfxTextRun::SetSpaceGlyphIfSimple(gfxFont* aFont, uint32_t aCharIndex,
                                        char16_t aSpaceChar,
                                        gfx::ShapedTextFlags aOrientation) {
@@ -1550,23 +1569,18 @@ bool gfxTextRun::SetSpaceGlyphIfSimple(g
           ? nsFontMetrics::eVertical
           : nsFontMetrics::eHorizontal;
   uint32_t spaceWidthAppUnits = NS_lroundf(
       aFont->GetMetrics(fontOrientation).spaceWidth * mAppUnitsPerDevUnit);
   if (!CompressedGlyph::IsSimpleAdvance(spaceWidthAppUnits)) {
     return false;
   }
 
-  const GlyphRun* prevRun = TrailingGlyphRun();
-  bool isCJK = prevRun && prevRun->mFont == aFont &&
-                       prevRun->mOrientation == aOrientation
-                   ? prevRun->mIsCJK
-                   : false;
-  AddGlyphRun(aFont, FontMatchType::Kind::kUnspecified, aCharIndex, false,
-              aOrientation, isCJK);
+  AddGlyphRun(aFont, FontMatchType::Kind::kFontGroup, aCharIndex, false,
+              aOrientation);
   CompressedGlyph g =
       CompressedGlyph::MakeSimpleGlyph(spaceWidthAppUnits, spaceGlyph);
   if (aSpaceChar == ' ') {
     g.SetIsSpace();
   }
   GetCharacterGlyphs()[aCharIndex] = g;
   return true;
 }
@@ -2144,18 +2158,18 @@ already_AddRefed<gfxTextRun> gfxFontGrou
   }
 
   gfxFont* font = GetFirstValidFont();
   if (MOZ_UNLIKELY(GetStyle()->size == 0) ||
       MOZ_UNLIKELY(GetStyle()->sizeAdjust == 0.0f)) {
     // Short-circuit for size-0 fonts, as Windows and ATSUI can't handle
     // them, and always create at least size 1 fonts, i.e. they still
     // render something for size 0 fonts.
-    textRun->AddGlyphRun(font, FontMatchType::Kind::kUnspecified, 0, false,
-                         orientation, false);
+    textRun->AddGlyphRun(font, FontMatchType::Kind::kFontGroup, 0, false,
+                         orientation);
   } else {
     if (font->GetSpaceGlyph()) {
       // Normally, the font has a cached space glyph, so we can avoid
       // the cost of calling FindFontForChar.
       textRun->SetSpaceGlyph(font, aParams->mDrawTarget, 0, orientation);
     } else {
       // In case the primary font doesn't have <space> (bug 970891),
       // find one that does.
@@ -2182,18 +2196,18 @@ already_AddRefed<gfxTextRun> gfxFontGrou
   if (!textRun) {
     return nullptr;
   }
 
   gfx::ShapedTextFlags orientation = aFlags & ShapedTextFlags::TEXT_ORIENT_MASK;
   if (orientation == ShapedTextFlags::TEXT_ORIENT_VERTICAL_MIXED) {
     orientation = ShapedTextFlags::TEXT_ORIENT_VERTICAL_UPRIGHT;
   }
-  textRun->AddGlyphRun(GetFirstValidFont(), FontMatchType::Kind::kUnspecified,
-                       0, false, orientation, false);
+  textRun->AddGlyphRun(GetFirstValidFont(), FontMatchType::Kind::kFontGroup, 0,
+                       false, orientation);
   return textRun.forget();
 }
 
 already_AddRefed<gfxTextRun> gfxFontGroup::MakeHyphenTextRun(
     DrawTarget* aDrawTarget, uint32_t aAppUnitsPerDevUnit) {
   // only use U+2010 if it is supported by the first font in the group;
   // it's better to use ASCII '-' from the primary font than to fall back to
   // U+2010 from some other, possibly poorly-matching face
@@ -2481,28 +2495,27 @@ void gfxFontGroup::InitScriptRun(DrawTar
     orientation = ShapedTextFlags::TEXT_ORIENT_VERTICAL_SIDEWAYS_RIGHT;
   }
 
   uint32_t runStart = 0;
   AutoTArray<TextRange, 3> fontRanges;
   ComputeRanges(fontRanges, aString, aLength, aRunScript, orientation);
   uint32_t numRanges = fontRanges.Length();
   bool missingChars = false;
-  bool isCJK = gfxTextRun::IsCJKScript(aRunScript);
 
   for (uint32_t r = 0; r < numRanges; r++) {
     const TextRange& range = fontRanges[r];
     uint32_t matchedLength = range.Length();
     gfxFont* matchedFont = range.font;
     // create the glyph run for this range
     if (matchedFont && mStyle.noFallbackVariantFeatures) {
       // common case - just do glyph layout and record the
       // resulting positioned glyphs
       aTextRun->AddGlyphRun(matchedFont, range.matchType, aOffset + runStart,
-                            (matchedLength > 0), range.orientation, isCJK);
+                            (matchedLength > 0), range.orientation);
       if (!matchedFont->SplitAndInitTextRun(
               aDrawTarget, aTextRun, aString + runStart, aOffset + runStart,
               matchedLength, aRunScript, range.orientation)) {
         // glyph layout failed! treat as missing glyphs
         matchedFont = nullptr;
       }
     } else if (matchedFont) {
       // shape with some variant feature that requires fallback handling
@@ -2527,17 +2540,17 @@ void gfxFontGroup::InitScriptRun(DrawTar
         } else if (ss == gfxTextRun::eShapingState_ShapingWithFeature) {
           aTextRun->SetShapingState(gfxTextRun::eShapingState_Aborted);
           return;
         }
 
         RefPtr<gfxFont> subSuperFont = matchedFont->GetSubSuperscriptFont(
             aTextRun->GetAppUnitsPerDevUnit());
         aTextRun->AddGlyphRun(subSuperFont, range.matchType, aOffset + runStart,
-                              (matchedLength > 0), range.orientation, isCJK);
+                              (matchedLength > 0), range.orientation);
         if (!subSuperFont->SplitAndInitTextRun(
                 aDrawTarget, aTextRun, aString + runStart, aOffset + runStart,
                 matchedLength, aRunScript, range.orientation)) {
           // glyph layout failed! treat as missing glyphs
           matchedFont = nullptr;
         }
       } else if (mStyle.variantCaps != NS_FONT_VARIANT_CAPS_NORMAL &&
                  !matchedFont->SupportsVariantCaps(
@@ -2561,28 +2574,28 @@ void gfxFontGroup::InitScriptRun(DrawTar
         } else if (ss == gfxTextRun::eShapingState_ShapingWithFallback) {
           // already have shaping results using fallback, need to redo
           aTextRun->SetShapingState(gfxTextRun::eShapingState_Aborted);
           return;
         }
 
         // do glyph layout and record the resulting positioned glyphs
         aTextRun->AddGlyphRun(matchedFont, range.matchType, aOffset + runStart,
-                              (matchedLength > 0), range.orientation, isCJK);
+                              (matchedLength > 0), range.orientation);
         if (!matchedFont->SplitAndInitTextRun(
                 aDrawTarget, aTextRun, aString + runStart, aOffset + runStart,
                 matchedLength, aRunScript, range.orientation)) {
           // glyph layout failed! treat as missing glyphs
           matchedFont = nullptr;
         }
       }
     } else {
       aTextRun->AddGlyphRun(mainFont, FontMatchType::Kind::kFontGroup,
                             aOffset + runStart, (matchedLength > 0),
-                            range.orientation, isCJK);
+                            range.orientation);
     }
 
     if (!matchedFont) {
       // We need to set cluster boundaries (and mark spaces) so that
       // surrogate pairs, combining characters, etc behave properly,
       // even if we don't have glyphs for them
       aTextRun->SetupClusterBoundaries(aOffset + runStart, aString + runStart,
                                        matchedLength);
--- a/gfx/thebes/gfxTextRun.h
+++ b/gfx/thebes/gfxTextRun.h
@@ -466,62 +466,18 @@ class gfxTextRun : public gfxShapedText 
   // The text is divided into GlyphRuns as necessary. (In the vast majority
   // of cases, a gfxTextRun contains just a single GlyphRun.)
   struct GlyphRun {
     RefPtr<gfxFont> mFont;      // never null in a valid GlyphRun
     uint32_t mCharacterOffset;  // into original UTF16 string
     mozilla::gfx::ShapedTextFlags
         mOrientation;  // gfxTextRunFactory::TEXT_ORIENT_* value
     FontMatchType mMatchType;
-    bool mIsCJK;  // Whether the text was a CJK script run (used to decide if
-                  // text-decoration-skip-ink should not be applied)
-
-    // Set up the properties (but NOT offset) of the GlyphRun.
-    void SetProperties(gfxFont* aFont,
-                       mozilla::gfx::ShapedTextFlags aOrientation, bool aIsCJK,
-                       FontMatchType aMatchType) {
-      mFont = aFont;
-      mOrientation = aOrientation;
-      mIsCJK = aIsCJK;
-      mMatchType = aMatchType;
-    }
-
-    // Return whether the GlyphRun matches the given properties;
-    // the given FontMatchType will be added to the run if not present.
-    bool Matches(gfxFont* aFont, mozilla::gfx::ShapedTextFlags aOrientation,
-                 bool aIsCJK, FontMatchType aMatchType) {
-      if (mFont == aFont && mOrientation == aOrientation && mIsCJK == aIsCJK) {
-        mMatchType |= aMatchType;
-        return true;
-      }
-      return false;
-    }
   };
 
-  // Script run codes that we will mark as CJK to suppress skip-ink behavior.
-  static inline bool IsCJKScript(Script aScript) {
-    switch (aScript) {
-      case Script::BOPOMOFO:
-      case Script::HAN:
-      case Script::HANGUL:
-      case Script::HIRAGANA:
-      case Script::KATAKANA:
-      case Script::KATAKANA_OR_HIRAGANA:
-      case Script::SIMPLIFIED_HAN:
-      case Script::TRADITIONAL_HAN:
-      case Script::JAPANESE:
-      case Script::KOREAN:
-      case Script::HAN_WITH_BOPOMOFO:
-      case Script::JAMO:
-        return true;
-      default:
-        return false;
-    }
-  }
-
   class MOZ_STACK_CLASS GlyphRunIterator {
    public:
     GlyphRunIterator(const gfxTextRun* aTextRun, Range aRange,
                      bool aReverse = false)
         : mTextRun(aTextRun),
           mDirection(aReverse ? -1 : 1),
           mStartOffset(aRange.start),
           mEndOffset(aRange.end) {
@@ -568,19 +524,19 @@ class gfxTextRun : public gfxShapedText 
    *
    * If aForceNewRun, a new glyph run will be added, even if the
    * previously added run uses the same font.  If glyph runs are
    * added out of strictly increasing aStartCharIndex order (via
    * force), then SortGlyphRuns must be called after all glyph runs
    * are added before any further operations are performed with this
    * TextRun.
    */
-  void AddGlyphRun(gfxFont* aFont, FontMatchType aMatchType,
-                   uint32_t aUTF16Offset, bool aForceNewRun,
-                   mozilla::gfx::ShapedTextFlags aOrientation, bool aIsCJK);
+  nsresult AddGlyphRun(gfxFont* aFont, FontMatchType aMatchType,
+                       uint32_t aUTF16Offset, bool aForceNewRun,
+                       mozilla::gfx::ShapedTextFlags aOrientation);
   void ResetGlyphRuns() {
     if (mHasGlyphRunArray) {
       MOZ_ASSERT(mGlyphRunArray.Length() > 1);
       // Discard all but the first GlyphRun...
       mGlyphRunArray.TruncateLength(1);
       // ...and then convert to the single-run representation.
       ConvertFromGlyphRunArray();
     }
@@ -654,23 +610,16 @@ class gfxTextRun : public gfxShapedText 
     if (mHasGlyphRunArray) {
       *aNumGlyphRuns = mGlyphRunArray.Length();
       return mGlyphRunArray.Elements();
     } else {
       *aNumGlyphRuns = mSingleGlyphRun.mFont ? 1 : 0;
       return &mSingleGlyphRun;
     }
   }
-
-  const GlyphRun* TrailingGlyphRun() const {
-    uint32_t count;
-    const GlyphRun* runs = GetGlyphRuns(&count);
-    return count ? runs + count - 1 : nullptr;
-  }
-
   // Returns the index of the GlyphRun containing the given offset.
   // Returns mGlyphRuns.Length() when aOffset is mCharacterCount.
   uint32_t FindFirstGlyphRunContaining(uint32_t aOffset) const;
 
   // Copy glyph data from a ShapedWord into this textrun.
   void CopyGlyphDataFrom(gfxShapedWord* aSource, uint32_t aStart);
 
   // Copy glyph data for a range of characters from aSource to this
--- a/gfx/thebes/gfxTypes.h
+++ b/gfx/thebes/gfxTypes.h
@@ -85,17 +85,16 @@ enum class gfxAlphaType {
 };
 
 /**
  * Type used to record how a particular font is selected during the font-
  * matching process, so that this can be exposed to the Inspector.
  */
 struct FontMatchType {
   enum class Kind : uint8_t {
-    kUnspecified = 0,
     kFontGroup = 1,
     kPrefsFallback = 1 << 1,
     kSystemFallback = 1 << 2,
   };
 
   inline FontMatchType& operator|=(const FontMatchType& aOther);
 
   bool operator==(const FontMatchType& aOther) const {
--- a/layout/generic/nsTextRunTransformations.cpp
+++ b/layout/generic/nsTextRunTransformations.cpp
@@ -128,18 +128,21 @@ void MergeCharactersInTextRun(gfxTextRun
 
   gfxTextRun::GlyphRunIterator iter(aSrc, gfxTextRun::Range(aSrc));
   uint32_t offset = 0;
   AutoTArray<gfxTextRun::DetailedGlyph, 2> glyphs;
   const gfxTextRun::CompressedGlyph continuationGlyph =
       gfxTextRun::CompressedGlyph::MakeComplex(false, false, 0);
   while (iter.NextRun()) {
     const gfxTextRun::GlyphRun* run = iter.GetGlyphRun();
-    aDest->AddGlyphRun(run->mFont, run->mMatchType, offset, false,
-                       run->mOrientation, run->mIsCJK);
+    nsresult rv = aDest->AddGlyphRun(run->mFont, run->mMatchType, offset, false,
+                                     run->mOrientation);
+    if (NS_FAILED(rv)) {
+      return;
+    }
 
     bool anyMissing = false;
     uint32_t mergeRunStart = iter.GetStringStart();
     const gfxTextRun::CompressedGlyph* srcGlyphs = aSrc->GetCharacterGlyphs();
     gfxTextRun::CompressedGlyph mergedGlyph = srcGlyphs[mergeRunStart];
     uint32_t stringEnd = iter.GetStringEnd();
     for (uint32_t k = iter.GetStringStart(); k < stringEnd; ++k) {
       const gfxTextRun::CompressedGlyph g = srcGlyphs[k];
--- a/layout/mathml/nsMathMLChar.cpp
+++ b/layout/mathml/nsMathMLChar.cpp
@@ -514,17 +514,17 @@ already_AddRefed<gfxTextRun> nsOpenTypeT
 
   gfxTextRunFactory::Parameters params = {
       aDrawTarget, nullptr, nullptr, nullptr, 0, aAppUnitsPerDevPixel};
   RefPtr<gfxTextRun> textRun =
       gfxTextRun::Create(&params, 1, aFontGroup, gfx::ShapedTextFlags(),
                          nsTextFrameUtils::Flags());
   textRun->AddGlyphRun(aFontGroup->GetFirstValidFont(),
                        FontMatchType::Kind::kFontGroup, 0, false,
-                       gfx::ShapedTextFlags::TEXT_ORIENT_HORIZONTAL, false);
+                       gfx::ShapedTextFlags::TEXT_ORIENT_HORIZONTAL);
   // We don't care about CSS writing mode here;
   // math runs are assumed to be horizontal.
   gfxTextRun::DetailedGlyph detailedGlyph;
   detailedGlyph.mGlyphID = aGlyph.glyphID;
   detailedGlyph.mAdvance = NSToCoordRound(
       aAppUnitsPerDevPixel * aFontGroup->GetFirstValidFont()->GetGlyphHAdvance(
                                  aDrawTarget, aGlyph.glyphID));
   textRun->SetGlyphs(0,
--- a/layout/painting/nsCSSRendering.cpp
+++ b/layout/painting/nsCSSRendering.cpp
@@ -4095,25 +4095,21 @@ void nsCSSRendering::PaintDecorationLine
   // loop through each glyph run
   // in most cases there will only be one
   bool isRTL = textRun->IsRightToLeft();
   int32_t spacingOffset = isRTL ? aParams.glyphRange.Length() - 1 : 0;
   gfxTextRun::GlyphRunIterator iter(textRun, aParams.glyphRange, isRTL);
 
   while (iter.NextRun()) {
     if (iter.GetGlyphRun()->mOrientation ==
-            mozilla::gfx::ShapedTextFlags::TEXT_ORIENT_VERTICAL_UPRIGHT ||
-        iter.GetGlyphRun()->mIsCJK) {
+        mozilla::gfx::ShapedTextFlags::TEXT_ORIENT_VERTICAL_UPRIGHT) {
       // We don't support upright text in vertical modes currently
       // (see https://bugzilla.mozilla.org/show_bug.cgi?id=1572294),
       // but we do need to update textPos so that following runs will be
       // correctly positioned.
-      // We also don't apply skip-ink to CJK text runs because many fonts
-      // have an underline that looks really bad if this is done
-      // (see https://bugzilla.mozilla.org/show_bug.cgi?id=1573249).
       textPos.fX +=
           textRun->GetAdvanceWidth(
               gfxTextRun::Range(iter.GetStringStart(), iter.GetStringEnd()),
               aParams.provider) /
           appUnitsPerDevPixel;
       continue;
     }
 
--- a/layout/reftests/text-decoration/reftest.list
+++ b/layout/reftests/text-decoration/reftest.list
@@ -113,11 +113,8 @@ fuzzy-if(skiaContent,0-4,0-2) == underli
 == vertical-mode-decorations-1.html vertical-mode-decorations-1-ref.html
 fuzzy-if(Android,0-238,0-36) == vertical-mode-decorations-2.html vertical-mode-decorations-2-ref.html
 != 1415214.html 1415214-notref.html
 test-pref(layout.css.text-decoration-thickness.enabled,false) == text-decoration-shorthands-001.html text-decoration-shorthands-001-ref.html
 # fails because of bug 1572302
 test-pref(layout.css.text-decoration-skip-ink.enabled,true) test-pref(layout.css.text-underline-offset.enabled,true) fails HTTP(..) == skip-ink-multiline-position.html skip-ink-multiline-position-ref.html
 # fails due to bug 1573711
 test-pref(layout.css.text-decoration-skip-ink.enabled,true) fails == skip-ink-vertical-align.html skip-ink-vertical-align-ref.html
-pref(layout.css.text-decoration-skip-ink.enabled,true) fuzzy(0-94,0-4) == skip-ink-cjk-1.html skip-ink-cjk-1-ref.html
-pref(layout.css.text-decoration-skip-ink.enabled,true) == skip-ink-cjk-2.html skip-ink-cjk-2-ref.html
-pref(layout.css.text-decoration-skip-ink.enabled,true) == skip-ink-cjk-3.html skip-ink-cjk-3-ref.html
deleted file mode 100644
--- a/layout/reftests/text-decoration/skip-ink-cjk-1-ref.html
+++ /dev/null
@@ -1,18 +0,0 @@
-<!DOCTYPE html>
-<html lang="ja">
-<meta charset="utf-8">
-<style>
-body {
-  font: 16px/2 sans-serif;
-}
-.test {
-  font: 32px sans-serif;
-  text-decoration: 2px red underline;
-  text-underline-offset: 1px;
-}
-span {
-  text-decoration-skip-ink: none;
-}
-</style>
-<div>The underline should skip descenders on the English but not the Japanese:</div>
-<div class=test>skipping <span>土砂災害警戒情報</span> skipping</div>
deleted file mode 100644
--- a/layout/reftests/text-decoration/skip-ink-cjk-1.html
+++ /dev/null
@@ -1,15 +0,0 @@
-<!DOCTYPE html>
-<html lang="ja">
-<meta charset="utf-8">
-<style>
-body {
-  font: 16px/2 sans-serif;
-}
-.test {
-  font: 32px sans-serif;
-  text-decoration: 2px red underline;
-  text-underline-offset: 1px;
-}
-</style>
-<div>The underline should skip descenders on the English but not the Japanese:</div>
-<div class=test>skipping 土砂災害警戒情報 skipping</div>
deleted file mode 100644
--- a/layout/reftests/text-decoration/skip-ink-cjk-2-ref.html
+++ /dev/null
@@ -1,19 +0,0 @@
-<!DOCTYPE html>
-<html lang="ja">
-<!-- lang=ja will cause the default japanese font to be used throughout -->
-<meta charset="utf-8">
-<style>
-body {
-  font: 16px/2 sans-serif;
-}
-.test {
-  font: 32px sans-serif;
-  text-decoration: 2px red underline;
-  text-underline-offset: -1px; /* make underline clash with digits etc */
-  text-decoration-skip-ink: none;
-}
-</style>
-<div>The underline should not skip-ink in Japanese runs, even on embedded "common" characters:</div>
-<div class=test>黄砂情報 実況図 / 予測図</div>
-<div class=test>気象衛星 10分ごと / 2.5分ごと</div>
-<div class=test>レーダー•ナウキャスト(降水•雷•竜巻)</div>
deleted file mode 100644
--- a/layout/reftests/text-decoration/skip-ink-cjk-2.html
+++ /dev/null
@@ -1,18 +0,0 @@
-<!DOCTYPE html>
-<html lang="ja">
-<!-- lang=ja will cause the default japanese font to be used throughout -->
-<meta charset="utf-8">
-<style>
-body {
-  font: 16px/2 sans-serif;
-}
-.test {
-  font: 32px sans-serif;
-  text-decoration: 2px red underline;
-  text-underline-offset: -1px; /* make underline clash with digits etc */
-}
-</style>
-<div>The underline should not skip-ink in Japanese runs, even on embedded "common" characters:</div>
-<div class=test>黄砂情報 実況図 / 予測図</div>
-<div class=test>気象衛星 10分ごと / 2.5分ごと</div>
-<div class=test>レーダー•ナウキャスト(降水•雷•竜巻)</div>
deleted file mode 100644
--- a/layout/reftests/text-decoration/skip-ink-cjk-3-ref.html
+++ /dev/null
@@ -1,19 +0,0 @@
-<!DOCTYPE html>
-<html lang="en">
-<!-- lang=en will result in a mix of western and japanese fonts -->
-<meta charset="utf-8">
-<style>
-body {
-  font: 16px/2 sans-serif;
-}
-.test {
-  font: 32px sans-serif;
-  text-decoration: 2px red underline;
-  text-underline-offset: -1px; /* make underline clash with digits etc */
-  text-decoration-skip-ink: none;
-}
-</style>
-<div>The underline should not skip-ink in Japanese runs, even on embedded "common" characters:</div>
-<div class=test>黄砂情報 実況図 / 予測図</div>
-<div class=test>気象衛星 10分ごと / 2.5分ごと</div>
-<div class=test>レーダー•ナウキャスト(降水•雷•竜巻)</div>
deleted file mode 100644
--- a/layout/reftests/text-decoration/skip-ink-cjk-3.html
+++ /dev/null
@@ -1,18 +0,0 @@
-<!DOCTYPE html>
-<html lang="en">
-<!-- lang=en will result in a mix of western and japanese fonts -->
-<meta charset="utf-8">
-<style>
-body {
-  font: 16px/2 sans-serif;
-}
-.test {
-  font: 32px sans-serif;
-  text-decoration: 2px red underline;
-  text-underline-offset: -1px; /* make underline clash with digits etc */
-}
-</style>
-<div>The underline should not skip-ink in Japanese runs, even on embedded "common" characters:</div>
-<div class=test>黄砂情報 実況図 / 予測図</div>
-<div class=test>気象衛星 10分ごと / 2.5分ごと</div>
-<div class=test>レーダー•ナウキャスト(降水•雷•竜巻)</div>