Use shared pointer reference instead of raw pointer

To be able to increment the reference from layout result, keep shared
pointer reference instead of raw pointer.

Bug: 179113771
Test: minikin_tests
Change-Id: Ibc21f1a10c33070c95d891a0eb70d5aae250e6db

7 files changed, 48 insertions, 40 deletions

diff --git a/include/minikin/Font.h b/include/minikin/Font.h
index bd31429..434cd16 100644
--- a/include/minikin/Font.h
+++ b/include/minikin/Font.h
@@ -58,7 +58,10 @@ struct FakedFont {
     inline bool operator!=(const FakedFont& o) const { return !(*this == o); }
 
     // ownership is the enclosing FontCollection
-    const Font* font;
+    // FakedFont will be stored in the LayoutCache. It is not a good idea too keep font instance
+    // even if the enclosing FontCollection, i.e. Typeface is GC-ed. The layout cache is only
+    // purged when it is overflown, thus intentionally keep only reference.
+    const std::shared_ptr<Font>& font;
     FontFakery fakery;
 };
 
diff --git a/include/minikin/FontFamily.h b/include/minikin/FontFamily.h
index a23190b..34f601a 100644
--- a/include/minikin/FontFamily.h
+++ b/include/minikin/FontFamily.h
@@ -66,6 +66,7 @@ public:
     // API's for enumerating the fonts in a family. These don't guarantee any particular order
     size_t getNumFonts() const { return mFonts.size(); }
     const Font* getFont(size_t index) const { return mFonts[index].get(); }
+    const std::shared_ptr<Font>& getFontRef(size_t index) const { return mFonts[index]; }
     FontStyle getStyle(size_t index) const { return mFonts[index]->style(); }
     bool isColorEmojiFamily() const { return mIsColorEmoji; }
     const std::unordered_set<AxisTag>& supportedAxes() const { return mSupportedAxes; }
diff --git a/include/minikin/Layout.h b/include/minikin/Layout.h
index f25faf2..388a7a7 100644
--- a/include/minikin/Layout.h
+++ b/include/minikin/Layout.h
@@ -84,7 +84,8 @@ public:
 
     // public accessors
     size_t nGlyphs() const { return mGlyphs.size(); }
-    const Font* getFont(int i) const { return mGlyphs[i].font.font; }
+    const Font* getFont(int i) const { return mGlyphs[i].font.font.get(); }
+    const std::shared_ptr<Font>& getFontRef(int i) const { return mGlyphs[i].font.font; }
     FontFakery getFakery(int i) const { return mGlyphs[i].font.fakery; }
     unsigned int getGlyphId(int i) const { return mGlyphs[i].glyph_id; }
     float getX(int i) const { return mGlyphs[i].x; }
diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp
index 26695e9..a8b96c3 100644
--- a/libs/minikin/FontCollection.cpp
+++ b/libs/minikin/FontCollection.cpp
@@ -540,7 +540,7 @@ std::vector<FontCollection::Run> FontCollection::itemize(U16StringPiece text, Fo
 
     if (result.size() > runMax) {
         // The itemization has terminated since it reaches the runMax. Remove last unfinalized runs.
-        result.resize(runMax);
+        return std::vector<Run>(result.begin(), result.begin() + runMax);
     }
     return result;
 }
