Bug 1265648 - Remove the global nsTextFrameTextRunCache, as it no longer serves any useful purpose. r=mats
authorJonathan Kew <jkew@mozilla.com>
Wed, 20 Apr 2016 10:54:43 +0100
changeset 331891 77ba0dcb977a5accadbdefe0563f2ee9940ce6a8
parent 331890 c10de64dc242b252b57bb0244e464e599c6dbb27
child 331892 20956a16ffb857176bf34571d277b81685d0200c
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1265648
milestone48.0a1
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
Bug 1265648 - Remove the global nsTextFrameTextRunCache, as it no longer serves any useful purpose. r=mats
gfx/tests/gtest/gfxWordCacheTest.cpp
gfx/tests/gtest/moz.build
gfx/thebes/gfxFont.cpp
gfx/thebes/gfxFontEntry.cpp
gfx/thebes/gfxTextRun.h
layout/build/nsLayoutStatics.cpp
layout/generic/nsTextFrame.cpp
layout/generic/nsTextFrame.h
deleted file mode 100644
--- a/gfx/tests/gtest/gfxWordCacheTest.cpp
+++ /dev/null
@@ -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;
-}
--- a/gfx/tests/gtest/moz.build
+++ b/gfx/tests/gtest/moz.build
@@ -1,18 +1,16 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # 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/.
 
 UNIFIED_SOURCES += [
     'gfxSurfaceRefCountTest.cpp',
-    # Disabled on suspicion of causing bug 904227
-    #'gfxWordCacheTest.cpp',
     'TestArena.cpp',
     'TestArrayView.cpp',
     'TestBufferRotation.cpp',
     'TestColorNames.cpp',
     'TestCompositor.cpp',
     'TestGfxPrefs.cpp',
     'TestGfxWidgets.cpp',
     'TestJobScheduler.cpp',
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -7,17 +7,16 @@
 
 #include "mozilla/BinarySearch.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/MathAlgorithms.h"
 
 #include "mozilla/Logging.h"
 
-#include "nsExpirationTracker.h"
 #include "nsITimer.h"
 
 #include "gfxGlyphExtents.h"
 #include "gfxPlatform.h"
 #include "gfxTextRun.h"
 #include "nsGkAtoms.h"
 
 #include "gfxTypes.h"
--- a/gfx/thebes/gfxFontEntry.cpp
+++ b/gfx/thebes/gfxFontEntry.cpp
@@ -4,19 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/DebugOnly.h"
 #include "mozilla/MathAlgorithms.h"
 
 #include "mozilla/Logging.h"
 
 #include "nsServiceManagerUtils.h"
-#include "nsExpirationTracker.h"
 #include "nsILanguageAtomService.h"
-#include "nsITimer.h"
 
 #include "gfxFontEntry.h"
 #include "gfxTextRun.h"
 #include "gfxPlatform.h"
 #include "nsGkAtoms.h"
 
 #include "gfxTypes.h"
 #include "gfxContext.h"
--- a/gfx/thebes/gfxTextRun.h
+++ b/gfx/thebes/gfxTextRun.h
@@ -571,18 +571,16 @@ public:
 
     // 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
     // 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
     // permanently alive due to a circular reference. (The caller of this is
     // taking responsibility for ensuring the textrun will not outlive its
     // mFontGroup.)
     void ReleaseFontGroup();
 
@@ -742,17 +740,16 @@ private:
     // XXX this should be changed to a GlyphRun plus a maybe-null GlyphRun*,
     // for smaller size especially in the super-common one-glyphrun case
     AutoTArray<GlyphRun,1>        mGlyphRuns;
 
     void             *mUserData;
     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
                                     // 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
--- a/layout/build/nsLayoutStatics.cpp
+++ b/layout/build/nsLayoutStatics.cpp
@@ -196,18 +196,16 @@ nsLayoutStatics::Initialize()
     return rv;
   }
 
   nsCellMap::Init();
 
   StaticPresData::Init();
   nsCSSRendering::Init();
 
-  nsTextFrameTextRunCache::Init();
-
   rv = nsHTMLDNSPrefetch::Initialize();
   if (NS_FAILED(rv)) {
     NS_ERROR("Could not initialize HTML DNS prefetch");
     return rv;
   }
 
 #ifdef MOZ_XUL
   rv = nsXULContentUtils::Init();
@@ -341,17 +339,16 @@ nsLayoutStatics::Shutdown()
 #endif
   DOMStorageObserver::Shutdown();
   txMozillaXSLTProcessor::Shutdown();
   Attr::Shutdown();
   EventListenerManager::Shutdown();
   IMEStateManager::Shutdown();
   nsCSSParser::Shutdown();
   nsCSSRuleProcessor::Shutdown();
-  nsTextFrameTextRunCache::Shutdown();
   nsHTMLDNSPrefetch::Shutdown();
   nsCSSRendering::Shutdown();
   StaticPresData::Shutdown();
 #ifdef DEBUG
   nsFrame::DisplayReflowShutdown();
 #endif
   nsCellMap::Shutdown();
   ActiveLayerTracker::Shutdown();
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -44,17 +44,16 @@
 #include "nsLayoutUtils.h"
 #include "nsDisplayList.h"
 #include "nsFrame.h"
 #include "nsIMathMLFrame.h"
 #include "nsPlaceholderFrame.h"
 #include "nsTextFrameUtils.h"
 #include "nsTextRunTransformations.h"
 #include "MathMLTextRunFactory.h"
