Reland fonts: Cleanup cache miss logging for font remoting.

This reverts commit fc6cf92e4b21c92ead769fae557534056eac6d83.

TBR=herb@google.com
Bug: skia:7913
Change-Id: I93301e49839fdb895a5f1f5845bb9975981c69fc
Reviewed-on: https://skia-review.googlesource.com/129880
Reviewed-by: Khusal Sagar <khushalsagar@chromium.org>
Commit-Queue: Khusal Sagar <khushalsagar@chromium.org>

5 files changed, 82 insertions, 82 deletions

diff --git a/src/core/SkRemoteGlyphCache.cpp b/src/core/SkRemoteGlyphCache.cpp
index d5e96fce88..216a2a9dab 100644
--- a/src/core/SkRemoteGlyphCache.cpp
+++ b/src/core/SkRemoteGlyphCache.cpp
@@ -801,40 +801,7 @@ sk_sp<SkTypeface> SkStrikeClient::addTypeface(const WireTypeface& wire) {
     if (typeface) return *typeface;
 
     auto newTypeface = sk_make_sp<SkTypefaceProxy>(wire.typefaceID, wire.glyphCount, wire.style,
-                                                   wire.isFixed, this);
+                                                   wire.isFixed, fDiscardableHandleManager);
     fRemoteFontIdToTypeface.set(wire.typefaceID, newTypeface);
     return std::move(newTypeface);
 }
