Use local strike caches to avoid flaky test behavior

The remote glyph cache tests assume that the strike cache will not
change during a test. This is not true because other test also
mutate the strike cache. This is causing flaky tests.

 BUG=skia:8091

Change-Id: I397d411f9412006715f6860941dfb05ad54ae1b6
Reviewed-on: https://skia-review.googlesource.com/136741
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>

7 files changed, 38 insertions, 38 deletions

diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp
index fa4767069f..774a1eb77c 100644
--- a/src/core/SkRemoteGlyphCache.cpp
+++ b/src/core/SkRemoteGlyphCache.cpp
@@ -734,8 +734,11 @@ private:
     sk_sp<DiscardableHandleManager> fManager;
 };
 
-SkStrikeClient::SkStrikeClient(sk_sp<DiscardableHandleManager> discardableManager, bool isLogging)
+SkStrikeClient::SkStrikeClient(sk_sp<DiscardableHandleManager> discardableManager,
+                               bool isLogging,
+                               SkStrikeCache* strikeCache)
         : fDiscardableHandleManager(std::move(discardableManager))
+        , fStrikeCache{strikeCache ? strikeCache : SkStrikeCache::GlobalStrikeCache()}
         , fIsLogging{isLogging} {}
 
 SkStrikeClient::~SkStrikeClient() = default;
