Keep source ID into native object

To avoid static field and synchronization for the mapping, keep source
identifier into native font instance.

Bug: 179113771
Test: minikin_tests
Test: hwui_unit_tests
Test: atest CtsTextTestCases CtsGraphicsTestCases
Change-Id: I238e7b8090ee89101937ec22cbe7c68aea97bcfd

8 files changed, 47 insertions, 44 deletions

diff --git a/graphics/java/android/graphics/fonts/Font.java b/graphics/java/android/graphics/fonts/Font.java
index 7b67ca919a0f..f826b24b2df2 100644
--- a/graphics/java/android/graphics/fonts/Font.java
+++ b/graphics/java/android/graphics/fonts/Font.java
@@ -27,7 +27,6 @@ import android.graphics.RectF;
 import android.os.LocaleList;
 import android.os.ParcelFileDescriptor;
 import android.text.TextUtils;
-import android.util.LongSparseLongArray;
 import android.util.TypedValue;
 
 import com.android.internal.annotations.GuardedBy;
@@ -67,11 +66,6 @@ public final class Font {
             NativeAllocationRegistry.createMalloced(Font.class.getClassLoader(),
                     nGetReleaseNativeFont());
 
-    private static final Object SOURCE_ID_LOCK = new Object();
-    @GuardedBy("SOURCE_ID_LOCK")
-    private static final LongSparseLongArray FONT_SOURCE_ID_MAP =
-            new LongSparseLongArray(300);  // System font has 200 fonts, so 300 should be enough.
-
     /**
      * A builder class for creating new Font.
      */
@@ -523,8 +517,6 @@ public final class Font {
     private @Nullable FontVariationAxis[] mAxes = null;
     @GuardedBy("mLock")
     private @NonNull LocaleList mLocaleList = null;
-    @GuardedBy("mLock")
-    private int mSourceIdentifier = -1;
 
     /**
      * Use Builder instead
@@ -758,20 +750,7 @@ public final class Font {
      * @return an unique identifier for the font source data.
      */
     public int getSourceIdentifier() {
-        synchronized (mLock) {
-            if (mSourceIdentifier == -1) {
-                long bufferAddress = nGetBufferAddress(mNativePtr);
-                synchronized (SOURCE_ID_LOCK) {
-                    long id = FONT_SOURCE_ID_MAP.get(bufferAddress, -1);
-                    if (id == -1) {
-                        id = FONT_SOURCE_ID_MAP.size();
-                        FONT_SOURCE_ID_MAP.append(bufferAddress, id);
-                    }
-                    mSourceIdentifier = (int) id;
-                }
-            }
-            return mSourceIdentifier;
-        }
+        return nGetSourceId(mNativePtr);
     }
 
     /**
@@ -890,6 +869,9 @@ public final class Font {
     private static native long nGetBufferAddress(long font);
 
     @CriticalNative
+    private static native int nGetSourceId(long font);
+
+    @CriticalNative
     private static native long nGetReleaseNativeFont();
 
     @FastNative
diff --git a/libs/hwui/hwui/MinikinSkia.cpp b/libs/hwui/hwui/MinikinSkia.cpp
index 0e338f35b8e7..2db3ace1cd43 100644
--- a/libs/hwui/hwui/MinikinSkia.cpp
+++ b/libs/hwui/hwui/MinikinSkia.cpp
@@ -30,10 +30,11 @@
 
 namespace android {
 
-MinikinFontSkia::MinikinFontSkia(sk_sp<SkTypeface> typeface, const void* fontData, size_t fontSize,
-                                 std::string_view filePath, int ttcIndex,
+MinikinFontSkia::MinikinFontSkia(sk_sp<SkTypeface> typeface, int sourceId, const void* fontData,
+                                 size_t fontSize, std::string_view filePath, int ttcIndex,
                                  const std::vector<minikin::FontVariation>& axes)
         : mTypeface(std::move(typeface))
+        , mSourceId(sourceId)
         , mFontData(fontData)
         , mFontSize(fontSize)
         , mTtcIndex(ttcIndex)
@@ -141,8 +142,8 @@ std::shared_ptr<minikin::MinikinFont> MinikinFontSkia::createFontWithVariation(
     sk_sp<SkFontMgr> fm(SkFontMgr::RefDefault());
     sk_sp<SkTypeface> face(fm->makeFromStream(std::move(stream), args));
 
-    return std::make_shared<MinikinFontSkia>(std::move(face), mFontData, mFontSize, mFilePath,
-                                             ttcIndex, variations);
+    return std::make_shared<MinikinFontSkia>(std::move(face), mSourceId, mFontData, mFontSize,
+                                             mFilePath, ttcIndex, variations);
 }
 
 // hinting<<16 | edging<<8 | bools:5bits
diff --git a/libs/hwui/hwui/MinikinSkia.h b/libs/hwui/hwui/MinikinSkia.h
index 77a21428f36a..de9a5c2af0aa 100644
--- a/libs/hwui/hwui/MinikinSkia.h
+++ b/libs/hwui/hwui/MinikinSkia.h
@@ -30,7 +30,7 @@ namespace android {
 
 class ANDROID_API MinikinFontSkia : public minikin::MinikinFont {
 public:
-    MinikinFontSkia(sk_sp<SkTypeface> typeface, const void* fontData, size_t fontSize,
+    MinikinFontSkia(sk_sp<SkTypeface> typeface, int sourceId, const void* fontData, size_t fontSize,
                     std::string_view filePath, int ttcIndex,
                     const std::vector<minikin::FontVariation>& axes);
 
@@ -62,6 +62,7 @@ public:
     const std::vector<minikin::FontVariation>& GetAxes() const;
     std::shared_ptr<minikin::MinikinFont> createFontWithVariation(
             const std::vector<minikin::FontVariation>&) const;
+    int GetSourceId() const override { return mSourceId; }
 
     static uint32_t packFontFlags(const SkFont&);
     static void unpackFontFlags(SkFont*, uint32_t fontFlags);
@@ -73,6 +74,7 @@ public:
 private:
     sk_sp<SkTypeface> mTypeface;
 
+    int mSourceId;
     // A raw pointer to the font data - it should be owned by some other object with
     // lifetime at least as long as this object.
     const void* mFontData;
diff --git a/libs/hwui/hwui/Typeface.cpp b/libs/hwui/hwui/Typeface.cpp
index 03f1d62625f1..5a9d2508230e 100644
--- a/libs/hwui/hwui/Typeface.cpp
+++ b/libs/hwui/hwui/Typeface.cpp
@@ -185,9 +185,9 @@ void Typeface::setRobotoTypefaceForTest() {
     sk_sp<SkTypeface> typeface = SkTypeface::MakeFromStream(std::move(fontData));
     LOG_ALWAYS_FATAL_IF(typeface == nullptr, "Failed to make typeface from %s", kRobotoFont);
 
-    std::shared_ptr<minikin::MinikinFont> font = std::make_shared<MinikinFontSkia>(
-            std::move(typeface), data, st.st_size, kRobotoFont, 0,
-            std::vector<minikin::FontVariation>());
+    std::shared_ptr<minikin::MinikinFont> font =
+            std::make_shared<MinikinFontSkia>(std::move(typeface), 0, data, st.st_size, kRobotoFont,
+                                              0, std::vector<minikin::FontVariation>());
     std::vector<std::shared_ptr<minikin::Font>> fonts;
     fonts.push_back(minikin::Font::Builder(font).build());
 
diff --git a/libs/hwui/jni/FontFamily.cpp b/libs/hwui/jni/FontFamily.cpp
index 2e85840cad99..ce5ac382aeff 100644
--- a/libs/hwui/jni/FontFamily.cpp
+++ b/libs/hwui/jni/FontFamily.cpp
@@ -17,15 +17,16 @@
 #undef LOG_TAG
 #define LOG_TAG "Minikin"
 
+#include <nativehelper/ScopedPrimitiveArray.h>
+#include <nativehelper/ScopedUtfChars.h>
+#include "FontUtils.h"
+#include "GraphicsJNI.h"
 #include "SkData.h"
 #include "SkFontMgr.h"
 #include "SkRefCnt.h"
 #include "SkTypeface.h"
-#include "GraphicsJNI.h"
-#include <nativehelper/ScopedPrimitiveArray.h>
-#include <nativehelper/ScopedUtfChars.h>
 #include "Utils.h"
-#include "FontUtils.h"
+#include "fonts/Font.h"
 
 #include <hwui/MinikinSkia.h>
 #include <hwui/Typeface.h>
@@ -35,6 +36,12 @@
 
 #include <memory>
 
+///////////////////////////////////////////////////////////////////////////////////////////////////
+//
+// The following JNI methods are kept only for compatibility reasons due to hidden API accesses.
+//
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
 namespace android {
 
 struct NativeFamilyBuilder {
@@ -125,8 +132,8 @@ static bool addSkTypeface(NativeFamilyBuilder* builder, sk_sp<SkData>&& data, in
         return false;
     }
     std::shared_ptr<minikin::MinikinFont> minikinFont =
-            std::make_shared<MinikinFontSkia>(std::move(face), fontPtr, fontSize, "", ttcIndex,
-                    builder->axes);
+            std::make_shared<MinikinFontSkia>(std::move(face), fonts::getNewSourceId(), fontPtr,
+                                              fontSize, "", ttcIndex, builder->axes);
     minikin::Font::Builder fontBuilder(minikinFont);
 
     if (weight != RESOLVE_BY_FONT_TABLE) {
diff --git a/libs/hwui/jni/fonts/Font.cpp b/libs/hwui/jni/fonts/Font.cpp
index c8471a9b8feb..5a972f56ea87 100644
--- a/libs/hwui/jni/fonts/Font.cpp
+++ b/libs/hwui/jni/fonts/Font.cpp
@@ -137,12 +137,9 @@ static jlong Font_Builder_clone(JNIEnv* env, jobject clazz, jlong fontPtr, jlong
     sk_sp<SkTypeface> newTypeface = minikinSkia->GetSkTypeface()->makeClone(args);
 
     std::shared_ptr<minikin::MinikinFont> newMinikinFont = std::make_shared<MinikinFontSkia>(
-        std::move(newTypeface),
-        minikinSkia->GetFontData(),
-        minikinSkia->GetFontSize(),
-        minikinSkia->getFilePath(),
-        minikinSkia->GetFontIndex(),
-        builder->axes);
+            std::move(newTypeface), minikinSkia->GetSourceId(), minikinSkia->GetFontData(),
+            minikinSkia->GetFontSize(), minikinSkia->getFilePath(), minikinSkia->GetFontIndex(),
+            builder->axes);
     std::shared_ptr<minikin::Font> newFont = minikin::Font::Builder(newMinikinFont)
               .setWeight(weight)
               .setSlant(static_cast<minikin::FontStyle::Slant>(italic))
@@ -279,6 +276,12 @@ static jlong Font_getAxisInfo(CRITICAL_JNI_PARAMS_COMMA jlong fontPtr, jint inde
     return (static_cast<uint64_t>(var.axisTag) << 32) | static_cast<uint64_t>(floatBinary);
 }
 
+// Critical Native
+static jint Font_getSourceId(CRITICAL_JNI_PARAMS_COMMA jlong fontPtr) {
+    FontWrapper* font = reinterpret_cast<FontWrapper*>(fontPtr);
+    return font->font->typeface()->GetSourceId();
+}
+
 // Fast Native
 static jlong FontFileUtil_getFontRevision(JNIEnv* env, jobject, jobject buffer, jint index) {
     NPE_CHECK_RETURN_ZERO(env, buffer);
@@ -369,6 +372,7 @@ static const JNINativeMethod gFontMethods[] = {
         {"nGetIndex", "(J)I", (void*)Font_getIndex},
         {"nGetAxisCount", "(J)I", (void*)Font_getAxisCount},
         {"nGetAxisInfo", "(JI)J", (void*)Font_getAxisInfo},
+        {"nGetSourceId", "(J)I", (void*)Font_getSourceId},
 };
 
 static const JNINativeMethod gFontFileUtilMethods[] = {
@@ -409,10 +413,15 @@ std::shared_ptr<minikin::MinikinFont> createMinikinFontSkia(
     if (face == nullptr) {
         return nullptr;
     }
-    return std::make_shared<MinikinFontSkia>(std::move(face), fontPtr, fontSize,
+    return std::make_shared<MinikinFontSkia>(std::move(face), getNewSourceId(), fontPtr, fontSize,
                                              fontPath, ttcIndex, axes);
 }
 
+int getNewSourceId() {
+    static std::atomic<int> sSourceId = {0};
+    return sSourceId++;
+}
+
 }  // namespace fonts
 
 }  // namespace android
diff --git a/libs/hwui/jni/fonts/Font.h b/libs/hwui/jni/fonts/Font.h
index b5d20bf8cc3c..4bf60ee85657 100644
--- a/libs/hwui/jni/fonts/Font.h
+++ b/libs/hwui/jni/fonts/Font.h
@@ -33,6 +33,8 @@ std::shared_ptr<minikin::MinikinFont> createMinikinFontSkia(
         sk_sp<SkData>&& data, std::string_view fontPath, const void *fontPtr, size_t fontSize,
         int ttcIndex, const std::vector<minikin::FontVariation>& axes);
 
+int getNewSourceId();
+
 } // namespace fonts
 
 } // namespace android
diff --git a/libs/hwui/tests/unit/TypefaceTests.cpp b/libs/hwui/tests/unit/TypefaceTests.cpp
index 5d2aa2ff83c9..ab23448ab93f 100644
--- a/libs/hwui/tests/unit/TypefaceTests.cpp
+++ b/libs/hwui/tests/unit/TypefaceTests.cpp
@@ -57,7 +57,7 @@ std::shared_ptr<minikin::FontFamily> buildFamily(const char* fileName) {
     sk_sp<SkTypeface> typeface(fm->makeFromStream(std::move(fontData)));
     LOG_ALWAYS_FATAL_IF(typeface == nullptr, "Failed to make typeface from %s", fileName);
     std::shared_ptr<minikin::MinikinFont> font =
-            std::make_shared<MinikinFontSkia>(std::move(typeface), data, st.st_size, fileName, 0,
+            std::make_shared<MinikinFontSkia>(std::move(typeface), 0, data, st.st_size, fileName, 0,
                                               std::vector<minikin::FontVariation>());
     std::vector<std::shared_ptr<minikin::Font>> fonts;
     fonts.push_back(minikin::Font::Builder(font).build());