[gfx/layout] Simplify textruns

Cache overflow textruns in a better place and stop using a lazy class.
This avoids doing caching in the rendering back-end, and does so where
necessary in TextOverflow (for ellipses)

gfx/thebes/gfxTextRun.cpp

@@ -147,7 +147,6 @@ gfxTextRun::gfxTextRun(const gfxTextRunFactory::Parameters *aParams,
     : gfxShapedText(aLength, aFlags, aParams->mAppUnitsPerDevUnit)
     , mUserData(aParams->mUserData)
     , mFontGroup(aFontGroup)
-    , mReleasedFontGroup(false)
     , mShapingState(eShapingState_Normal)
 {
     NS_ASSERTION(mAppUnitsPerDevUnit > 0, "Invalid app unit scale");
@@ -184,30 +183,18 @@ gfxTextRun::~gfxTextRun()
     mFlags = 0xFFFFFFFF;
 #endif
 
-    // The cached ellipsis textrun (if any) in a fontgroup will have already
-    // been told to release its reference to the group, so we mustn't do that
-    // again here.
-    if (!mReleasedFontGroup) {
 #ifdef DEBUG
-        gfxTextPerfMetrics *tp = mFontGroup->GetTextPerfMetrics();
-        if (tp) {
-            tp->current.textrunDestr++;
-        }
-#endif
-        NS_RELEASE(mFontGroup);
+    gfxTextPerfMetrics* tp = mFontGroup->GetTextPerfMetrics();
+    if (tp) {
+        tp->current.textrunDestr++;
     }
+#endif
+
+    NS_RELEASE(mFontGroup);
 
     MOZ_COUNT_DTOR(gfxTextRun);
 }
 
-void
-gfxTextRun::ReleaseFontGroup()
-{
-    NS_ASSERTION(!mReleasedFontGroup, "doubly released!");
-    NS_RELEASE(mFontGroup);
-    mReleasedFontGroup = true;
-}
-
 bool
 gfxTextRun::SetPotentialLineBreaks(Range aRange, const uint8_t* aBreakBefore)
 {
@@ -2541,17 +2528,11 @@ gfxFontGroup::InitScriptRun(DrawTarget* aDrawTarget,
     }
 }
 
-gfxTextRun *
-gfxFontGroup::GetEllipsisTextRun(int32_t aAppUnitsPerDevPixel, uint32_t aFlags,
-                                 LazyReferenceDrawTargetGetter& aRefDrawTargetGetter)
-{
+already_AddRefed<gfxTextRun>
+gfxFontGroup::MakeEllipsisTextRun(int32_t aAppUnitsPerDevPixel, uint32_t aFlags,
+                                 DrawTarget* aRefDrawTarget) {
     MOZ_ASSERT(!(aFlags & ~TEXT_ORIENT_MASK),
                "flags here should only be used to specify orientation");
-    if (mCachedEllipsisTextRun &&
-        (mCachedEllipsisTextRun->GetFlags() & TEXT_ORIENT_MASK) == aFlags &&
-        mCachedEllipsisTextRun->GetAppUnitsPerDevUnit() == aAppUnitsPerDevPixel) {
-        return mCachedEllipsisTextRun.get();
-    }
 
     // Use a Unicode ellipsis if the font supports it,
     // otherwise use three ASCII periods as fallback.
@@ -2562,20 +2543,11 @@ gfxFontGroup::GetEllipsisTextRun(int32_t aAppUnitsPerDevPixel, uint32_t aFlags,
         : nsDependentString(kASCIIPeriodsChar,
                             ArrayLength(kASCIIPeriodsChar) - 1);
 
-    RefPtr<DrawTarget> refDT = aRefDrawTargetGetter.GetRefDrawTarget();
     Parameters params = {
-        refDT, nullptr, nullptr, nullptr, 0, aAppUnitsPerDevPixel
+        aRefDrawTarget, nullptr, nullptr, nullptr, 0, aAppUnitsPerDevPixel
     };
-    mCachedEllipsisTextRun =
-        MakeTextRun(ellipsis.get(), ellipsis.Length(), &params,
-                    aFlags | TEXT_IS_PERSISTENT, nullptr);
-    if (!mCachedEllipsisTextRun) {
-        return nullptr;
-    }
-    // don't let the presence of a cached ellipsis textrun prolong the
-    // fontgroup's life
-    mCachedEllipsisTextRun->ReleaseFontGroup();
-    return mCachedEllipsisTextRun.get();
+    return MakeTextRun(ellipsis.get(), ellipsis.Length(), &params,
+                       aFlags | TEXT_IS_PERSISTENT, nullptr);
 }
 
 already_AddRefed<gfxFont>

gfx/thebes/gfxTextRun.h

@@ -755,8 +755,6 @@ private:
     bool              mSkipDrawing; // true if the font group we used had a user font
                                     // download that's in progress, so we should hide text
                                     // until the download completes (or timeout fires)
-    bool              mReleasedFontGroup; // we already called NS_RELEASE on
-                                          // mFontGroup, so don't do it again
 
     // shaping state for handling variant fallback features
     // such as subscript/superscript variant glyphs
@@ -897,7 +895,6 @@ public:
         mUnderlineOffset = UNDERLINE_OFFSET_NOT_SET;
         mSkipDrawing = false;
         mHyphenWidth = -1;
-        mCachedEllipsisTextRun = nullptr;
     }
 
     // If there is a user font set, check to see whether the font list or any
@@ -911,17 +908,11 @@ public:
         return mSkipDrawing;
     }
 
-    class LazyReferenceDrawTargetGetter {
-    public:
-      virtual already_AddRefed<DrawTarget> GetRefDrawTarget() = 0;
-    };
-    // The gfxFontGroup keeps ownership of this textrun.
-    // It is only guaranteed to exist until the next call to GetEllipsisTextRun
-    // (which might use a different appUnitsPerDev value or flags) for the font
-    // group, or until UpdateUserFonts is called, or the fontgroup is destroyed.
-    // Get it/use it/forget it :) - don't keep a reference that might go stale.
-    gfxTextRun* GetEllipsisTextRun(int32_t aAppUnitsPerDevPixel, uint32_t aFlags,
-                                   LazyReferenceDrawTargetGetter& aRefDrawTargetGetter);
+    // Make a textrun for the ellipsis character (with fallback to "..." if
+    // ellipsis is not supported by the font).
+    already_AddRefed<gfxTextRun> MakeEllipsisTextRun(int32_t aAppUnitsPerDevPixel,
+                                                     uint32_t aFlags,
+                                                     DrawTarget* aRefDrawTarget);
 
 protected:
     // search through pref fonts for a character, return nullptr if no matching pref font
@@ -1093,10 +1084,6 @@ protected:
 
     gfxTextPerfMetrics *mTextPerf;
 
-    // Cache a textrun representing an ellipsis (useful for CSS text-overflow)
-    // at a specific appUnitsPerDevPixel size and orientation
-    RefPtr<gfxTextRun>   mCachedEllipsisTextRun;
-
     // cache the most recent pref font to avoid general pref font lookup
     RefPtr<gfxFontFamily> mLastPrefFamily;
     RefPtr<gfxFont>       mLastPrefFont;

layout/generic/TextOverflow.cpp

@@ -27,34 +27,15 @@
 namespace mozilla {
 namespace css {
 
-class LazyReferenceRenderingDrawTargetGetterFromFrame final :
-    public gfxFontGroup::LazyReferenceDrawTargetGetter {
-public:
-  typedef mozilla::gfx::DrawTarget DrawTarget;
-
-  explicit LazyReferenceRenderingDrawTargetGetterFromFrame(nsIFrame* aFrame)
-    : mFrame(aFrame) {}
-  virtual already_AddRefed<DrawTarget> GetRefDrawTarget() override
-  {
-    RefPtr<gfxContext> ctx =
-      mFrame->PresContext()->PresShell()->CreateReferenceRenderingContext();
-    RefPtr<DrawTarget> dt = ctx->GetDrawTarget();
-    return dt.forget();
-  }
-private:
-  nsIFrame* mFrame;
-};
-
-static gfxTextRun*
-GetEllipsisTextRun(nsIFrame* aFrame)
+static already_AddRefed<gfxTextRun>
+MakeEllipsisTextRun(nsIFrame* aFrame)
 {
-  RefPtr<nsFontMetrics> fm =
-    nsLayoutUtils::GetInflatedFontMetricsForFrame(aFrame);
-  LazyReferenceRenderingDrawTargetGetterFromFrame lazyRefDrawTargetGetter(aFrame);
-  return fm->GetThebesFontGroup()->GetEllipsisTextRun(
+  RefPtr<nsFontMetrics> fm = nsLayoutUtils::GetInflatedFontMetricsForFrame(aFrame);
+  RefPtr<gfxContext> ctx = aFrame->PresContext()->PresShell()->CreateReferenceRenderingContext();
+  return fm->GetThebesFontGroup()->MakeEllipsisTextRun(
     aFrame->PresContext()->AppUnitsPerDevPixel(),
     nsLayoutUtils::GetTextRunOrientFlagsForStyle(aFrame->StyleContext()),
-    lazyRefDrawTargetGetter);
+    ctx->GetDrawTarget());
 }
 
 static nsIFrame*
@@ -166,16 +147,22 @@ public:
   nsDisplayTextOverflowMarker(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame,
                               const nsRect& aRect, nscoord aAscent,
                               const nsStyleTextOverflowSide* aStyle,
-                              uint32_t aIndex)
-    : nsDisplayItem(aBuilder, aFrame), mRect(aRect),
-      mStyle(aStyle), mAscent(aAscent), mIndex(aIndex) {
+                              uint32_t aIndex, gfxTextRun* aTextRun)
+    : nsDisplayItem(aBuilder, aFrame)
+    , mRect(aRect)
+    , mStyle(aStyle)
+    , mAscent(aAscent)
+    , mIndex(aIndex)
+    , mTextRun(aTextRun) {
     MOZ_COUNT_CTOR(nsDisplayTextOverflowMarker);
   }
+
 #ifdef NS_BUILD_REFCNT_LOGGING
   virtual ~nsDisplayTextOverflowMarker() {
     MOZ_COUNT_DTOR(nsDisplayTextOverflowMarker);
   }
 #endif
+
   virtual nsRect GetBounds(nsDisplayListBuilder* aBuilder,
                            bool* aSnap) override {
     *aSnap = false;
@@ -208,10 +195,11 @@ public:
                           nsPoint aOffsetFromRect);
   NS_DISPLAY_DECL_NAME("TextOverflow", TYPE_TEXT_OVERFLOW)
 private:
-  nsRect          mRect;   // in reference frame coordinates
+  nsRect          mRect; // in reference frame coordinates
   const nsStyleTextOverflowSide* mStyle;
   nscoord         mAscent; // baseline for the marker text in mRect
   uint32_t        mIndex;
+  RefPtr<gfxTextRun> mTextRun; // pre-cached textrun, if available
 };
 
 static void
@@ -259,21 +247,18 @@ nsDisplayTextOverflowMarker::PaintTextToContext(nsRenderingContext* aCtx,
   }
   pt += aOffsetFromRect;
 
-  if (mStyle->mType == NS_STYLE_TEXT_OVERFLOW_ELLIPSIS) {
-    gfxTextRun* textRun = GetEllipsisTextRun(mFrame);
-    if (textRun) {
-      NS_ASSERTION(!textRun->IsRightToLeft(),
-                   "Ellipsis textruns should always be LTR!");
-      gfxPoint gfxPt(pt.x, pt.y);
-      textRun->Draw(gfxTextRun::Range(textRun), gfxPt,
-                    gfxTextRun::DrawParams(aCtx->ThebesContext()));
-    }
-  } else {
-    RefPtr<nsFontMetrics> fm =
-      nsLayoutUtils::GetInflatedFontMetricsForFrame(mFrame);
-    nsLayoutUtils::DrawString(mFrame, *fm, aCtx, mStyle->mString.get(),
-                              mStyle->mString.Length(), pt);
+  if (mTextRun) {
+    gfxPoint gfxPt(pt.x, pt.y);
+    mTextRun->Draw(gfxTextRun::Range(mTextRun), gfxPt,
+                   gfxTextRun::DrawParams(aCtx->ThebesContext()));
+    return;
   }
+
+  MOZ_ASSERT(mStyle->mType != NS_STYLE_TEXT_OVERFLOW_ELLIPSIS);
+  RefPtr<nsFontMetrics> fm =
+      nsLayoutUtils::GetInflatedFontMetricsForFrame(mFrame);
+  nsLayoutUtils::DrawString(mFrame, *fm, aCtx, mStyle->mString.get(),
+                             mStyle->mString.Length(), pt);
 }
 
 TextOverflow::TextOverflow(nsDisplayListBuilder* aBuilder,
@@ -784,7 +769,10 @@ TextOverflow::CreateMarkers(const nsLineBox* aLine,
                markerRect, clipState);
     nsDisplayItem* marker = new (mBuilder)
       nsDisplayTextOverflowMarker(mBuilder, mBlock, markerRect,
-                                  aLine->GetLogicalAscent(), mIStart.mStyle, 0);
+                                  aLine->GetLogicalAscent(), mIStart.mStyle, 0,
+                                  mIStart.mStyle->mType == NS_STYLE_TEXT_OVERFLOW_ELLIPSIS ? 
+                                    GetEllipsisTextRun() :
+                                    nullptr);
     mMarkerList.AppendNewToTop(marker);
   }
 
@@ -801,11 +789,21 @@ TextOverflow::CreateMarkers(const nsLineBox* aLine,
                markerRect, clipState);
     nsDisplayItem* marker = new (mBuilder)
       nsDisplayTextOverflowMarker(mBuilder, mBlock, markerRect,
-                                  aLine->GetLogicalAscent(), mIEnd.mStyle, 1);
+                                  aLine->GetLogicalAscent(), mIEnd.mStyle, 1,
+                                  mIEnd.mStyle->mType == NS_STYLE_TEXT_OVERFLOW_ELLIPSIS ? 
+                                    GetEllipsisTextRun() :
+                                    nullptr);
     mMarkerList.AppendNewToTop(marker);
   }
 }
 
