fwb: Rework Typeface handling to only refresh theme when necessary

Myself5 · Myself5 · commit 224793bdc733 · 2023-07-10T23:47:43.000+02:00
Set font at boot completed once and only when changed afterwards, don't
hardcode true to always refresh theme on Zygote start.

This improve app startup times due to not having to regenerate default Typeface
and solves app starting issues caused by Typeface changes running OOM.

Also rework the way we send the information to Zygote to also allow clearing
the USAP pool on change.

Further, adjust the code for Android 12/13 and it's changes in the Font engine.
Disable ENABLE_LAZY_TYPEFACE_INITIALIZATION. With out implementation Typeface is initialized at any point anyways, no need to lazy load.

Change-Id: I80a92e4fac4a134f48919b7e8a9bcceb528ebc72
diff --git a/core/java/android/os/Process.java b/core/java/android/os/Process.java
@@ -730,14 +730,13 @@ public static ProcessStartResult start(@NonNull final String processClass,
                                                    whitelistedDataInfoMap,
                                            boolean bindMountAppsData,
                                            boolean bindMountAppStorageDirs,
-                                            boolean refreshTheme,
                                            @Nullable String[] zygoteArgs) {
         return ZYGOTE_PROCESS.start(processClass, niceName, uid, gid, gids,
                     runtimeFlags, mountExternal, targetSdkVersion, seInfo,
                     abi, instructionSet, appDataDir, invokeWith, packageName,
                     zygotePolicyFlags, isTopApp, disabledCompatChanges,
                     pkgDataInfoMap, whitelistedDataInfoMap, bindMountAppsData,
-                    bindMountAppStorageDirs, refreshTheme, zygoteArgs);
+                    bindMountAppStorageDirs, zygoteArgs);
     }
 
     /** @hide */
@@ -754,7 +753,6 @@ public static ProcessStartResult startWebView(@NonNull final String processClass
                                                   @Nullable String invokeWith,
                                                   @Nullable String packageName,
                                                   @Nullable long[] disabledCompatChanges,
-                                                  boolean refreshTheme,
                                                   @Nullable String[] zygoteArgs) {
         // Webview zygote can't access app private data files, so doesn't need to know its data
         // info.
@@ -763,7 +761,7 @@ public static ProcessStartResult startWebView(@NonNull final String processClass
                     abi, instructionSet, appDataDir, invokeWith, packageName,
                     /*zygotePolicyFlags=*/ ZYGOTE_POLICY_FLAG_EMPTY, /*isTopApp=*/ false,
                 disabledCompatChanges, /* pkgDataInfoMap */ null,
-                /* whitelistedDataInfoMap */ null, false, false, refreshTheme, zygoteArgs);
+                /* whitelistedDataInfoMap */ null, false, false, zygoteArgs);
     }
 
     /**
diff --git a/core/java/android/os/ZygoteProcess.java b/core/java/android/os/ZygoteProcess.java
@@ -362,7 +362,6 @@ public final Process.ProcessStartResult start(@NonNull final String processClass
                                                           allowlistedDataInfoList,
                                                   boolean bindMountAppsData,
                                                   boolean bindMountAppStorageDirs,
-                                                  boolean refreshTheme,
                                                   @Nullable String[] zygoteArgs) {
         // TODO (chriswailes): Is there a better place to check this value?
         if (fetchUsapPoolEnabledPropWithMinInterval()) {
@@ -375,7 +374,7 @@ public final Process.ProcessStartResult start(@NonNull final String processClass
                     abi, instructionSet, appDataDir, invokeWith, /*startChildZygote=*/ false,
                     packageName, zygotePolicyFlags, isTopApp, disabledCompatChanges,
                     pkgDataInfoMap, allowlistedDataInfoList, bindMountAppsData,
