Create XML parser only once.
This greatly cuts memory usage: 2.5M -> 0.5M (see bug for traces).
Bug: 200995209
Test: atest SystemConfigTest SystemConfigNamedActorTest
Change-Id: I0b76dc2610afaad3e418ef3115c5e54a05ab334e
diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java
index b0cf5dc..d7eef0d 100644
--- a/core/java/com/android/server/SystemConfig.java
+++ b/core/java/com/android/server/SystemConfig.java
@@ -488,12 +488,14 @@
}
private void readAllPermissions() {
+ final XmlPullParser parser = Xml.newPullParser();
+
// Read configuration from system
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getRootDirectory(), "etc", "sysconfig"), ALLOW_ALL);
// Read configuration from the old permissions dir
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getRootDirectory(), "etc", "permissions"), ALLOW_ALL);
// Vendors are only allowed to customize these
@@ -503,18 +505,18 @@
// For backward compatibility
vendorPermissionFlag |= (ALLOW_PERMISSIONS | ALLOW_APP_CONFIGS);
}
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getVendorDirectory(), "etc", "sysconfig"), vendorPermissionFlag);
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getVendorDirectory(), "etc", "permissions"), vendorPermissionFlag);
String vendorSkuProperty = SystemProperties.get(VENDOR_SKU_PROPERTY, "");
if (!vendorSkuProperty.isEmpty()) {
String vendorSkuDir = "sku_" + vendorSkuProperty;
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getVendorDirectory(), "etc", "sysconfig", vendorSkuDir),
vendorPermissionFlag);
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getVendorDirectory(), "etc", "permissions", vendorSkuDir),
vendorPermissionFlag);
}
@@ -522,18 +524,18 @@
// Allow ODM to customize system configs as much as Vendor, because /odm is another
// vendor partition other than /vendor.
int odmPermissionFlag = vendorPermissionFlag;
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getOdmDirectory(), "etc", "sysconfig"), odmPermissionFlag);
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getOdmDirectory(), "etc", "permissions"), odmPermissionFlag);
String skuProperty = SystemProperties.get(SKU_PROPERTY, "");
if (!skuProperty.isEmpty()) {
String skuDir = "sku_" + skuProperty;
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getOdmDirectory(), "etc", "sysconfig", skuDir), odmPermissionFlag);
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getOdmDirectory(), "etc", "permissions", skuDir),
odmPermissionFlag);
}
@@ -541,9 +543,9 @@
// Allow OEM to customize these
int oemPermissionFlag = ALLOW_FEATURES | ALLOW_OEM_PERMISSIONS | ALLOW_ASSOCIATIONS
| ALLOW_VENDOR_APEX;
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getOemDirectory(), "etc", "sysconfig"), oemPermissionFlag);
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getOemDirectory(), "etc", "permissions"), oemPermissionFlag);
// Allow Product to customize these configs
@@ -558,15 +560,15 @@
// DEVICE_INITIAL_SDK_INT for the devices without product interface enforcement.
productPermissionFlag = ALLOW_ALL;
}
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getProductDirectory(), "etc", "sysconfig"), productPermissionFlag);
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getProductDirectory(), "etc", "permissions"), productPermissionFlag);
// Allow /system_ext to customize all system configs
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getSystemExtDirectory(), "etc", "sysconfig"), ALLOW_ALL);
- readPermissions(Environment.buildPath(
+ readPermissions(parser, Environment.buildPath(
Environment.getSystemExtDirectory(), "etc", "permissions"), ALLOW_ALL);
// Skip loading configuration from apex if it is not a system process.
@@ -580,12 +582,13 @@
if (f.isFile() || f.getPath().contains("@")) {
continue;
}
- readPermissions(Environment.buildPath(f, "etc", "permissions"), apexPermissionFlag);
+ readPermissions(parser, Environment.buildPath(f, "etc", "permissions"),
+ apexPermissionFlag);
}
}
@VisibleForTesting
- public void readPermissions(File libraryDir, int permissionFlag) {
+ public void readPermissions(final XmlPullParser parser, File libraryDir, int permissionFlag) {
// Read permissions from given directory.
if (!libraryDir.exists() || !libraryDir.isDirectory()) {
if (permissionFlag == ALLOW_ALL) {
@@ -620,12 +623,12 @@
continue;
}
- readPermissionsFromXml(f, permissionFlag);
+ readPermissionsFromXml(parser, f, permissionFlag);
}
// Read platform permissions last so it will take precedence
if (platformFile != null) {
- readPermissionsFromXml(platformFile, permissionFlag);
+ readPermissionsFromXml(parser, platformFile, permissionFlag);
}
}
@@ -634,8 +637,9 @@
+ permFile + " at " + parser.getPositionDescription());
}
- private void readPermissionsFromXml(File permFile, int permissionFlag) {
- FileReader permReader = null;
+ private void readPermissionsFromXml(final XmlPullParser parser, File permFile,
+ int permissionFlag) {
+ final FileReader permReader;
try {
permReader = new FileReader(permFile);
} catch (FileNotFoundException e) {
@@ -647,7 +651,6 @@
final boolean lowRam = ActivityManager.isLowRamDeviceStatic();
try {
- XmlPullParser parser = Xml.newPullParser();
parser.setInput(permReader);
int type;
diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt
index b7199d4..150822b 100644
--- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt
+++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt
@@ -17,6 +17,7 @@
package com.android.server.systemconfig
import android.content.Context
+import android.util.Xml
import androidx.test.InstrumentationRegistry
import com.android.server.SystemConfig
import com.google.common.truth.Truth.assertThat
@@ -227,6 +228,7 @@
.writeText(this.trimIndent())
private fun assertPermissions() = SystemConfig(false).apply {
- readPermissions(tempFolder.root, 0)
+ val parser = Xml.newPullParser()
+ readPermissions(parser, tempFolder.root, 0)
}. let { assertThat(it.namedActors) }
}
diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java
index 5eb21a5..cc53160 100644
--- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java
+++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java
@@ -25,6 +25,7 @@
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
+import android.util.Xml;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
@@ -36,6 +37,7 @@
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
+import org.xmlpull.v1.XmlPullParser;
import java.io.BufferedWriter;
import java.io.File;
@@ -76,6 +78,11 @@
}
}
+ private void readPermissions(File libraryDir, int permissionFlag) {
+ final XmlPullParser parser = Xml.newPullParser();
+ mSysConfig.readPermissions(parser, libraryDir, permissionFlag);
+ }
+
/**
* Tests that readPermissions works correctly for the tag: install-in-user-type
*/
@@ -134,8 +141,8 @@
// Also, make a third file, but with the name folder1/permFile2.xml, to prove no conflicts.
createTempFile(folder1, "permFile2.xml", contents3);
- mSysConfig.readPermissions(folder1, /* No permission needed anyway */ 0);
- mSysConfig.readPermissions(folder2, /* No permission needed anyway */ 0);
+ readPermissions(folder1, /* No permission needed anyway */ 0);
+ readPermissions(folder2, /* No permission needed anyway */ 0);
Map<String, Set<String>> actualWhite = mSysConfig.getAndClearPackageToUserTypeWhitelist();
Map<String, Set<String>> actualBlack = mSysConfig.getAndClearPackageToUserTypeBlacklist();
@@ -165,7 +172,7 @@
final File folder = createTempSubfolder("folder");
createTempFile(folder, "component-override.xml", contents);
- mSysConfig.readPermissions(folder, /* No permission needed anyway */ 0);
+ readPermissions(folder, /* No permission needed anyway */ 0);
final ArrayMap<String, Boolean> packageOneExpected = new ArrayMap<>();
packageOneExpected.put("com.android.package1.Full", true);
@@ -197,7 +204,7 @@
final File folder = createTempSubfolder("folder");
createTempFile(folder, "staged-installer-whitelist.xml", contents);
- mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0);
+ readPermissions(folder, /* Grant all permission flags */ ~0);
assertThat(mSysConfig.getWhitelistedStagedInstallers())
.containsExactly("com.android.package1");
@@ -215,7 +222,7 @@
final File folder = createTempSubfolder("folder");
createTempFile(folder, "staged-installer-whitelist.xml", contents);
- mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0);
+ readPermissions(folder, /* Grant all permission flags */ ~0);
assertThat(mSysConfig.getWhitelistedStagedInstallers())
.containsExactly("com.android.package1");
@@ -238,7 +245,7 @@
IllegalStateException e = expectThrows(
IllegalStateException.class,
- () -> mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0));
+ () -> readPermissions(folder, /* Grant all permission flags */ ~0));
assertThat(e).hasMessageThat().contains("Multiple modules installers");
}
@@ -257,7 +264,7 @@
final File folder = createTempSubfolder("folder");
createTempFile(folder, "staged-installer-whitelist.xml", contents);
- mSysConfig.readPermissions(folder, /* Grant all but ALLOW_APP_CONFIGS flag */ ~0x08);
+ readPermissions(folder, /* Grant all but ALLOW_APP_CONFIGS flag */ ~0x08);
assertThat(mSysConfig.getWhitelistedStagedInstallers()).isEmpty();
}
@@ -277,7 +284,7 @@
final File folder = createTempSubfolder("folder");
createTempFile(folder, "vendor-apex-allowlist.xml", contents);
- mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0);
+ readPermissions(folder, /* Grant all permission flags */ ~0);
assertThat(mSysConfig.getAllowedVendorApexes())
.containsExactly("com.android.apex1", "com.installer");
@@ -297,7 +304,7 @@
final File folder = createTempSubfolder("folder");
createTempFile(folder, "vendor-apex-allowlist.xml", contents);
- mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0);
+ readPermissions(folder, /* Grant all permission flags */ ~0);
assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty();
}
@@ -317,7 +324,7 @@
final File folder = createTempSubfolder("folder");
createTempFile(folder, "vendor-apex-allowlist.xml", contents);
- mSysConfig.readPermissions(folder, /* Grant all but ALLOW_VENDOR_APEX flag */ ~0x400);
+ readPermissions(folder, /* Grant all but ALLOW_VENDOR_APEX flag */ ~0x400);
assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty();
}