@@ -792,18 +795,19 @@ bool SkStrikeClient::readStrikeData(const volatile void* memory, size_t memorySi
         SkAutoDescriptor ad;
         auto* client_desc = auto_descriptor_from_desc(sourceAd.getDesc(), tf->uniqueID(), &ad);
 
-        auto strike = SkStrikeCache::FindStrikeExclusive(*client_desc);
+        auto strike = fStrikeCache->findStrikeExclusive(*client_desc);
         if (strike == nullptr) {
             // Note that we don't need to deserialize the effects since we won't be generating any
             // glyphs here anyway, and the desc is still correct since it includes the serialized
             // effects.
             SkScalerContextEffects effects;
             auto scaler = SkStrikeCache::CreateScalerContext(*client_desc, effects, *tf);
-            strike = SkStrikeCache::CreateStrikeExclusive(
+            strike = fStrikeCache->createStrikeExclusive(
                     *client_desc, std::move(scaler), &fontMetrics,
                     skstd::make_unique<DiscardableStrikePinner>(spec.discardableHandleId,
                                                                 fDiscardableHandleManager));
-            static_cast<SkScalerContextProxy*>(strike->getScalerContext())->initCache(strike.get());
+            auto proxyContext = static_cast<SkScalerContextProxy*>(strike->getScalerContext());
+            proxyContext->initCache(strike.get(), fStrikeCache);
         }
 
         size_t glyphImagesCount = 0u;
@@ -865,7 +869,7 @@ sk_sp<SkTypeface> SkStrikeClient::deserializeTypeface(const void* buf, size_t le
     WireTypeface wire;
     if (len != sizeof(wire)) return nullptr;
     memcpy(&wire, buf, sizeof(wire));
-    return addTypeface(wire);
+    return this->addTypeface(wire);
 }
 
 sk_sp<SkTypeface> SkStrikeClient::addTypeface(const WireTypeface& wire) {
diff --git a/src/core/SkRemoteGlyphCache.h b/src/core/SkRemoteGlyphCache.h
index ac13920772..157e058331 100644
--- a/src/core/SkRemoteGlyphCache.h
+++ b/src/core/SkRemoteGlyphCache.h
@@ -20,7 +20,6 @@
 #include "SkMakeUnique.h"
 #include "SkNoDrawCanvas.h"
 #include "SkRefCnt.h"
-#include "SkRemoteGlyphCache.h"
 #include "SkSerialProcs.h"
 #include "SkTypeface.h"
 
@@ -30,6 +29,7 @@ class SkGlyphCache;
 struct SkPackedGlyphID;
 enum SkScalerContextFlags : uint32_t;
 class SkScalerContextRecDescriptor;
+class SkStrikeCache;
 class SkTextBlobRunIterator;
 class SkTypefaceProxy;
 struct WireTypeface;
@@ -212,7 +212,9 @@ public:
         virtual void notifyCacheMiss(CacheMissType) {}
     };
 
-    SkStrikeClient(sk_sp<DiscardableHandleManager>, bool isLogging = true);
+    SkStrikeClient(sk_sp<DiscardableHandleManager>,
+                   bool isLogging = true,
+                   SkStrikeCache* strikeCache = nullptr);
     ~SkStrikeClient();
 
     // Deserializes the typeface previously serialized using the SkStrikeServer. Returns null if the
@@ -232,6 +234,7 @@ private:
 
     SkTHashMap<SkFontID, sk_sp<SkTypeface>> fRemoteFontIdToTypeface;
     sk_sp<DiscardableHandleManager> fDiscardableHandleManager;
+    SkStrikeCache* const fStrikeCache;
     const bool fIsLogging;
 };
 
diff --git a/src/core/SkStrikeCache.cpp b/src/core/SkStrikeCache.cpp
index bac48d8230..94f8c6d8bd 100644
--- a/src/core/SkStrikeCache.cpp
+++ b/src/core/SkStrikeCache.cpp
@@ -111,16 +111,6 @@ SkExclusiveStrikePtr SkStrikeCache::FindStrikeExclusive(const SkDescriptor& desc
     return GlobalStrikeCache()->findStrikeExclusive(desc);
 }
 
-bool SkStrikeCache::DesperationSearchForImage(const SkDescriptor& desc, SkGlyph* glyph,
-                                              SkGlyphCache* targetCache) {
-    return GlobalStrikeCache()->desperationSearchForImage(desc, glyph, targetCache);
-}
-
-bool SkStrikeCache::DesperationSearchForPath(
-        const SkDescriptor& desc, SkGlyphID glyphID, SkPath* path) {
-    return GlobalStrikeCache()->desperationSearchForPath(desc, glyphID, path);
-}
-
 std::unique_ptr<SkScalerContext> SkStrikeCache::CreateScalerContext(
         const SkDescriptor& desc,
         const SkScalerContextEffects& effects,
diff --git a/src/core/SkStrikeCache.h b/src/core/SkStrikeCache.h
index 22f1daf9fb..5d88126960 100644
--- a/src/core/SkStrikeCache.h
+++ b/src/core/SkStrikeCache.h
@@ -97,16 +97,9 @@ public:
 
     // Routines to find suitable data when working in a remote cache situation. These are
     // suitable as substitutes for similar calls in SkScalerContext.
-    static bool DesperationSearchForImage(const SkDescriptor& desc,
-                                          SkGlyph* glyph,
-                                          SkGlyphCache* targetCache);
-
     bool desperationSearchForImage(const SkDescriptor& desc,
                                    SkGlyph* glyph,
                                    SkGlyphCache* targetCache);
-
-    static bool DesperationSearchForPath(
-            const SkDescriptor& desc, SkGlyphID glyphID, SkPath* path);
     bool desperationSearchForPath(const SkDescriptor& desc, SkGlyphID glyphID, SkPath* path);
 
     static ExclusiveStrikePtr FindOrCreateStrikeExclusive(
diff --git a/src/core/SkTypeface_remote.cpp b/src/core/SkTypeface_remote.cpp
index 4b10f1e96f..bc6376d8fe 100644
--- a/src/core/SkTypeface_remote.cpp
+++ b/src/core/SkTypeface_remote.cpp
@@ -19,11 +19,12 @@ SkScalerContextProxy::SkScalerContextProxy(sk_sp<SkTypeface> tf,
         : SkScalerContext{std::move(tf), effects, desc}
         , fDiscardableManager{std::move(manager)} {}
 
-void SkScalerContextProxy::initCache(SkGlyphCache* cache) {
+void SkScalerContextProxy::initCache(SkGlyphCache* cache, SkStrikeCache* strikeCache) {
     SkASSERT(fCache == nullptr);
     SkASSERT(cache != nullptr);
 
     fCache = cache;
+    fStrikeCache = strikeCache;
 }
 
 unsigned SkScalerContextProxy::generateGlyphCount()  {
@@ -57,7 +58,7 @@ void SkScalerContextProxy::generateMetrics(SkGlyph* glyph) {
         }
 
         // Now check other caches for a desc mismatch.
-        if (SkStrikeCache::DesperationSearchForImage(fCache->getDescriptor(), glyph, fCache)) {
+        if (fStrikeCache->desperationSearchForImage(fCache->getDescriptor(), glyph, fCache)) {
             fDiscardableManager->notifyCacheMiss(
                     SkStrikeClient::CacheMissType::kGlyphMetricsFallback);
             return;
@@ -88,7 +89,7 @@ bool SkScalerContextProxy::generatePath(SkGlyphID glyphID, SkPath* path) {
     // Since the scaler context is being called, we don't have the needed data. Try to find a
     // fallback before failing.
     auto desc = SkScalerContext::DescriptorGivenRecAndEffects(this->getRec(), this->getEffects());
-    bool foundPath = SkStrikeCache::DesperationSearchForPath(*desc, glyphID, path);
+    bool foundPath = fStrikeCache->desperationSearchForPath(*desc, glyphID, path);
     fDiscardableManager->notifyCacheMiss(foundPath
                                                  ? SkStrikeClient::CacheMissType::kGlyphPathFallback
                                                  : SkStrikeClient::CacheMissType::kGlyphPath);
diff --git a/src/core/SkTypeface_remote.h b/src/core/SkTypeface_remote.h
index 88628d13b4..f4d3f73e2b 100644
--- a/src/core/SkTypeface_remote.h
+++ b/src/core/SkTypeface_remote.h
@@ -18,6 +18,7 @@
 #include "SkTypeface.h"
 
 class SkTypefaceProxy;
+class SkStrikeCache;
 
 class SkScalerContextProxy : public SkScalerContext {
 public:
@@ -26,7 +27,7 @@ public:
                          const SkDescriptor* desc,
                          sk_sp<SkStrikeClient::DiscardableHandleManager> manager);
 
-    void initCache(SkGlyphCache*);
+    void initCache(SkGlyphCache*, SkStrikeCache*);
 
 protected:
     unsigned generateGlyphCount() override;
@@ -41,6 +42,7 @@ protected:
 private:
     sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
     SkGlyphCache* fCache = nullptr;
+    SkStrikeCache* fStrikeCache = nullptr;
     typedef SkScalerContext INHERITED;
 };
 
diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp
index 828ca031c0..4a7bd86dc0 100644
--- a/tests/SkRemoteGlyphCacheTest.cpp
+++ b/tests/SkRemoteGlyphCacheTest.cpp
@@ -428,6 +428,8 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
     auto lostGlyphID = SkPackedGlyphID(1, SK_FixedHalf, SK_FixedHalf);
     const uint8_t glyphImage[] = {0xFF, 0xFF};
 
+    SkStrikeCache strikeCache;
+
     // Build a fallback cache.
     {
         SkAutoDescriptor ad;
@@ -437,7 +439,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
         SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
         auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
 
-        auto fallbackCache = SkStrikeCache::FindOrCreateStrikeExclusive(*desc, effects, *clientTf);
+        auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf);
         auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
         glyph->fMaskFormat = SkMask::kA8_Format;
         glyph->fHeight = 1;
@@ -454,7 +456,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
         SkScalerContextFlags flags = SkScalerContextFlags::kFakeGammaAndBoostContrast;
         SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
         auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
-        auto testCache = SkStrikeCache::FindStrikeExclusive(*desc);
+        auto testCache = strikeCache.findStrikeExclusive(*desc);
         REPORTER_ASSERT(reporter, !(testCache == nullptr));
     }
 
@@ -466,11 +468,12 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
     SkScalerContextFlags flags = SkScalerContextFlags::kNone;
     SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
     auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
-    testCache = SkStrikeCache::FindStrikeExclusive(*desc);
+    testCache = strikeCache.findStrikeExclusive(*desc);
     REPORTER_ASSERT(reporter, testCache == nullptr);
-    testCache = SkStrikeCache::CreateStrikeExclusive(*desc,
+    testCache = strikeCache.createStrikeExclusive(*desc,
                                                      clientTf->createScalerContext(effects, desc));
-    static_cast<SkScalerContextProxy*>(testCache->getScalerContext())->initCache(testCache.get());
+    auto scalerProxy = static_cast<SkScalerContextProxy*>(testCache->getScalerContext());
+    scalerProxy->initCache(testCache.get(), &strikeCache);
 
     // Look for the lost glyph.
     {
@@ -504,7 +507,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
             REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(i) == 0);
         }
     }
-    SkStrikeCache::ValidateGlyphCacheDataSize();
+    strikeCache.validateGlyphCacheDataSize();
 
     // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
     discardableManager->unlockAndDeleteAll();
@@ -530,6 +533,8 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
     uint32_t realMask;
     uint32_t fakeMask;
 
+    SkStrikeCache strikeCache;
+
     {
         SkAutoDescriptor ad;
         SkScalerContextRec rec;
@@ -557,7 +562,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
         SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
         auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
 
-        auto fallbackCache = SkStrikeCache::FindOrCreateStrikeExclusive(*desc, effects, *clientTf);
+        auto fallbackCache = strikeCache.findOrCreateStrikeExclusive(*desc, effects, *clientTf);
         auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
         fakeMask = (realMask == SkMask::kA8_Format) ? SkMask::kBW_Format : SkMask::kA8_Format;
         glyph->fMaskFormat = fakeMask;
@@ -579,7 +584,9 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
         std::vector<uint8_t> serverStrikeData;
         server.writeStrikeData(&serverStrikeData);
         REPORTER_ASSERT(reporter,
-                        client.readStrikeData(serverStrikeData.data(), serverStrikeData.size()));
+                        client.readStrikeData(
+                                serverStrikeData.data(),
+                                serverStrikeData.size()));
     }
 
     {
@@ -591,7 +598,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
         SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
         auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
 
-        auto fallbackCache = SkStrikeCache::FindStrikeExclusive(*desc);
+        auto fallbackCache = strikeCache.findStrikeExclusive(*desc);
         REPORTER_ASSERT(reporter, fallbackCache.get() != nullptr);
         auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
         REPORTER_ASSERT(reporter, glyph->fMaskFormat == fakeMask);
@@ -605,7 +612,7 @@ DEF_TEST(SkRemoteGlyphCache_ReWriteGlyph, reporter) {
                         memcmp(glyph->fImage, glyphImage, glyph->computeImageSize()) == 0);
     }
 
-    SkStrikeCache::ValidateGlyphCacheDataSize();
+    strikeCache.validateGlyphCacheDataSize();
 
     // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
     discardableManager->unlockAndDeleteAll();