Bug 1265648 - Remove the global nsTextFrameTextRunCache, as it no longer serves any useful purpose. r=mats

gfx/tests/gtest/gfxWordCacheTest.cpp

@@ -1,141 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "gtest/gtest.h"
-
-#include "mozilla/gfx/2D.h"
-#include "mozilla/RefPtr.h"
-#include "nsCOMPtr.h"
-#include "nsTArray.h"
-#include "nsString.h"
-#include "nsDependentString.h"
-
-#include "prinrval.h"
-
-#include "gfxContext.h"
-#include "gfxFont.h"
-#include "gfxPlatform.h"
-
-#include "gfxFontTest.h"
-#include "mozilla/Attributes.h"
-
-using namespace mozilla;
-using namespace mozilla::gfx;
-
-class FrameTextRunCache;
-
-static FrameTextRunCache *gTextRuns = nullptr;
-
-/*
- * Cache textruns and expire them after 3*10 seconds of no use.
- */
-class FrameTextRunCache final : public nsExpirationTracker<gfxTextRun,3> {
-public:
-  enum { TIMEOUT_SECONDS = 10 };
-  FrameTextRunCache()
-    : nsExpirationTracker<gfxTextRun,3>(TIMEOUT_SECONDS * 1000,
-                                        "FrameTextRunCache")
-  {}
-  ~FrameTextRunCache() {
-    AgeAllGenerations();
-  }
-
-  void RemoveFromCache(gfxTextRun* aTextRun) {
-    if (aTextRun->GetExpirationState()->IsTracked()) {
-      RemoveObject(aTextRun);
-    }
-  }
-
-  // This gets called when the timeout has expired on a gfxTextRun
-  virtual void NotifyExpired(gfxTextRun* aTextRun) {
-    RemoveFromCache(aTextRun);
-    delete aTextRun;
-  }
-};
-
-static gfxTextRun *
-MakeTextRun(const char16_t *aText, uint32_t aLength, gfxFontGroup *aFontGroup,
-            const gfxFontGroup::Parameters* aParams, uint32_t aFlags)
-{
-  UniquePtr<gfxTextRun> textRun;
-  if (aLength == 0) {
-    abort();
-    //textRun = aFontGroup->MakeEmptyTextRun(aParams, aFlags);
-  } else if (aLength == 1 && aText[0] == ' ') {
-    abort();
-    //textRun = aFontGroup->MakeSpaceTextRun(aParams, aFlags);
-  } else {
-    textRun = aFontGroup->MakeTextRun(aText, aLength, aParams, aFlags);
-  }
-  if (!textRun) {
-    return nullptr;
-  }
-  nsresult rv = gTextRuns->AddObject(textRun.get());
-  if (NS_FAILED(rv)) {
-    gTextRuns->RemoveFromCache(textRun.get());
-    return nullptr;
-  }
-  return textRun.release();
-}
-
-static already_AddRefed<DrawTarget>
-MakeDrawTarget()
-{
-  const int size = 200;
-
-  RefPtr<DrawTarget> drawTarget = gfxPlatform::GetPlatform()->
-      CreateOffscreenContentDrawTarget(IntSize(size, size),
-                                       SurfaceFormat::B8G8R8X8);
-  return drawTarget.forget();
-}
-
-TEST(Gfx, WordCache) {
-  gTextRuns = new FrameTextRunCache();
-
-  RefPtr<DrawTarget> dt = MakeDrawTarget();
-  {
-    gfxFontStyle style(mozilla::gfx::FontStyle::NORMAL,
-                       139,
-                       10.0,
-                       0,
-                       NS_Atomize(NS_LITERAL_STRING("en")),
-                       0.0,
-                       false, false,
-                       NS_LITERAL_STRING(""));
-
-    RefPtr<gfxFontGroup> fontGroup =
-      gfxPlatform::GetPlatform()->CreateFontGroup(
-        NS_LITERAL_STRING("Geneva, MS Sans Serif, Helvetica,serif"), &style,
-        nullptr, nullptr, 1.0);
-
-    gfxTextRunFactory::Parameters params = {
-      dt, nullptr, nullptr, nullptr, 0, 60
-    };
-
-    uint32_t flags = gfxTextRunFactory::TEXT_IS_PERSISTENT;
-
-    // First load an Arabic word into the cache
-    const char cString[] = "\xd8\xaa\xd9\x85";
-    nsDependentCString cStr(cString);
-    NS_ConvertUTF8toUTF16 str(cStr);
-    gfxTextRun *tr =
-      MakeTextRun(str.get(), str.Length(), fontGroup, &params, flags);
-    tr->GetAdvanceWidth(0, str.Length(), nullptr);
-
-    // Now try to trigger an assertion with a word cache bug. The first
-    // word is in the cache so it gets added to the new textrun directly.
-    // The second word is not in the cache.
-    const char cString2[] = "\xd8\xaa\xd9\x85\n\xd8\xaa\xd8\x85 ";
-    nsDependentCString cStr2(cString2);
-    NS_ConvertUTF8toUTF16 str2(cStr2);
-    gfxTextRun *tr2 =
-      MakeTextRun(str2.get(), str2.Length(), fontGroup, &params, flags);
-    tr2->GetAdvanceWidth(0, str2.Length(), nullptr);
-  }
-
-  delete gTextRuns;
-  gTextRuns = nullptr;
-}