-                    bindMountAppStorageDirs, refreshTheme, zygoteArgs);
+                    bindMountAppStorageDirs, zygoteArgs);
         } catch (ZygoteStartFailedEx ex) {
             Log.e(LOG_TAG,
                     "Starting VM process through Zygote failed");
@@ -566,7 +565,8 @@ private static boolean policySpecifiesUsapPoolLaunch(int zygotePolicyFlags) {
         "--set-api-denylist-exemptions",
         "--hidden-api-log-sampling-rate",
         "--hidden-api-statslog-sampling-rate",
-        "--invoke-with"
+        "--invoke-with",
+        "--refresh-typeface"
     };
 
     /**
@@ -646,7 +646,6 @@ private Process.ProcessStartResult startViaZygote(@NonNull final String processC
                                                               allowlistedDataInfoList,
                                                       boolean bindMountAppsData,
                                                       boolean bindMountAppStorageDirs,
-                                                      boolean refreshTheme,
                                                       @Nullable String[] extraArgs)
                                                       throws ZygoteStartFailedEx {
         ArrayList<String> argsForZygote = new ArrayList<>();
@@ -657,9 +656,6 @@ private Process.ProcessStartResult startViaZygote(@NonNull final String processC
         argsForZygote.add("--setuid=" + uid);
         argsForZygote.add("--setgid=" + gid);
         argsForZygote.add("--runtime-flags=" + runtimeFlags);
-        if (refreshTheme) {
-            argsForZygote.add("--refresh_theme");
-        }
         if (mountExternal == Zygote.MOUNT_EXTERNAL_DEFAULT) {
             argsForZygote.add("--mount-external-default");
         } else if (mountExternal == Zygote.MOUNT_EXTERNAL_INSTALLER) {
@@ -968,6 +964,13 @@ public void setHiddenApiAccessStatslogSampleRate(int rate) {
         }
     }
 
+    public void refreshTypeface() {
+        synchronized (mLock) {
+            maybeRefreshTypeface(primaryZygoteState);
+            maybeRefreshTypeface(secondaryZygoteState);
+        }
+    }
+
     @GuardedBy("mLock")
     private boolean maybeSetApiDenylistExemptions(ZygoteState state, boolean sendIfEmpty) {
         if (state == null || state.isClosed()) {
@@ -1042,6 +1045,27 @@ private void maybeSetHiddenApiAccessStatslogSampleRate(ZygoteState state) {
         }
     }
 
+    @GuardedBy("mLock")
+    private void maybeRefreshTypeface(ZygoteState state) {
+        if (state == null || state.isClosed()) {
+            return;
+        }
+
+        try {
+            state.mZygoteOutputWriter.write(Integer.toString(1));
+            state.mZygoteOutputWriter.newLine();
+            state.mZygoteOutputWriter.write("--refresh-typeface");
+            state.mZygoteOutputWriter.newLine();
+            state.mZygoteOutputWriter.flush();
+            int status = state.mZygoteInputStream.readInt();
+            if (status != 0) {
+                Slog.e(LOG_TAG, "Failed to refresh USAP Pool for Typeface change status:" + status);
+            }
+        } catch (IOException ioe) {
+            Slog.e(LOG_TAG, "Failed to refresh USAP Pool for Typeface change", ioe);
+        }
+    }
+
     /**
      * Creates a ZygoteState for the primary zygote if it doesn't exist or has been disconnected.
      */
@@ -1053,6 +1077,7 @@ private void attemptConnectionToPrimaryZygote() throws IOException {
 
             maybeSetApiDenylistExemptions(primaryZygoteState, false);
             maybeSetHiddenApiAccessLogSampleRate(primaryZygoteState);
+            maybeRefreshTypeface(primaryZygoteState);
         }
     }
 
@@ -1068,6 +1093,7 @@ private void attemptConnectionToSecondaryZygote() throws IOException {
 
             maybeSetApiDenylistExemptions(secondaryZygoteState, false);
             maybeSetHiddenApiAccessLogSampleRate(secondaryZygoteState);
+            maybeRefreshTypeface(secondaryZygoteState);
         }
     }
 
