Use a MRUCache for typeface cache used by Harfbuzz

This CL is replacing the previous cache by an MRUCache.

The previous cache was an 'std::map' and was unbounded.
It was not an issue since typically the amount of typeface
loaded is low on the browser side. Unfortunately, that was
not the case when expensive fallback fonts were used and
lot of fonts were installed on the user computer.

The GlyphCache is still an std::map, but it is bounded
to the max amount of glyphs that a font can have (e.g. 16-bits).

Bug: 890298
Change-Id: I1148ea7ebfc285dfcc05e8da454db830e809ebb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1874246
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709101}

ui/gfx/harfbuzz_font_skia.cc

@@ -10,6 +10,7 @@
 #include <limits>
 #include <map>
 
+#include "base/containers/mru_cache.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
@@ -24,14 +25,13 @@ namespace gfx {
 
 namespace {
 
-class HarfBuzzFace;
+class TypefaceData;
 
 // Maps from code points to glyph indices in a font.
-typedef std::map<uint32_t, uint16_t> GlyphCache;
+using GlyphCache = std::map<uint32_t, uint16_t>;
 
-typedef std::pair<HarfBuzzFace, GlyphCache> FaceCache;
-
-// Font data provider for HarfBuzz using Skia. Copied from Blink.
+// Wraps a custom user data attached to a hb_font object. Font data provider for
+// HarfBuzz using Skia. Copied from Blink.
 // TODO(ckocagil): Eliminate the duplication. http://crbug.com/368375
 struct FontData {
   explicit FontData(GlyphCache* glyph_cache) : glyph_cache_(glyph_cache) {}
@@ -231,33 +231,35 @@ hb_blob_t* GetFontTable(hb_face_t* face, hb_tag_t tag, void* user_data) {
                         DeleteArrayByType<char>);
 }
 
-void UnrefSkTypeface(void* data) {
-  SkTypeface* skia_face = reinterpret_cast<SkTypeface*>(data);
-  SkSafeUnref(skia_face);
-}
-
-// Wrapper class for a HarfBuzz face created from a given Skia face.
-class HarfBuzzFace {
+// For a given skia typeface, maps to its harfbuzz face and its glyphs cache.
+class TypefaceData {
  public:
-  HarfBuzzFace() : face_(NULL) {}
-
-  ~HarfBuzzFace() {
-    if (face_)
-      hb_face_destroy(face_);
-  }
-
-  void Init(SkTypeface* skia_face) {
-    SkSafeRef(skia_face);
-    face_ = hb_face_create_for_tables(GetFontTable, skia_face, UnrefSkTypeface);
+  explicit TypefaceData(sk_sp<SkTypeface> skia_face) : sk_typeface_(skia_face) {
+    face_ = hb_face_create_for_tables(GetFontTable, skia_face.get(), nullptr);
     DCHECK(face_);
   }
 
-  hb_face_t* get() {
-    return face_;
+  TypefaceData(TypefaceData&& data) {
+    face_ = data.face_;
+    glyphs_ = std::move(data.glyphs_);
+    data.face_ = nullptr;
   }
 
+  ~TypefaceData() { hb_face_destroy(face_); }
+
+  hb_face_t* face() { return face_; }
+  GlyphCache* glyphs() { return &glyphs_; }
+
  private:
-  hb_face_t* face_;
+  TypefaceData() = delete;
+
+  GlyphCache glyphs_;
+  hb_face_t* face_ = nullptr;
+
+  // The skia typeface must outlive |face_| since it's being used by harfbuzz.
+  sk_sp<SkTypeface> sk_typeface_;
+
+  DISALLOW_COPY_AND_ASSIGN(TypefaceData);
 };
 
 }  // namespace
@@ -267,21 +269,27 @@ hb_font_t* CreateHarfBuzzFont(sk_sp<SkTypeface> skia_face,
                               SkScalar text_size,
                               const FontRenderParams& params,
                               bool subpixel_rendering_suppressed) {
-  // TODO(https://crbug.com/890298): This shouldn't grow indefinitely.
-  // Maybe use base::MRUCache?
-  static base::NoDestructor<std::map<SkFontID, FaceCache>> face_caches;
-
-  FaceCache* face_cache = &(*face_caches)[skia_face->uniqueID()];
-  if (face_cache->first.get() == NULL)
-    face_cache->first.Init(skia_face.get());
+  // A cache from Skia font to harfbuzz typeface information.
+  using TypefaceCache = base::MRUCache<SkFontID, TypefaceData>;
+
+  constexpr int kTypefaceCacheSize = 64;
+  static base::NoDestructor<TypefaceCache> face_caches(kTypefaceCacheSize);
+
+  TypefaceCache* typeface_cache = face_caches.get();
+  TypefaceCache::iterator typeface_data =
+      typeface_cache->Get(skia_face->uniqueID());
+  if (typeface_data == typeface_cache->end()) {
+    TypefaceData new_typeface_data(skia_face);
+    typeface_data = typeface_cache->Put(skia_face->uniqueID(),
+                                        std::move(new_typeface_data));
+  }
 
-  hb_font_t* harfbuzz_font = nullptr;
-  if (!harfbuzz_font)
-    harfbuzz_font = hb_font_create(face_cache->first.get());
+  DCHECK(typeface_data->second.face());
+  hb_font_t* harfbuzz_font = hb_font_create(typeface_data->second.face());
 
   const int scale = SkiaScalarToHarfBuzzUnits(text_size);
   hb_font_set_scale(harfbuzz_font, scale, scale);
-  FontData* hb_font_data = new FontData(&face_cache->second);
+  FontData* hb_font_data = new FontData(typeface_data->second.glyphs());
   hb_font_data->font_.setTypeface(std::move(skia_face));
   hb_font_data->font_.setSize(text_size);
   // TODO(ckocagil): Do we need to update these params later?