fonts: Cap the max number of entries in server side glyph cache tracking

The SkStrikeServer maintains a map of SkGlyphCacheState to track the
cache for a strike on the client. This entry is evicted only on a
failure to lock, which means that the map can potentially grow unbounded
if entries are not reused after eviction.

Make sure we check that they are deleted after some limit.

R=herb@google.com

Bug:878966
Change-Id: I4adb06c35661049328f6e0bde52fb1c774d0c29b
Reviewed-on: https://skia-review.googlesource.com/150443
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Khusal Sagar <khushalsagar@chromium.org>

src/core/SkRemoteGlyphCache.cpp

@@ -515,9 +515,23 @@ SkStrikeServer::SkGlyphCacheState* SkStrikeServer::getOrCreateCache(
     fLockedDescs.insert(keyDescPtr);
     fRemoteGlyphStateMap[keyDescPtr] = std::move(cacheState);
     cacheStatePtr->ensureScalerContext(paint, props, matrix, flags, effects);
+
+    checkForDeletedEntries();
     return cacheStatePtr;
 }
 
+void SkStrikeServer::checkForDeletedEntries() {
+    auto it = fRemoteGlyphStateMap.begin();
+    while (fRemoteGlyphStateMap.size() > fMaxEntriesInDescriptorMap &&
+           it != fRemoteGlyphStateMap.end()) {
+        if (fDiscardableHandleManager->isHandleDeleted(it->second->discardableHandleId())) {
+            it = fRemoteGlyphStateMap.erase(it);
+        } else {
+            ++it;
+        }
+    }
+}
+
 // -- SkGlyphCacheState ----------------------------------------------------------------------------
 SkStrikeServer::SkGlyphCacheState::SkGlyphCacheState(
         std::unique_ptr<SkDescriptor> keyDescriptor, uint32_t discardableHandleId)

src/core/SkRemoteGlyphCache.h

@@ -105,9 +105,10 @@ public:
         // allowed.
         virtual bool lockHandle(SkDiscardableHandleId) = 0;
 
-        // TODO(khushalsagar): Add an API which checks whether a handle is still
+        // Returns true if a handle has been deleted on the remote client. It is
-        // valid without locking, so we can avoid tracking stale handles once they
+        // invalid to use a handle id again with this manager once this returns true.
-        // have been purged on the remote side.
+        // TODO(khushalsagar): Make pure virtual once chrome implementation lands.
+        virtual bool isHandleDeleted(SkDiscardableHandleId) { return false; }
     };
 
     SkStrikeServer(DiscardableHandleManager* discardableHandleManager);
@@ -128,10 +129,20 @@ public:
                                         SkScalerContextFlags flags,
                                         SkScalerContextEffects* effects);
 
+    void setMaxEntriesInDescriptorMapForTesting(size_t count) {
+        fMaxEntriesInDescriptorMap = count;
+    }
+    size_t remoteGlyphStateMapSizeForTesting() const { return fRemoteGlyphStateMap.size(); }
+
 private:
+    static constexpr size_t kMaxEntriesInDescriptorMap = 2000u;
+
+    void checkForDeletedEntries();
+
     SkDescriptorMap<std::unique_ptr<SkGlyphCacheState>> fRemoteGlyphStateMap;
     DiscardableHandleManager* const fDiscardableHandleManager;
     SkTHashSet<SkFontID> fCachedTypefaces;
+    size_t fMaxEntriesInDescriptorMap = kMaxEntriesInDescriptorMap;
 
     // State cached until the next serialization.
     SkDescriptorSet fLockedDescs;

tests/SkRemoteGlyphCacheTest.cpp

@@ -41,6 +41,7 @@ public:
     // Client implementation.
     bool deleteHandle(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; }
     void notifyCacheMiss(SkStrikeClient::CacheMissType type) override { fCacheMissCount[type]++; }
+    bool isHandleDeleted(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; }
 
     void unlockAll() { fLockedHandles.reset(); }
     void unlockAndDeleteAll() {
@@ -342,6 +343,50 @@ DEF_TEST(SkRemoteGlyphCache_ClientMemoryAccounting, reporter) {
     discardableManager->unlockAndDeleteAll();
 }
 
+DEF_TEST(SkRemoteGlyphCache_PurgesServerEntries, reporter) {
+    sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
+    SkStrikeServer server(discardableManager.get());
+    server.setMaxEntriesInDescriptorMapForTesting(1u);
+    SkStrikeClient client(discardableManager, false);
+
+    {
+        auto serverTf = SkTypeface::MakeFromName("monospace", SkFontStyle());
+        int glyphCount = 10;
+        auto serverBlob = buildTextBlob(serverTf, glyphCount);
+
+        const SkSurfaceProps props(SkSurfaceProps::kLegacyFontHost_InitType);
+        SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, SkMatrix::I(), props, &server);
+        SkPaint paint;
+        REPORTER_ASSERT(reporter, server.remoteGlyphStateMapSizeForTesting() == 0u);
+        cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint);
+        REPORTER_ASSERT(reporter, server.remoteGlyphStateMapSizeForTesting() == 1u);
+    }
+
+    // Serialize to release the lock from the strike server and delete all current
+    // handles.
+    std::vector<uint8_t> fontData;
+    server.writeStrikeData(&fontData);
+    discardableManager->unlockAndDeleteAll();
+
+    // Use a different typeface. Creating a new strike should evict the previous
+    // one.
+    {
+        auto serverTf = SkTypeface::MakeFromName("Georgia", SkFontStyle());
+        int glyphCount = 10;
+        auto serverBlob = buildTextBlob(serverTf, glyphCount);
+
+        const SkSurfaceProps props(SkSurfaceProps::kLegacyFontHost_InitType);
+        SkTextBlobCacheDiffCanvas cache_diff_canvas(10, 10, SkMatrix::I(), props, &server);
+        SkPaint paint;
+        REPORTER_ASSERT(reporter, server.remoteGlyphStateMapSizeForTesting() == 1u);
+        cache_diff_canvas.drawTextBlob(serverBlob.get(), 0, 0, paint);
+        REPORTER_ASSERT(reporter, server.remoteGlyphStateMapSizeForTesting() == 1u);
+    }
+
+    // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
+    discardableManager->unlockAndDeleteAll();
+}
+
 DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsPath, reporter, ctxInfo) {
     sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
     SkStrikeServer server(discardableManager.get());