gfx/tests/gtest/moz.build

@@ -6,8 +6,6 @@
 
 UNIFIED_SOURCES += [
     'gfxSurfaceRefCountTest.cpp',
-    # Disabled on suspicion of causing bug 904227
-    #'gfxWordCacheTest.cpp',
     'TestArena.cpp',
     'TestArrayView.cpp',
     'TestBufferRotation.cpp',

gfx/thebes/gfxFont.cpp

@@ -12,7 +12,6 @@
 
 #include "mozilla/Logging.h"
 
-#include "nsExpirationTracker.h"
 #include "nsITimer.h"
 
 #include "gfxGlyphExtents.h"

gfx/thebes/gfxFontEntry.cpp

@@ -9,9 +9,7 @@
 #include "mozilla/Logging.h"
 
 #include "nsServiceManagerUtils.h"
-#include "nsExpirationTracker.h"
 #include "nsILanguageAtomService.h"
-#include "nsITimer.h"
 
 #include "gfxFontEntry.h"
 #include "gfxTextRun.h"

gfx/thebes/gfxTextRun.h

@@ -576,8 +576,6 @@ public:
     // textrun.
     void CopyGlyphDataFrom(gfxTextRun *aSource, Range aRange, uint32_t aDest);
 
-    nsExpirationState *GetExpirationState() { return &mExpirationState; }
-
     // Tell the textrun to release its reference to its creating gfxFontGroup
     // immediately, rather than on destruction. This is used for textruns
     // that are actually owned by a gfxFontGroup, so that they don't keep it
@@ -747,7 +745,6 @@ private:
     gfxFontGroup     *mFontGroup; // addrefed on creation, but our reference
                                   // may be released by ReleaseFontGroup()
     gfxSkipChars      mSkipChars;
-    nsExpirationState mExpirationState;
 
     bool              mSkipDrawing; // true if the font group we used had a user font
                                     // download that's in progress, so we should hide text

layout/build/nsLayoutStatics.cpp

@@ -201,8 +201,6 @@ nsLayoutStatics::Initialize()
   StaticPresData::Init();
   nsCSSRendering::Init();
 
-  nsTextFrameTextRunCache::Init();
-
   rv = nsHTMLDNSPrefetch::Initialize();
   if (NS_FAILED(rv)) {
     NS_ERROR("Could not initialize HTML DNS prefetch");
@@ -346,7 +344,6 @@ nsLayoutStatics::Shutdown()
   IMEStateManager::Shutdown();
   nsCSSParser::Shutdown();
   nsCSSRuleProcessor::Shutdown();
-  nsTextFrameTextRunCache::Shutdown();
   nsHTMLDNSPrefetch::Shutdown();
   nsCSSRendering::Shutdown();
   StaticPresData::Shutdown();

layout/generic/nsTextFrame.cpp

@@ -49,7 +49,6 @@
 #include "nsTextFrameUtils.h"
 #include "nsTextRunTransformations.h"
 #include "MathMLTextRunFactory.h"
-#include "nsExpirationTracker.h"
 #include "nsUnicodeProperties.h"
 #include "nsStyleUtil.h"
 #include "nsRubyFrame.h"
@@ -185,7 +184,6 @@ NS_DECLARE_FRAME_PROPERTY_DELETABLE(TabWidthProperty, TabWidthStore)
 
 NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(OffsetToFrameProperty, nsTextFrame)
 
-// text runs are destroyed by the text run cache
 NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(UninflatedTextRunProperty, gfxTextRun)
 
 NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(FontSizeInflationProperty, float)
@@ -555,73 +553,6 @@ GlyphObserver::NotifyGlyphsChanged()
   }
 }
 