diff --git a/libs/minikin/FontFamily.cpp b/libs/minikin/FontFamily.cpp
index ef262e9..f11c16e 100644
--- a/libs/minikin/FontFamily.cpp
+++ b/libs/minikin/FontFamily.cpp
@@ -145,21 +145,23 @@ static FontFakery computeFakery(FontStyle wanted, FontStyle actual) {
 }
 
 FakedFont FontFamily::getClosestMatch(FontStyle style) const {
-    Font* bestFont = mFonts[0].get();
+    int bestIndex = 0;
+    Font* bestFont = mFonts[bestIndex].get();
     int bestMatch = computeMatch(bestFont->style(), style);
     for (size_t i = 1; i < mFonts.size(); i++) {
         Font* font = mFonts[i].get();
         int match = computeMatch(font->style(), style);
         if (i == 0 || match < bestMatch) {
             bestFont = font;
+            bestIndex = i;
             bestMatch = match;
         }
     }
-    return FakedFont{bestFont, computeFakery(style, bestFont->style())};
+    return FakedFont{mFonts[bestIndex], computeFakery(style, bestFont->style())};
 }
 
 void FontFamily::computeCoverage() {
-    const Font* font = getClosestMatch(FontStyle()).font;
+    const std::shared_ptr<Font>& font = getClosestMatch(FontStyle()).font;
     HbBlob cmapTable(font->baseFont(), MinikinFont::MakeTag('c', 'm', 'a', 'p'));
     if (cmapTable.get() == nullptr) {
         ALOGE("Could not get cmap table size!\n");
diff --git a/libs/minikin/LayoutCore.cpp b/libs/minikin/LayoutCore.cpp
index 7ec0b53..03c72b2 100644
--- a/libs/minikin/LayoutCore.cpp
+++ b/libs/minikin/LayoutCore.cpp
@@ -385,13 +385,13 @@ LayoutPiece::LayoutPiece(const U16StringPiece& textBuf, const Range& range, bool
          isRtl ? --run_ix : ++run_ix) {
         FontCollection::Run& run = items[run_ix];
         const FakedFont& fakedFont = run.fakedFont;
-        auto it = fontMap.find(fakedFont.font);
+        auto it = fontMap.find(fakedFont.font.get());
         uint8_t font_ix;
         if (it == fontMap.end()) {
             // First time to see this font.
             font_ix = mFonts.size();
             mFonts.push_back(fakedFont);
-            fontMap.insert(std::make_pair(fakedFont.font, font_ix));
+            fontMap.insert(std::make_pair(fakedFont.font.get(), font_ix));
 
             // We override some functions which are not thread safe.
             HbFontUniquePtr font(hb_font_create_sub_font(fakedFont.font->baseFont().get()));
diff --git a/tests/unittest/FontCollectionItemizeTest.cpp b/tests/unittest/FontCollectionItemizeTest.cpp
index 8ac038d..35745c7 100644
--- a/tests/unittest/FontCollectionItemizeTest.cpp
+++ b/tests/unittest/FontCollectionItemizeTest.cpp
@@ -102,8 +102,8 @@ std::vector<FontCollection::Run> itemize(const std::shared_ptr<FontCollection>&
 
 // Utility function to obtain font path associated with run.
 std::string getFontName(const FontCollection::Run& run) {
-    EXPECT_NE(nullptr, run.fakedFont.font);
-    return getBasename(run.fakedFont.font->typeface()->GetFontPath());
+    EXPECT_NE(nullptr, run.fakedFont.font.get());
+    return getBasename(run.fakedFont.font.get()->typeface()->GetFontPath());
 }
 
 // Utility function to obtain LocaleList from string.
@@ -514,13 +514,13 @@ TEST(FontCollectionItemizeTest, itemize_variationSelector) {
     ASSERT_EQ(1U, runs.size());
     EXPECT_EQ(0, runs[0].start);
     EXPECT_EQ(1, runs[0].end);
-    EXPECT_TRUE(runs[0].fakedFont.font == nullptr || kLatinFont == getFontName(runs[0]));
+    EXPECT_TRUE(runs[0].fakedFont.font.get() == nullptr || kLatinFont == getFontName(runs[0]));
 
     runs = itemize(collection, "U+FE00", "zh-Hant");
     ASSERT_EQ(1U, runs.size());
     EXPECT_EQ(0, runs[0].start);
     EXPECT_EQ(1, runs[0].end);
-    EXPECT_TRUE(runs[0].fakedFont.font == nullptr || kLatinFont == getFontName(runs[0]));
+    EXPECT_TRUE(runs[0].fakedFont.font.get() == nullptr || kLatinFont == getFontName(runs[0]));
 
     // First font family (Regular.ttf) supports U+203C but doesn't support U+203C U+FE0F.
     // Emoji.ttf font supports U+203C U+FE0F.  Emoji.ttf should be selected.
@@ -651,13 +651,13 @@ TEST(FontCollectionItemizeTest, itemize_variationSelectorSupplement) {
     ASSERT_EQ(1U, runs.size());
     EXPECT_EQ(0, runs[0].start);
     EXPECT_EQ(2, runs[0].end);
-    EXPECT_TRUE(runs[0].fakedFont.font == nullptr || kLatinFont == getFontName(runs[0]));
+    EXPECT_TRUE(runs[0].fakedFont.font.get() == nullptr || kLatinFont == getFontName(runs[0]));
 
     runs = itemize(collection, "U+E0100", "zh-Hant");
     ASSERT_EQ(1U, runs.size());
     EXPECT_EQ(0, runs[0].start);
     EXPECT_EQ(2, runs[0].end);
-    EXPECT_TRUE(runs[0].fakedFont.font == nullptr || kLatinFont == getFontName(runs[0]));
+    EXPECT_TRUE(runs[0].fakedFont.font.get() == nullptr || kLatinFont == getFontName(runs[0]));
 }
 
 TEST(FontCollectionItemizeTest, itemize_no_crash) {
@@ -952,14 +952,15 @@ TEST(FontCollectionItemizeTest, itemize_LocaleScore) {
         // Do itemize
         auto runs = itemize(collection, "U+9AA8", testCase.userPreferredLocale);
         ASSERT_EQ(1U, runs.size());
-        ASSERT_NE(nullptr, runs[0].fakedFont.font);
+        ASSERT_NE(nullptr, runs[0].fakedFont.font.get());
 
         // First family doesn't support U+9AA8 and others support it, so the first font should not
         // be selected.
-        EXPECT_NE(firstFamilyMinikinFont.get(), runs[0].fakedFont.font->typeface().get());
+        EXPECT_NE(firstFamilyMinikinFont.get(), runs[0].fakedFont.font.get()->typeface().get());
 
         // Lookup used font family by MinikinFont*.
-        const int usedLocaleIndex = fontLocaleIdxMap[runs[0].fakedFont.font->typeface().get()];
+        const int usedLocaleIndex =
+                fontLocaleIdxMap[runs[0].fakedFont.font.get()->typeface().get()];
         EXPECT_EQ(testCase.selectedFontIndex, usedLocaleIndex);
     }
 }
@@ -1520,10 +1521,10 @@ TEST(FontCollectionItemizeTest, itemizeShouldKeepOrderForVS) {
     // Both fontA/fontB support U+35A8 but don't support U+35A8 U+E0100. The first font should be
     // selected.
     auto runs = itemize(collection, "U+35A8 U+E0100");
-    EXPECT_EQ(familyA->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(familyA->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(reversedCollection, "U+35A8 U+E0100");
-    EXPECT_EQ(familyB->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(familyB->getFont(0), runs[0].fakedFont.font.get());
 }
 
 // For b/29585939
@@ -1543,10 +1544,10 @@ TEST(FontCollectionItemizeTest, itemizeShouldKeepOrderForVS2) {
     // Both hasCmapFormat14Font/noCmapFormat14Font support U+5380 but don't support U+5380 U+E0100.
     // The first font should be selected.
     auto runs = itemize(collection, "U+5380 U+E0100");
-    EXPECT_EQ(hasCmapFormat14Family->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(hasCmapFormat14Family->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(reversedCollection, "U+5380 U+E0100");
-    EXPECT_EQ(noCmapFormat14Family->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(noCmapFormat14Family->getFont(0), runs[0].fakedFont.font.get());
 }
 
 TEST(FontCollectionItemizeTest, colorEmojiSelectionTest) {
@@ -1560,44 +1561,44 @@ TEST(FontCollectionItemizeTest, colorEmojiSelectionTest) {
     // Both textEmojiFamily and colorEmojiFamily supports U+203C and U+23E9.
     // U+203C is text default emoji, and U+23E9 is color default emoji.
     auto runs = itemize(collection, "U+203C", "en-US,en-Zsym");
-    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "U+23E9", "en-US,en-Zsym");
-    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(collection, "U+203C", "en-US,en-Zsye");
-    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "U+23E9", "en-US,en-Zsye");
-    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(collection, "U+203C", "ja-Zsym-JP");
-    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "U+23E9", "ja-Zsym-JP");
-    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(collection, "U+203C", "ja-Zsye-JP");
-    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "U+23E9", "ja-Zsye-JP");
-    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(collection, "U+203C", "ja-JP-u-em-text");
-    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "U+23E9", "ja-JP-u-em-text");
-    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(collection, "U+203C", "ja-JP-u-em-emoji");
-    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "U+23E9", "ja-JP-u-em-emoji");
-    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(collection, "U+203C", "ja-JP,und-Zsym");
-    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "U+23E9", "ja-JP,und-Zsym");
-    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(textEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
 
     runs = itemize(collection, "U+203C", "ja-JP,und-Zsye");
-    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "U+23E9", "ja-JP,und-Zsye");
-    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(colorEmojiFamily->getFont(0), runs[0].fakedFont.font.get());
 }
 
 TEST(FontCollectionItemizeTest, customFallbackTest) {
@@ -1611,11 +1612,11 @@ TEST(FontCollectionItemizeTest, customFallbackTest) {
     auto collection = std::make_shared<FontCollection>(families);
 
     auto runs = itemize(collection, "'a'", "");
-    EXPECT_EQ(customFallbackFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(customFallbackFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "'a'", "en-US");
-    EXPECT_EQ(customFallbackFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(customFallbackFamily->getFont(0), runs[0].fakedFont.font.get());
     runs = itemize(collection, "'a'", "ja-JP");
-    EXPECT_EQ(customFallbackFamily->getFont(0), runs[0].fakedFont.font);
+    EXPECT_EQ(customFallbackFamily->getFont(0), runs[0].fakedFont.font.get());
 }
 
 }  // namespace minikin