Merge "Read font attributes from buffer." into sc-dev am: 7adad678e0 am: f1e4d0e3e3

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/14924347

Change-Id: I6a97fe0bedfa64b7ffd041220ba0a14068d57806

libs/hwui/jni/Typeface.cpp

@@ -204,10 +204,9 @@ static sk_sp<SkData> makeSkDataCached(const std::string& path, bool hasVerity) {
     return entry;
 }
 
-static std::function<std::shared_ptr<minikin::MinikinFont>()> readMinikinFontSkia(
+static std::shared_ptr<minikin::MinikinFont> loadMinikinFontSkia(minikin::BufferReader);
-        minikin::BufferReader* reader) {
+
-    const void* buffer = reader->data();
+static minikin::Font::TypefaceLoader* readMinikinFontSkia(minikin::BufferReader* reader) {
-    size_t pos = reader->pos();
     // Advance reader's position.
     reader->skipString(); // fontPath
     reader->skip<int>(); // fontIndex
@@ -217,60 +216,63 @@ static std::function<std::shared_ptr<minikin::MinikinFont>()> readMinikinFontSki
         reader->skip<uint32_t>(); // expectedFontRevision
         reader->skipString(); // expectedPostScriptName
     }
-    return [buffer, pos]() -> std::shared_ptr<minikin::MinikinFont> {
+    return &loadMinikinFontSkia;
-        minikin::BufferReader fontReader(buffer, pos);
+}
-        std::string_view fontPath = fontReader.readString();
+
-        std::string path(fontPath.data(), fontPath.size());
+static std::shared_ptr<minikin::MinikinFont> loadMinikinFontSkia(minikin::BufferReader reader) {
-        ATRACE_FORMAT("Loading font %s", path.c_str());
+    std::string_view fontPath = reader.readString();
-        int fontIndex = fontReader.read<int>();
+    std::string path(fontPath.data(), fontPath.size());
-        const minikin::FontVariation* axesPtr;
+    ATRACE_FORMAT("Loading font %s", path.c_str());
-        uint32_t axesCount;
+    int fontIndex = reader.read<int>();
-        std::tie(axesPtr, axesCount) = fontReader.readArray<minikin::FontVariation>();
+    const minikin::FontVariation* axesPtr;
-        bool hasVerity = static_cast<bool>(fontReader.read<int8_t>());
+    uint32_t axesCount;
-        uint32_t expectedFontRevision;
+    std::tie(axesPtr, axesCount) = reader.readArray<minikin::FontVariation>();
-        std::string_view expectedPostScriptName;
+    bool hasVerity = static_cast<bool>(reader.read<int8_t>());
-        if (hasVerity) {
+    uint32_t expectedFontRevision;
-            expectedFontRevision = fontReader.read<uint32_t>();
+    std::string_view expectedPostScriptName;
-            expectedPostScriptName = fontReader.readString();
+    if (hasVerity) {
-        }
+        expectedFontRevision = reader.read<uint32_t>();
-        sk_sp<SkData> data = makeSkDataCached(path, hasVerity);
+        expectedPostScriptName = reader.readString();
-        if (data.get() == nullptr) {
+    }
-            // This may happen if:
+    sk_sp<SkData> data = makeSkDataCached(path, hasVerity);
-            // 1. When the process failed to open the file (e.g. invalid path or permission).
+    if (data.get() == nullptr) {
-            // 2. When the process failed to map the file (e.g. hitting max_map_count limit).
+        // This may happen if:
-            ALOGE("Failed to make SkData from file name: %s", path.c_str());
+        // 1. When the process failed to open the file (e.g. invalid path or permission).
+        // 2. When the process failed to map the file (e.g. hitting max_map_count limit).
+        ALOGE("Failed to make SkData from file name: %s", path.c_str());
+        return nullptr;
+    }
+    const void* fontPtr = data->data();
+    size_t fontSize = data->size();
+    if (hasVerity) {
+        // Verify font metadata if verity is enabled.
+        minikin::FontFileParser parser(fontPtr, fontSize, fontIndex);
+        std::optional<uint32_t> revision = parser.getFontRevision();
+        if (!revision.has_value() || revision.value() != expectedFontRevision) {
+            LOG_ALWAYS_FATAL("Wrong font revision: %s", path.c_str());
             return nullptr;
         }
-        const void* fontPtr = data->data();
+        std::optional<std::string> psName = parser.getPostScriptName();
-        size_t fontSize = data->size();
+        if (!psName.has_value() || psName.value() != expectedPostScriptName) {
-        if (hasVerity) {
+            LOG_ALWAYS_FATAL("Wrong PostScript name: %s", path.c_str());
-            // Verify font metadata if verity is enabled.
-            minikin::FontFileParser parser(fontPtr, fontSize, fontIndex);
-            std::optional<uint32_t> revision = parser.getFontRevision();
-            if (!revision.has_value() || revision.value() != expectedFontRevision) {
-              LOG_ALWAYS_FATAL("Wrong font revision: %s", path.c_str());
-              return nullptr;
-            }
-            std::optional<std::string> psName = parser.getPostScriptName();
-            if (!psName.has_value() || psName.value() != expectedPostScriptName) {
-              LOG_ALWAYS_FATAL("Wrong PostScript name: %s", path.c_str());
-              return nullptr;
-            }
-        }
-        std::vector<minikin::FontVariation> axes(axesPtr, axesPtr + axesCount);
-        std::shared_ptr<minikin::MinikinFont> minikinFont =
-                fonts::createMinikinFontSkia(std::move(data), fontPath, fontPtr, fontSize,
-                                             fontIndex, axes);
-        if (minikinFont == nullptr) {
-            ALOGE("Failed to create MinikinFontSkia: %s", path.c_str());
             return nullptr;
         }
-        return minikinFont;
+    }
-    };
+    std::vector<minikin::FontVariation> axes(axesPtr, axesPtr + axesCount);
+    std::shared_ptr<minikin::MinikinFont> minikinFont = fonts::createMinikinFontSkia(
+            std::move(data), fontPath, fontPtr, fontSize, fontIndex, axes);
+    if (minikinFont == nullptr) {
+        ALOGE("Failed to create MinikinFontSkia: %s", path.c_str());
+        return nullptr;
+    }
+    return minikinFont;
 }
 
 static void writeMinikinFontSkia(minikin::BufferWriter* writer,
         const minikin::MinikinFont* typeface) {
+    // When you change the format of font metadata, please update code to parse
+    // typefaceMetadataReader() in
+    // frameworks/base/libs/hwui/jni/fonts/Font.cpp too.
     const std::string& path = typeface->GetFontPath();
     writer->writeString(path);
     writer->write<int>(typeface->GetFontIndex());

libs/hwui/jni/fonts/Font.cpp

@@ -192,7 +192,7 @@ static jfloat Font_getFontMetrics(JNIEnv* env, jobject, jlong fontHandle, jlong
 // Critical Native
 static jlong Font_getMinikinFontPtr(CRITICAL_JNI_PARAMS_COMMA jlong fontPtr) {
     FontWrapper* font = reinterpret_cast<FontWrapper*>(fontPtr);
-    return reinterpret_cast<jlong>(font->font->typeface().get());
+    return reinterpret_cast<jlong>(font->font.get());
 }
 
 // Critical Native
@@ -224,12 +224,21 @@ static jlong Font_getReleaseNativeFontFunc(CRITICAL_JNI_PARAMS) {
 // Fast Native
 static jstring Font_getFontPath(JNIEnv* env, jobject, jlong fontPtr) {
     FontWrapper* font = reinterpret_cast<FontWrapper*>(fontPtr);
-    const std::shared_ptr<minikin::MinikinFont>& minikinFont = font->font->typeface();
+    minikin::BufferReader reader = font->font->typefaceMetadataReader();
-    const std::string& path = minikinFont->GetFontPath();
+    if (reader.data() != nullptr) {
-    if (path.empty()) {
+        std::string path = std::string(reader.readString());
-        return nullptr;
+        if (path.empty()) {
+            return nullptr;
+        }
+        return env->NewStringUTF(path.c_str());
+    } else {
+        const std::shared_ptr<minikin::MinikinFont>& minikinFont = font->font->typeface();
+        const std::string& path = minikinFont->GetFontPath();
+        if (path.empty()) {
+            return nullptr;
+        }
+        return env->NewStringUTF(path.c_str());
     }
-    return env->NewStringUTF(path.c_str());
 }
 
 // Fast Native
@@ -257,22 +266,43 @@ static jint Font_getPackedStyle(CRITICAL_JNI_PARAMS_COMMA jlong fontPtr) {
 // Critical Native
 static jint Font_getIndex(CRITICAL_JNI_PARAMS_COMMA jlong fontPtr) {
     FontWrapper* font = reinterpret_cast<FontWrapper*>(fontPtr);
-    const std::shared_ptr<minikin::MinikinFont>& minikinFont = font->font->typeface();
+    minikin::BufferReader reader = font->font->typefaceMetadataReader();
-    return minikinFont->GetFontIndex();
+    if (reader.data() != nullptr) {
+        reader.skipString();  // fontPath
+        return reader.read<int>();
+    } else {
+        const std::shared_ptr<minikin::MinikinFont>& minikinFont = font->font->typeface();
+        return minikinFont->GetFontIndex();
+    }
 }
 
 // Critical Native
 static jint Font_getAxisCount(CRITICAL_JNI_PARAMS_COMMA jlong fontPtr) {
     FontWrapper* font = reinterpret_cast<FontWrapper*>(fontPtr);
-    const std::shared_ptr<minikin::MinikinFont>& minikinFont = font->font->typeface();
+    minikin::BufferReader reader = font->font->typefaceMetadataReader();
-    return minikinFont->GetAxes().size();
+    if (reader.data() != nullptr) {
+        reader.skipString();  // fontPath
+        reader.skip<int>();   // fontIndex
+        return reader.readArray<minikin::FontVariation>().second;
+    } else {
+        const std::shared_ptr<minikin::MinikinFont>& minikinFont = font->font->typeface();
+        return minikinFont->GetAxes().size();
+    }
 }
 
 // Critical Native
 static jlong Font_getAxisInfo(CRITICAL_JNI_PARAMS_COMMA jlong fontPtr, jint index) {
     FontWrapper* font = reinterpret_cast<FontWrapper*>(fontPtr);
-    const std::shared_ptr<minikin::MinikinFont>& minikinFont = font->font->typeface();
+    minikin::BufferReader reader = font->font->typefaceMetadataReader();
-    minikin::FontVariation var = minikinFont->GetAxes().at(index);
+    minikin::FontVariation var;
+    if (reader.data() != nullptr) {
+        reader.skipString();  // fontPath
+        reader.skip<int>();   // fontIndex
+        var = reader.readArray<minikin::FontVariation>().first[index];
+    } else {
+        const std::shared_ptr<minikin::MinikinFont>& minikinFont = font->font->typeface();
+        var = minikinFont->GetAxes().at(index);
+    }
     uint32_t floatBinary = *reinterpret_cast<const uint32_t*>(&var.value);
     return (static_cast<uint64_t>(var.axisTag) << 32) | static_cast<uint64_t>(floatBinary);
 }

tests/UpdatableSystemFontTest/Android.bp

@@ -27,6 +27,7 @@ android_test {
     libs: ["android.test.runner"],
     static_libs: [
         "androidx.test.ext.junit",
+        "androidx.test.uiautomator_uiautomator",
         "compatibility-device-util-axt",
         "platform-test-annotations",
         "truth-prebuilt",

tests/UpdatableSystemFontTest/EmojiRenderingTestApp/AndroidManifest.xml

@@ -19,5 +19,6 @@
     package="com.android.emojirenderingtestapp">
     <application>
         <activity android:name=".EmojiRenderingTestActivity"/>
+        <activity android:name=".GetAvailableFontsTestActivity"/>
     </application>
 </manifest>

tests/UpdatableSystemFontTest/EmojiRenderingTestApp/src/com/android/emojirenderingtestapp/GetAvailableFontsTestActivity.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.emojirenderingtestapp;
+
+import static android.view.ViewGroup.LayoutParams.MATCH_PARENT;
+import static android.view.ViewGroup.LayoutParams.WRAP_CONTENT;
+
+import android.app.Activity;
+import android.graphics.fonts.Font;
+import android.graphics.fonts.SystemFonts;
+import android.os.Bundle;
+import android.widget.LinearLayout;
+import android.widget.TextView;
+
+public class GetAvailableFontsTestActivity extends Activity {
+
+    @Override
+    protected void onCreate(Bundle savedInstanceState) {
+        super.onCreate(savedInstanceState);
+        String emojiFontPath = "<Not found>";
+        for (Font font : SystemFonts.getAvailableFonts()) {
+            // Calls font attribute getters to make sure that they don't open font files.
+            font.getAxes();
+            font.getFile();
+            font.getLocaleList();
+            font.getStyle();
+            font.getTtcIndex();
+            if ("NotoColorEmoji.ttf".equals(font.getFile().getName())) {
+                emojiFontPath = font.getFile().getAbsolutePath();
+            }
+        }
+        LinearLayout container = new LinearLayout(this);
+        container.setOrientation(LinearLayout.VERTICAL);
+        TextView textView = new TextView(this);
+        textView.setText(emojiFontPath);
+        container.addView(textView, new LinearLayout.LayoutParams(MATCH_PARENT, WRAP_CONTENT));
+        setContentView(container);
+    }
+}

tests/UpdatableSystemFontTest/src/com/android/updatablesystemfont/UpdatableSystemFontTest.java

@@ -40,6 +40,9 @@ import android.util.Pair;
 import androidx.annotation.Nullable;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import androidx.test.platform.app.InstrumentationRegistry;
+import androidx.test.uiautomator.By;
+import androidx.test.uiautomator.UiDevice;
+import androidx.test.uiautomator.Until;
 
 import com.android.compatibility.common.util.StreamUtil;
 import com.android.compatibility.common.util.SystemUtil;
@@ -101,6 +104,9 @@ public class UpdatableSystemFontTest {
             EMOJI_RENDERING_TEST_APP_ID + "/.EmojiRenderingTestActivity";
     private static final long ACTIVITY_TIMEOUT_MILLIS = SECONDS.toMillis(10);
 
+    private static final String GET_AVAILABLE_FONTS_TEST_ACTIVITY =
+            EMOJI_RENDERING_TEST_APP_ID + "/.GetAvailableFontsTestActivity";
+
     private static final Pattern PATTERN_FONT_FILES = Pattern.compile("\\.(ttf|otf|ttc|otc)$");
     private static final Pattern PATTERN_TMP_FILES = Pattern.compile("^/data/local/tmp/");
     private static final Pattern PATTERN_DATA_FONT_FILES = Pattern.compile("^/data/fonts/files/");
@@ -109,6 +115,7 @@ public class UpdatableSystemFontTest {
 
     private String mKeyId;
     private FontManager mFontManager;
+    private UiDevice mUiDevice;
 
     @Before
     public void setUp() throws Exception {
@@ -120,6 +127,7 @@ public class UpdatableSystemFontTest {
         mKeyId = insertCert(CERT_PATH);
         mFontManager = context.getSystemService(FontManager.class);
         expectCommandToSucceed("cmd font clear");
+        mUiDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation());
     }
 
     @After
@@ -287,6 +295,19 @@ public class UpdatableSystemFontTest {
         assertThat(countMatch(openFiles, patternEmojiVPlus1)).isEqualTo(0);
     }
 
+    @Test
+    public void getAvailableFonts() throws Exception {
+        String fontPath = getFontPath(NOTO_COLOR_EMOJI_POSTSCRIPT_NAME);
+        startActivity(EMOJI_RENDERING_TEST_APP_ID, GET_AVAILABLE_FONTS_TEST_ACTIVITY);
+        // GET_AVAILABLE_FONTS_TEST_ACTIVITY shows the NotoColorEmoji path it got.
+        mUiDevice.wait(
+                Until.findObject(By.pkg(EMOJI_RENDERING_TEST_APP_ID).text(fontPath)),
+                ACTIVITY_TIMEOUT_MILLIS);
+        // The font file should not be opened just by querying the path using
+        // SystemFont.getAvailableFonts().
+        assertThat(isFileOpenedBy(fontPath, EMOJI_RENDERING_TEST_APP_ID)).isFalse();
+    }
+
     private static String insertCert(String certPath) throws Exception {
         Pair<String, String> result;
         try (InputStream is = new FileInputStream(certPath)) {