@@ -1324,7 +1350,7 @@ public ChildZygoteProcess startChildZygote(final String processClass,
                     ZYGOTE_POLICY_FLAG_SYSTEM_PROCESS /* zygotePolicyFlags */, false /* isTopApp */,
                     null /* disabledCompatChanges */, null /* pkgDataInfoMap */,
                     null /* allowlistedDataInfoList */, true /* bindMountAppsData*/,
-                    /* bindMountAppStorageDirs */ false, true /*refreshTheme*/, extraArgs);
+                    /* bindMountAppStorageDirs */ false, extraArgs);
         } catch (ZygoteStartFailedEx ex) {
             throw new RuntimeException("Starting child-zygote through Zygote failed", ex);
         }
diff --git a/core/java/com/android/internal/os/Zygote.java b/core/java/com/android/internal/os/Zygote.java
@@ -924,6 +924,9 @@ private static void validateUsapCommand(ZygoteArguments args) {
         } else if (args.mHiddenApiAccessStatslogSampleRate != -1) {
             throw new IllegalArgumentException(
                     USAP_ERROR_PREFIX + "--hidden-api-statslog-sampling-rate=");
+        } else if (args.refreshTypeface) {
+            throw new IllegalArgumentException(
+                    USAP_ERROR_PREFIX + "--refresh-typeface");
         } else if (args.mInvokeWith != null) {
             throw new IllegalArgumentException(USAP_ERROR_PREFIX + "--invoke-with");
         } else if (args.mPermittedCapabilities != 0 || args.mEffectiveCapabilities != 0) {
diff --git a/core/java/com/android/internal/os/ZygoteArguments.java b/core/java/com/android/internal/os/ZygoteArguments.java
@@ -119,8 +119,8 @@ class ZygoteArguments {
     /** from --package-name */
     String mPackageName;
 
-    /** from --refresh-theme */
-    boolean refreshTheme;
+    /** from --refresh-typeface */
+    boolean refreshTypeface;
 
     /**
      * Any args after and including the first non-option arg (or after a '--')
@@ -484,8 +484,9 @@ private void parseArgs(ZygoteCommandBuffer args, int argCount)
                 mBindMountAppStorageDirs = true;
             } else if (arg.equals(Zygote.BIND_MOUNT_APP_DATA_DIRS)) {
                 mBindMountAppDataDirs = true;
-            } else if (arg.equals("--refresh_theme")) {
-                refreshTheme = true;
+            } else if (arg.equals("--refresh-typeface")) {
+                refreshTypeface = true;
+                expectRuntimeArgs = false;
             } else {
                 unprocessedArg = arg;
                 break;
diff --git a/core/java/com/android/internal/os/ZygoteConnection.java b/core/java/com/android/internal/os/ZygoteConnection.java
@@ -187,6 +187,10 @@ Runnable processCommand(ZygoteServer zygoteServer, boolean multipleOK) {
                     return null;
                 }
 
+                if (parsedArgs.refreshTypeface) {
+                    return handleRefreshTypeface(zygoteServer);
+                }
+
                 if (parsedArgs.mPermittedCapabilities != 0
                         || parsedArgs.mEffectiveCapabilities != 0) {
                     throw new ZygoteSecurityException("Client may not specify capabilities: "
@@ -318,10 +322,6 @@ Runnable processCommand(ZygoteServer zygoteServer, boolean multipleOK) {
                     parsedArgs.mHiddenApiAccessStatslogSampleRate);
         }
 
-        if (parsedArgs.refreshTheme) {
-            Typeface.recreateDefaults();
-        }
-
         throw new AssertionError("Shouldn't get here");
     }
 
@@ -425,6 +425,11 @@ private Runnable handleApiDenylistExemptions(ZygoteServer zygoteServer, String[]
                 () -> ZygoteInit.setApiDenylistExemptions(exemptions));
     }
 
+    private Runnable handleRefreshTypeface(ZygoteServer zygoteServer) {
+        return stateChangeWithUsapPoolReset(zygoteServer,
+                () -> Typeface.recreateDefaults());
+    }
+
     private Runnable handleUsapPoolStatusChange(ZygoteServer zygoteServer, boolean newStatus) {
         try {
             Runnable fpResult = zygoteServer.setUsapPoolStatus(newStatus, mSocket);
diff --git a/graphics/java/android/graphics/FontListParser.java b/graphics/java/android/graphics/FontListParser.java
@@ -31,6 +31,7 @@
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
 
+import java.io.BufferedInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
@@ -80,7 +81,7 @@ public static FontConfig parse(InputStream in) throws XmlPullParserException, IO
     }
 
     public static FontConfig parse(File configFilename, String fontDir) throws XmlPullParserException, IOException {
-        try (InputStream is = new FileInputStream(configFilename)) {
+        try (InputStream is = new BufferedInputStream(new FileInputStream(configFilename))) {
             XmlPullParser parser = Xml.newPullParser();
             parser.setInput(is, null);
             parser.nextTag();
@@ -270,6 +271,10 @@ private static boolean keepReading(XmlPullParser parser)
             boolean allowNonExistingFile)
             throws XmlPullParserException, IOException {
 
+        if (!fontDir.endsWith("/")) {
+            fontDir = fontDir + "/";
+        }
+
         String indexStr = parser.getAttributeValue(null, ATTR_INDEX);
         int index = indexStr == null ? 0 : Integer.parseInt(indexStr);
         List<FontVariationAxis> axes = new ArrayList<>();
diff --git a/graphics/java/android/graphics/Typeface.java b/graphics/java/android/graphics/Typeface.java
@@ -93,7 +93,7 @@ public class Typeface {
     private static String TAG = "Typeface";
 
     /** @hide */
-    public static final boolean ENABLE_LAZY_TYPEFACE_INITIALIZATION = true;
+    public static final boolean ENABLE_LAZY_TYPEFACE_INITIALIZATION = false;
 
     private static final NativeAllocationRegistry sRegistry =
             NativeAllocationRegistry.createMalloced(
@@ -184,6 +184,11 @@ public class Typeface {
 
     static final String SANS_SERIF_FAMILY_NAME = "sans-serif";
 
+    static final String SYSTEM_FONT_CONFIG_LOCATION = "/system/etc/";
+    static final String THEME_FONT_CONFIG_LOCATION = "/data/system/theme/fonts/";
+    static final String THEME_FONT_DIR_LOCATION = "/data/system/theme/fonts/";
+    static final String SYSTEM_FONT_DIR_LOCATION = "/system/fonts/";
+
     /**
      * Returns the shared memory that used for creating Typefaces.
      *
@@ -1177,8 +1182,17 @@ private static Typeface createFromFamiliesWithDefault(android.graphics.FontFamil
         for (int i = 0; i < families.length; i++) {
             ptrArray[i] = families[i].mNativePtr;
         }
+
+        long ni;
+
+        // getSystemDefaultTypeface can return null, so manually set the native_instance then
+        if (fallbackTypeface == null) {
+            ni = 0;
+        } else {
+            ni = fallbackTypeface.native_instance;
+        }
         return new Typeface(nativeCreateFromArray(
-                ptrArray, fallbackTypeface.native_instance, weight, italic));
+                ptrArray, ni, weight, italic));
     }
 
     // don't allow clients to call this directly
@@ -1231,7 +1245,7 @@ private static android.graphics.FontFamily makeFamilyFromParsed(FontConfig.FontF
         android.graphics.FontFamily fontFamily =
                 new android.graphics.FontFamily(family.getLanguages(), family.getVariant());
         for (FontConfig.Font font : family.getFonts()) {
-            String fullPathName = font.getFontFamilyName();
+            String fullPathName = font.getFile().getAbsolutePath();
             ByteBuffer fontBuffer = bufferForPath.get(fullPathName);
             if (fontBuffer == null) {
                 try (FileInputStream file = new FileInputStream(fullPathName)) {
@@ -1325,30 +1339,27 @@ private static void addMissingFontAliases(FontConfig src,
     }
     private static void init() {
         // Load font config and initialize Minikin state
-        File systemFontConfigLocation = getSystemFontConfigLocation();
-        File themeFontConfigLocation = getThemeFontConfigLocation();
-        File systemConfigFile = new File(systemFontConfigLocation, FONTS_CONFIG);
-        File themeConfigFile = new File(themeFontConfigLocation, FONTS_CONFIG);
+        File systemConfigFile = new File(SYSTEM_FONT_CONFIG_LOCATION, FONTS_CONFIG);
+        File themeConfigFile = new File(THEME_FONT_CONFIG_LOCATION, FONTS_CONFIG);
         File configFile = null;
-        File fontDir;
+        String fontDir;
         if (themeConfigFile.exists()) {
             // /data/system/theme/fonts/ exists so use it and copy default fonts
             configFile = themeConfigFile;
-            fontDir = getThemeFontDirLocation();
+            fontDir = THEME_FONT_DIR_LOCATION;
         } else {
             configFile = systemConfigFile;
-            fontDir = getSystemFontDirLocation();
+            fontDir = SYSTEM_FONT_DIR_LOCATION;
         }
         try {
             FontConfig fontConfig = FontListParser.parse(configFile,
-                    fontDir.getAbsolutePath());
+                    fontDir);
             FontConfig systemFontConfig = null;
             // If the fonts are coming from a theme, we will need to make sure that we include
             // any font families from the system fonts that the theme did not include.
             // NOTE: All the system font families without names ALWAYS get added.
             if (configFile == themeConfigFile) {
-                systemFontConfig = FontListParser.parse(systemConfigFile,
-                        getSystemFontDirLocation().getAbsolutePath());
+                systemFontConfig = FontListParser.parse(systemConfigFile, SYSTEM_FONT_DIR_LOCATION);
                 addFallbackFontsForFamilyName(systemFontConfig, fontConfig, SANS_SERIF_FAMILY_NAME);
                 addMissingFontFamilies(systemFontConfig, fontConfig);
                 addMissingFontAliases(systemFontConfig, fontConfig);
@@ -1378,7 +1389,7 @@ private static void init() {
                     if (i == 0) {
                         // The first entry is the default typeface; no sense in
                         // duplicating the corresponding FontFamily.
-                        typeface = sDefaultTypeface;
+                        typeface = getDefault();
                     } else {
                         android.graphics.FontFamily fontFamily
                                 = makeFamilyFromParsed(f, bufferForPath);
@@ -1416,20 +1427,13 @@ private static void init() {
     /** @hide */
     public static void recreateDefaults() {
         sDynamicTypefaceCache.evictAll();
-        sSystemFontMap.clear();
         sStyledTypefaceCache.clear();
         init();
-        DEFAULT_BOLD_INTERNAL = create((String) null, Typeface.BOLD);
-        SANS_SERIF_INTERNAL = create("sans-serif", 0);
-        SERIF_INTERNAL = create("serif", 0);
-        MONOSPACE_INTERNAL = create("monospace", 0);
         DEFAULT.native_instance = DEFAULT_INTERNAL.native_instance;
         DEFAULT_BOLD.native_instance = DEFAULT_BOLD_INTERNAL.native_instance;
         SANS_SERIF.native_instance = SANS_SERIF_INTERNAL.native_instance;
         SERIF.native_instance = SERIF_INTERNAL.native_instance;
         MONOSPACE.native_instance = MONOSPACE_INTERNAL.native_instance;
-        sDefaults[2] = create((String) null, Typeface.ITALIC);
-        sDefaults[3] = create((String) null, Typeface.BOLD_ITALIC);
     }
 
     /**
@@ -1674,22 +1678,6 @@ public static void loadPreinstalledSystemFontMap() {
         }
     }
 
-    private static File getSystemFontConfigLocation() {
-        return new File("/system/etc/");
-    }
-
-    private static File getSystemFontDirLocation() {
-        return new File("/system/fonts/");
-    }
-
-    private static File getThemeFontConfigLocation() {
-        return new File("/data/system/theme/fonts/");
-    }
-
-    private static File getThemeFontDirLocation() {
-        return new File("/data/system/theme/fonts/");
-    }
-
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
diff --git a/services/core/java/com/android/server/FontService.java b/services/core/java/com/android/server/FontService.java
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java