-
-void SkStrikeClient::generateFontMetrics(const SkTypefaceProxy& typefaceProxy,
-                                         const SkScalerContextRec& rec,
-                                         SkPaint::FontMetrics* metrics) {
-    TRACE_EVENT1("skia", "generateFontMetrics", "rec", TRACE_STR_COPY(rec.dump().c_str()));
-    SkDebugf("generateFontMetrics: %s\n", rec.dump().c_str());
-    SkStrikeCache::Dump();
-    SkDEBUGFAIL("GlyphCacheMiss");
-
-    sk_bzero(metrics, sizeof(*metrics));
-}
-
-void SkStrikeClient::generateMetricsAndImage(const SkTypefaceProxy& typefaceProxy,
-                                             const SkScalerContextRec& rec,
-                                             SkArenaAlloc* alloc,
-                                             SkGlyph* glyph) {
-    TRACE_EVENT1("skia", "generateMetricsAndImage", "rec", TRACE_STR_COPY(rec.dump().c_str()));
-    SkDebugf("generateMetricsAndImage: %s\n", rec.dump().c_str());
-    SkStrikeCache::Dump();
-    SkDEBUGFAIL("GlyphCacheMiss");
-
-    glyph->zeroMetrics();
-}
-
-void SkStrikeClient::generatePath(const SkTypefaceProxy& typefaceProxy,
-                                  const SkScalerContextRec& rec,
-                                  SkGlyphID glyphID,
-                                  SkPath* path) {
-    TRACE_EVENT1("skia", "generatePath", "rec", TRACE_STR_COPY(rec.dump().c_str()));
-    SkDebugf("generatePath: %s\n", rec.dump().c_str());
-    SkStrikeCache::Dump();
-    SkDEBUGFAIL("GlyphCacheMiss");
-}
diff --git a/src/core/SkRemoteGlyphCache.h b/src/core/SkRemoteGlyphCache.h
index d1119cf70a..32d0b47c18 100644
--- a/src/core/SkRemoteGlyphCache.h
+++ b/src/core/SkRemoteGlyphCache.h
@@ -189,6 +189,14 @@ private:
 
 class SK_API SkStrikeClient {
 public:
+    enum CacheMissType : uint32_t {
+        kFontMetrics,
+        kGlyphMetrics,
+        kGlyphImage,
+        kGlyphPath,
+        kLast = kGlyphPath
+    };
+
     // An interface to delete handles that may be pinned by the remote server.
     class DiscardableHandleManager : public SkRefCnt {
     public:
@@ -197,6 +205,8 @@ public:
         // Returns true if the handle was unlocked and can be safely deleted. Once
         // successful, subsequent attempts to delete the same handle are invalid.
         virtual bool deleteHandle(SkDiscardableHandleId) = 0;
+
+        virtual void NotifyCacheMiss(CacheMissType) {}
     };
 
     SkStrikeClient(sk_sp<DiscardableHandleManager>);
@@ -212,19 +222,6 @@ public:
     // Returns false if the data is invalid.
     bool readStrikeData(const volatile void* memory, size_t memorySize);
 
-    // TODO: Remove these since we don't support pulling this data on-demand.
-    void generateFontMetrics(const SkTypefaceProxy& typefaceProxy,
-                             const SkScalerContextRec& rec,
-                             SkPaint::FontMetrics* metrics);
-    void generateMetricsAndImage(const SkTypefaceProxy& typefaceProxy,
-                                 const SkScalerContextRec& rec,
-                                 SkArenaAlloc* alloc,
-                                 SkGlyph* glyph);
-    void generatePath(const SkTypefaceProxy& typefaceProxy,
-                      const SkScalerContextRec& rec,
-                      SkGlyphID glyphID,
-                      SkPath* path);
-
 private:
     class DiscardableStrikePinner;
 
diff --git a/src/core/SkTypeface_remote.cpp b/src/core/SkTypeface_remote.cpp
index fe3fde83f5..cbddb1fcaa 100644
--- a/src/core/SkTypeface_remote.cpp
+++ b/src/core/SkTypeface_remote.cpp
@@ -6,15 +6,16 @@
  */
 
 #include "SkTypeface_remote.h"
-#include "SkRemoteGlyphCache.h"
-
 #include "SkPaint.h"
+#include "SkRemoteGlyphCache.h"
+#include "SkStrikeCache.h"
+#include "SkTraceEvent.h"
 
 SkScalerContextProxy::SkScalerContextProxy(sk_sp<SkTypeface> tf,
                                            const SkScalerContextEffects& effects,
                                            const SkDescriptor* desc,
-                                           SkStrikeClient* rsc)
-        : SkScalerContext{std::move(tf), effects, desc}, fClient{rsc} {}
+                                           sk_sp<SkStrikeClient::DiscardableHandleManager> manager)
+        : SkScalerContext{std::move(tf), effects, desc}, fDiscardableManager{std::move(manager)} {}
 
 unsigned SkScalerContextProxy::generateGlyphCount()  {
     SK_ABORT("Should never be called.");
@@ -31,21 +32,34 @@ void  SkScalerContextProxy::generateAdvance(SkGlyph* glyph) {
 }
 
 void SkScalerContextProxy::generateMetrics(SkGlyph* glyph) {
-    fClient->generateMetricsAndImage(*this->typefaceProxy(), this->getRec(), &fAlloc, glyph);
+    TRACE_EVENT1("skia", "generateMetrics", "rec", TRACE_STR_COPY(this->getRec().dump().c_str()));
+    SkDebugf("GlyphCacheMiss generateMetrics: %s\n", this->getRec().dump().c_str());
+
+    fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kGlyphMetrics);
+    glyph->zeroMetrics();
 }
 
 void SkScalerContextProxy::generateImage(const SkGlyph& glyph) {
+    TRACE_EVENT1("skia", "generateImage", "rec", TRACE_STR_COPY(this->getRec().dump().c_str()));
+    SkDebugf("GlyphCacheMiss generateImage: %s\n", this->getRec().dump().c_str());
+
+    fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kGlyphImage);
 }
 
 bool SkScalerContextProxy::generatePath(SkGlyphID glyphID, SkPath* path) {
-    fClient->generatePath(*this->typefaceProxy(), this->getRec(), glyphID, path);
+    TRACE_EVENT1("skia", "generatePath", "rec", TRACE_STR_COPY(this->getRec().dump().c_str()));
+    SkDebugf("GlyphCacheMiss generatePath: %s\n", this->getRec().dump().c_str());
+
+    fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kGlyphPath);
     return false;
 }
 
 void SkScalerContextProxy::generateFontMetrics(SkPaint::FontMetrics* metrics) {
-    fClient->generateFontMetrics(*this->typefaceProxy(), this->getRec(), metrics);
-}
+    TRACE_EVENT1(
+            "skia", "generateFontMetrics", "rec", TRACE_STR_COPY(this->getRec().dump().c_str()));
+    SkDebugf("GlyphCacheMiss generateFontMetrics: %s\n", this->getRec().dump().c_str());
+    SkDEBUGCODE(SkStrikeCache::Dump());
 