+gfxTextRun* TextOverflow::GetEllipsisTextRun() {
+  if (!mEllipsisTextRun) {
+    mEllipsisTextRun = MakeEllipsisTextRun(mBlock);
+  }
+  return mEllipsisTextRun;
+}
+
 void
 TextOverflow::Marker::SetupString(nsIFrame* aFrame)
 {
@@ -814,7 +812,7 @@ TextOverflow::Marker::SetupString(nsIFrame* aFrame)
   }
 
   if (mStyle->mType == NS_STYLE_TEXT_OVERFLOW_ELLIPSIS) {
-    gfxTextRun* textRun = GetEllipsisTextRun(aFrame);
+    RefPtr<gfxTextRun> textRun = MakeEllipsisTextRun(aFrame);
     if (textRun) {
       mISize = textRun->GetAdvanceWidth();
     } else {

layout/generic/TextOverflow.h

@@ -203,6 +203,8 @@ class TextOverflow {
                      bool aCreateIStart, bool aCreateIEnd,
                      const LogicalRect& aInsideMarkersArea);
 
+  gfxTextRun* GetEllipsisTextRun();
+  
   LogicalRect            mContentArea;
   nsDisplayListBuilder*  mBuilder;
   nsIFrame*              mBlock;
@@ -250,6 +252,8 @@ class TextOverflow {
 
   Marker mIStart; // the inline start marker
   Marker mIEnd; // the inline end marker
+  
+  RefPtr<gfxTextRun> mEllipsisTextRun; // Cached ellipsis textrun, if available
 };
 
 } // namespace css