-class FrameTextRunCache;
-
-static FrameTextRunCache *gTextRuns = nullptr;
-
-/*
- * Cache textruns and expire them after 3*10 seconds of no use.
- */
-class FrameTextRunCache final : public nsExpirationTracker<gfxTextRun,3> {
-public:
-  enum { TIMEOUT_SECONDS = 10 };
-  FrameTextRunCache()
-    : nsExpirationTracker<gfxTextRun,3>(TIMEOUT_SECONDS * 1000,
-                                        "FrameTextRunCache")
-  {}
-  ~FrameTextRunCache() {
-    AgeAllGenerations();
-  }
-
-  void RemoveFromCache(gfxTextRun* aTextRun) {
-    if (aTextRun->GetExpirationState()->IsTracked()) {
-      RemoveObject(aTextRun);
-    }
-  }
-
-  // This gets called when the timeout has expired on a gfxTextRun
-  virtual void NotifyExpired(gfxTextRun* aTextRun) {
-    UnhookTextRunFromFrames(aTextRun, nullptr);
-    RemoveFromCache(aTextRun);
-    delete aTextRun;
-  }
-};
-
-// Helper to create a textrun and remember it in the textframe cache,
-// for either 8-bit or 16-bit text strings
-template<typename T>
-UniquePtr<gfxTextRun>
-MakeTextRun(const T *aText, uint32_t aLength,
-            gfxFontGroup *aFontGroup, const gfxFontGroup::Parameters* aParams,
-            uint32_t aFlags, gfxMissingFontRecorder *aMFR)
-{
-    UniquePtr<gfxTextRun> textRun =
-        aFontGroup->MakeTextRun(aText, aLength, aParams, aFlags, aMFR);
-    if (!textRun) {
-        return nullptr;
-    }
-    nsresult rv = gTextRuns->AddObject(textRun.get());
-    if (NS_FAILED(rv)) {
-        gTextRuns->RemoveFromCache(textRun.get());
-        return nullptr;
-    }
-#ifdef NOISY_BIDI
-    printf("Created textrun\n");
-#endif
-    return textRun;
-}
-
-void
-nsTextFrameTextRunCache::Init() {
-    gTextRuns = new FrameTextRunCache();
-}
-
-void
-nsTextFrameTextRunCache::Shutdown() {
-    delete gTextRuns;
-    gTextRuns = nullptr;
-}
-
 int32_t nsTextFrame::GetContentEnd() const {
   nsTextFrame* next = static_cast<nsTextFrame*>(GetNextContinuation());
   return next ? next->GetContentOffset() : mContent->GetText()->GetLength();
@@ -1545,7 +1476,6 @@ void BuildTextRunsScanner::FlushLineBreaks(gfxTextRun* aTrailingTextRun)
 
   for (uint32_t i = 0; i < mTextRunsToDelete.Length(); ++i) {
     gfxTextRun* deleteTextRun = mTextRunsToDelete[i];
-    gTextRuns->RemoveFromCache(deleteTextRun);
     delete deleteTextRun;
   }
   mTextRunsToDelete.Clear();
@@ -2241,8 +2171,8 @@ BuildTextRunsScanner::BuildTextRunForFrames(void* aTextBuffer)
         transformingFactory.forget();
       }
     } else {
-      textRun = MakeTextRun(text, transformedLength, fontGroup, &params,
-                            textFlags, mMissingFonts);
+      textRun = fontGroup->MakeTextRun(text, transformedLength, &params,
+                                       textFlags, mMissingFonts);
     }
   } else {
     const uint8_t* text = static_cast<const uint8_t*>(textPtr);
@@ -2256,8 +2186,8 @@ BuildTextRunsScanner::BuildTextRunForFrames(void* aTextBuffer)
         transformingFactory.forget();
       }
     } else {
-      textRun = MakeTextRun(text, transformedLength, fontGroup, &params,
-                            textFlags, mMissingFonts);
+      textRun = fontGroup->MakeTextRun(text, transformedLength, &params,
+                                       textFlags, mMissingFonts);
     }
   }
   if (!textRun) {
@@ -2708,11 +2638,7 @@ nsTextFrame::EnsureTextRun(TextRunType aWhichTextRun,
                            uint32_t* aFlowEndInTextRun)
 {
   gfxTextRun *textRun = GetTextRun(aWhichTextRun);
-  if (textRun && (!aLine || !(*aLine)->GetInvalidateTextRuns())) {
-    if (textRun->GetExpirationState()->IsTracked()) {
-      gTextRuns->MarkUsed(textRun);
-    }
-  } else {
+  if (!textRun || (aLine && (*aLine)->GetInvalidateTextRuns())) {
     RefPtr<DrawTarget> refDT = aRefDrawTarget;
     if (!refDT) {
       refDT = CreateReferenceDrawTarget(this);
@@ -4548,21 +4474,8 @@ nsTextFrame::ClearTextRun(nsTextFrame* aStartContinuation,
   MOZ_ASSERT(checkmTextrun ? !mTextRun
                            : !Properties().Get(UninflatedTextRunProperty()));
 
-  // see comments in BuildTextRunForFrames...
-//  if (textRun->GetFlags() & gfxFontGroup::TEXT_IS_PERSISTENT) {
-//    NS_ERROR("Shouldn't reach here for now...");
-//    // the textrun's text may be referencing a DOM node that has changed,
-//    // so we'd better kill this textrun now.
-//    if (textRun->GetExpirationState()->IsTracked()) {
-//      gTextRuns->RemoveFromCache(textRun);
-//    }
-//    delete textRun;
-//    return;
-//  }
-
   if (!textRun->GetUserData()) {
-    // Remove it now because it's not doing anything useful
-    gTextRuns->RemoveFromCache(textRun);
+    // Delete it now because it's not doing anything useful
     delete textRun;
   }
 }

layout/generic/nsTextFrame.h

@@ -31,12 +31,6 @@ class nsTextFragment;
 class nsDisplayTextGeometry;
 class nsDisplayText;
 
-class nsTextFrameTextRunCache {
-public:
-  static void Init();
-  static void Shutdown();
-};
-
 class nsTextFrame : public nsFrame {
   typedef mozilla::LayoutDeviceRect LayoutDeviceRect;
   typedef mozilla::TextRangeStyle TextRangeStyle;