-SkTypefaceProxy* SkScalerContextProxy::typefaceProxy() {
-    return SkTypefaceProxy::DownCast(this->getTypeface());
+    fDiscardableManager->NotifyCacheMiss(SkStrikeClient::CacheMissType::kFontMetrics);
+    sk_bzero(metrics, sizeof(*metrics));
 }
diff --git a/src/core/SkTypeface_remote.h b/src/core/SkTypeface_remote.h
index ad729aded3..ff5bc66775 100644
--- a/src/core/SkTypeface_remote.h
+++ b/src/core/SkTypeface_remote.h
@@ -13,18 +13,16 @@
 #include "SkFontDescriptor.h"
 #include "SkFontStyle.h"
 #include "SkPaint.h"
+#include "SkRemoteGlyphCache.h"
 #include "SkScalerContext.h"
 #include "SkTypeface.h"
 
-class SkStrikeClient;
-class SkTypefaceProxy;
-
 class SkScalerContextProxy : public SkScalerContext {
 public:
     SkScalerContextProxy(sk_sp<SkTypeface> tf,
                          const SkScalerContextEffects& effects,
                          const SkDescriptor* desc,
-                         SkStrikeClient* rsc);
+                         sk_sp<SkStrikeClient::DiscardableHandleManager> manager);
 
 protected:
     unsigned generateGlyphCount() override;
@@ -42,10 +40,8 @@ private:
     static constexpr size_t kMinGlyphImageSize = 16 /* height */ * 8 /* width */;
     static constexpr size_t kMinAllocAmount = kMinGlyphImageSize * kMinGlyphCount;
 
-    SkTypefaceProxy* typefaceProxy();
-
     SkArenaAlloc          fAlloc{kMinAllocAmount};
-    SkStrikeClient* const fClient;
+    sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
     typedef SkScalerContext INHERITED;
 };
 
@@ -55,14 +51,13 @@ public:
                     int glyphCount,
                     const SkFontStyle& style,
                     bool isFixed,
-                    SkStrikeClient* rsc)
-            : INHERITED{style, false}, fFontId{fontId}, fGlyphCount{glyphCount}, fRsc{rsc} {}
+                    sk_sp<SkStrikeClient::DiscardableHandleManager> manager)
+            : INHERITED{style, false}
+            , fFontId{fontId}
+            , fGlyphCount{glyphCount}
+            , fDiscardableManager{std::move(manager)} {}
     SkFontID remoteTypefaceID() const {return fFontId;}
     int glyphCount() const {return fGlyphCount;}
-    static SkTypefaceProxy* DownCast(SkTypeface* typeface) {
-        // TODO: how to check the safety of the down cast?
-        return (SkTypefaceProxy*)typeface;
-    }
 
 protected:
     int onGetUPEM() const override { SK_ABORT("Should never be called."); return 0; }
@@ -97,16 +92,11 @@ protected:
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects& effects,
                                            const SkDescriptor* desc) const override {
         return new SkScalerContextProxy(sk_ref_sp(const_cast<SkTypefaceProxy*>(this)), effects,
-                                         desc, fRsc);
-
+                                        desc, fDiscardableManager);
     }
     void onFilterRec(SkScalerContextRec* rec) const override {
-        // Add all the device information here.
-        //rec->fPost2x2[0][0] = 0.5f;
-
-        // This would be the best place to run the host SkTypeface_* onFilterRec.
-        // Can we move onFilterRec to the FongMgr, that way we don't need to cross the boundary to
-        // filter.
+        // The rec filtering is already applied by the server when generating
+        // the glyphs.
     }
     void onGetFontDescriptor(SkFontDescriptor*, bool*) const override {
         SK_ABORT("Should never be called.");
@@ -138,8 +128,7 @@ private:
     const SkFontID        fFontId;
     const int             fGlyphCount;
 
-    // TODO: Does this need a ref to the strike client? If yes, make it a weak ref.
-    SkStrikeClient* const fRsc;
+    sk_sp<SkStrikeClient::DiscardableHandleManager> fDiscardableManager;
 
     typedef SkTypeface INHERITED;
 };
diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp
index aa26fbeea8..23e684315e 100644
--- a/tests/SkRemoteGlyphCacheTest.cpp
+++ b/tests/SkRemoteGlyphCacheTest.cpp
@@ -22,7 +22,7 @@
 class DiscardableManager : public SkStrikeServer::DiscardableHandleManager,
                            public SkStrikeClient::DiscardableHandleManager {
 public:
-    DiscardableManager() = default;
+    DiscardableManager() { sk_bzero(&fCacheMissCount, sizeof(fCacheMissCount)); }
     ~DiscardableManager() override = default;
 
     // Server implementation.
@@ -39,6 +39,7 @@ public:
 
     // Client implementation.
     bool deleteHandle(SkDiscardableHandleId id) override { return id <= fLastDeletedHandleId; }
+    void NotifyCacheMiss(SkStrikeClient::CacheMissType type) override { fCacheMissCount[type]++; }
 
     void unlockAll() { fLockedHandles.reset(); }
     void unlockAndDeleteAll() {
@@ -47,11 +48,13 @@ public:
     }
     const SkTHashSet<SkDiscardableHandleId>& lockedHandles() const { return fLockedHandles; }
     SkDiscardableHandleId handleCount() { return fNextHandleId; }
+    int cacheMissCount(SkStrikeClient::CacheMissType type) { return fCacheMissCount[type]; }
 
 private:
     SkDiscardableHandleId fNextHandleId = 0u;
     SkDiscardableHandleId fLastDeletedHandleId = 0u;
     SkTHashSet<SkDiscardableHandleId> fLockedHandles;
+    int fCacheMissCount[SkStrikeClient::CacheMissType::kLast + 1u];
 };
 
 sk_sp<SkTextBlob> buildTextBlob(sk_sp<SkTypeface> tf, int glyphCount) {
@@ -114,7 +117,7 @@ DEF_TEST(SkRemoteGlyphCache_TypefaceSerialization, reporter) {
 
     auto client_tf = client.deserializeTypeface(tf_data->data(), tf_data->size());
     REPORTER_ASSERT(reporter, client_tf);
-    REPORTER_ASSERT(reporter, SkTypefaceProxy::DownCast(client_tf.get())->remoteTypefaceID() ==
+    REPORTER_ASSERT(reporter, static_cast<SkTypefaceProxy*>(client_tf.get())->remoteTypefaceID() ==
                                       server_tf->uniqueID());
 
     // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
@@ -373,4 +376,34 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_DrawTextAsDFT, reporter, c
     // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
     discardableManager->unlockAndDeleteAll();
 }
+
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_CacheMissReporting, reporter, ctxInfo) {
+    sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
+    SkStrikeServer server(discardableManager.get());
+    SkStrikeClient client(discardableManager);
+
+    auto serverTf = SkTypeface::MakeFromName("monospace", SkFontStyle());
+    auto tfData = server.serializeTypeface(serverTf.get());
+    auto clientTf = client.deserializeTypeface(tfData->data(), tfData->size());
+    REPORTER_ASSERT(reporter, clientTf);
+    int glyphCount = 10;
+    auto clientBlob = buildTextBlob(clientTf, glyphCount);
+
+    // Raster the client-side blob without the glyph data, we should get cache miss notifications.
+    SkPaint paint;
+    SkMatrix matrix = SkMatrix::I();
+    RasterBlob(clientBlob, 10, 10, paint, ctxInfo.grContext(), &matrix);
+    REPORTER_ASSERT(reporter,
+                    discardableManager->cacheMissCount(SkStrikeClient::kFontMetrics) == 1);
+    REPORTER_ASSERT(reporter,
+                    discardableManager->cacheMissCount(SkStrikeClient::kGlyphMetrics) == 10);
+
+    // There shouldn't be any image or path requests, since we mark the glyph as empty on a cache
+    // miss.
+    REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(SkStrikeClient::kGlyphImage) == 0);
+    REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(SkStrikeClient::kGlyphPath) == 0);
+
+    // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
+    discardableManager->unlockAndDeleteAll();
+}
 #endif