-#include "nsExpirationTracker.h"
 #include "nsUnicodeProperties.h"
 #include "nsStyleUtil.h"
 #include "nsRubyFrame.h"
 
 #include "nsTextFragment.h"
 #include "nsGkAtoms.h"
 #include "nsFrameSelection.h"
 #include "nsRange.h"
@@ -180,17 +179,16 @@ TabWidthStore::ApplySpacing(gfxTextRun::
     i++;
   }
 }
 
 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)
 
 class GlyphObserver : public gfxFont::GlyphChangeObserver {
 public:
   GlyphObserver(gfxFont* aFont, nsTextFrame* aFrame)
     : gfxFont::GlyphChangeObserver(aFont), mFrame(aFrame) {}
@@ -550,83 +548,16 @@ GlyphObserver::NotifyGlyphsChanged()
     // OverflowChangedTracker, but that would do a bunch of work eagerly that
     // we should probably do lazily here since there could be a lot
     // of text frames affected and we'd like to coalesce the work. So that's
     // not easy to do well.
     shell->FrameNeedsReflow(f, nsIPresShell::eResize, NS_FRAME_IS_DIRTY);
   }
 }
 
-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();
 }
 
 struct FlowLengthProperty {
   int32_t mStartOffset;
   // The offset of the next fixed continuation after mStartOffset, or
@@ -1540,17 +1471,16 @@ void BuildTextRunsScanner::FlushLineBrea
     // TODO cause frames associated with the textrun to be reflowed, if they
     // aren't being reflowed already!
     mBreakSinks[i]->Finish(mMissingFonts);
   }
   mBreakSinks.Clear();
 
   for (uint32_t i = 0; i < mTextRunsToDelete.Length(); ++i) {
     gfxTextRun* deleteTextRun = mTextRunsToDelete[i];
-    gTextRuns->RemoveFromCache(deleteTextRun);
     delete deleteTextRun;
   }
   mTextRunsToDelete.Clear();
 }
 
 void BuildTextRunsScanner::AccumulateRunInfo(nsTextFrame* aFrame)
 {
   if (mMaxTextLength != UINT32_MAX) {
@@ -2236,33 +2166,33 @@ BuildTextRunsScanner::BuildTextRunForFra
       textRun = transformingFactory->MakeTextRun(text, transformedLength,
                                                  &params, fontGroup, textFlags,
                                                  Move(styles), true);
       if (textRun) {
         // ownership of the factory has passed to the textrun
         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);
     textFlags |= gfxFontGroup::TEXT_IS_8BIT;
     if (transformingFactory) {
       textRun = transformingFactory->MakeTextRun(text, transformedLength,
                                                  &params, fontGroup, textFlags,
                                                  Move(styles), true);
       if (textRun) {
         // ownership of the factory has passed to the textrun
         transformingFactory.forget();
       }
     } else {
-      textRun = MakeTextRun(text, transformedLength, fontGroup, &params,
-                            textFlags, mMissingFonts);
+      textRun = fontGroup->MakeTextRun(text, transformedLength, &params,
+                                       textFlags, mMissingFonts);
     }
   }
   if (!textRun) {
     DestroyUserData(userDataToDestroy);
     return nullptr;
   }
 
   // We have to set these up after we've created the textrun, because
@@ -2703,21 +2633,17 @@ NS_QUERYFRAME_TAIL_INHERITING(nsFrame)
 gfxSkipCharsIterator
 nsTextFrame::EnsureTextRun(TextRunType aWhichTextRun,
                            DrawTarget* aRefDrawTarget,
                            nsIFrame* aLineContainer,
                            const nsLineList::iterator* aLine,
                            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);
     }
     if (refDT) {
       BuildTextRuns(refDT, this, aLineContainer, aLine, aWhichTextRun);
     }
     textRun = GetTextRun(aWhichTextRun);
@@ -4543,31 +4469,18 @@ nsTextFrame::ClearTextRun(nsTextFrame* a
     return;
   }
 
   DebugOnly<bool> checkmTextrun = textRun == mTextRun;
   UnhookTextRunFromFrames(textRun, 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;
   }
 }
 
 void
 nsTextFrame::DisconnectTextRuns()
 {
   MOZ_ASSERT(!IsInTextRunUserData(),
--- a/layout/generic/nsTextFrame.h
+++ b/layout/generic/nsTextFrame.h
@@ -26,22 +26,16 @@
 class nsTextPaintStyle;
 class PropertyProvider;
 struct SelectionDetails;
 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;
   typedef mozilla::gfx::DrawTarget DrawTarget;
   typedef mozilla::gfx::Point Point;
   typedef mozilla::gfx::Rect Rect;
   typedef mozilla::gfx::Size Size;
   typedef gfxTextRun::Range Range;