From cf909694ad9394c68720727bdbf0d2ff0eaa649a Mon Sep 17 00:00:00 2001
From: Chip Fukuhara <cfukuhara@google.com>
Date: Thu, 15 Feb 2024 23:49:51 +0000
Subject: [PATCH] Avoid appcompat logs if old or disabled
For performance savings, we will only log appcompat changes to debug if
it targets the latest sdk version and it is not disabled. These can
still be turned back on with `adb setprop
log.tag.CompatChangeReporter=DEBUG`.
Test: atest PlatformCompatFrameworkTests; atest FrameworksServicesTests:ActivityManagerServiceTest; atest FrameworksServicesTests:ProcessRecordTests; atest FrameworksServicesTests:CompatConfigTest; atest FrameworksServicesTests:PlatformCompatTest
Bug: 323949942
Flag: skip_old_and_disabled_compat_logging
Change-Id: I9efef3af940d7597105ef796774f8cc474194c19
---
AconfigFlags.bp | 8 ++
core/java/android/app/ActivityThread.java | 5 +-
core/java/android/app/AppCompatCallbacks.java | 44 +++++++---
core/java/android/app/IApplicationThread.aidl | 2 +-
.../com/android/internal/compat/Android.bp | 7 ++
.../internal/compat/ChangeReporter.java | 65 +++++++++++---
.../compat/compat_logging_flags.aconfig | 9 ++
core/tests/PlatformCompatFramework/Android.bp | 1 +
.../internal/compat/ChangeReporterTest.java | 86 ++++++++++++++++++-
.../server/am/ActivityManagerService.java | 1 +
.../com/android/server/am/ProcessList.java | 2 +
.../com/android/server/am/ProcessRecord.java | 16 ++++
.../server/am/SettingsToPropertiesMapper.java | 1 +
.../android/server/compat/CompatConfig.java | 78 ++++++++++++++++-
.../android/server/compat/PlatformCompat.java | 41 +++++++--
.../java/com/android/server/SystemServer.java | 2 +-
.../server/am/AsyncProcessStartTest.java | 4 +-
.../server/am/ProcessObserverTest.java | 4 +-
.../server/compat/CompatConfigTest.java | 19 ++++
19 files changed, 357 insertions(+), 38 deletions(-)
create mode 100644 core/java/com/android/internal/compat/Android.bp
create mode 100644 core/java/com/android/internal/compat/compat_logging_flags.aconfig
diff --git a/AconfigFlags.bp b/AconfigFlags.bp
index 48d6392f8b9b..8591a9c4a195 100644
--- a/AconfigFlags.bp
+++ b/AconfigFlags.bp
@@ -77,6 +77,7 @@ aconfig_declarations_group {
"camera_platform_flags_core_java_lib",
"com.android.hardware.input-aconfig-java",
"com.android.input.flags-aconfig-java",
+ "com.android.internal.compat.flags-aconfig-java",
"com.android.internal.foldables.flags-aconfig-java",
"com.android.internal.pm.pkg.component.flags-aconfig-java",
"com.android.media.flags.bettertogether-aconfig-java",
@@ -663,6 +664,13 @@ java_aconfig_library {
defaults: ["framework-minus-apex-aconfig-java-defaults"],
}
+// Platform Compat
+java_aconfig_library {
+ name: "com.android.internal.compat.flags-aconfig-java",
+ aconfig_declarations: "compat_logging_flags",
+ defaults: ["framework-minus-apex-aconfig-java-defaults"],
+}
+
// Multi user
aconfig_declarations {
name: "android.multiuser.flags-aconfig",
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 1d39186de183..cf3b465906fa 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -974,6 +974,7 @@ public final class ActivityThread extends ClientTransactionHandler
ContentCaptureOptions contentCaptureOptions;
long[] disabledCompatChanges;
+ long[] mLoggableCompatChanges;
SharedMemory mSerializedSystemFontMap;
@@ -1283,6 +1284,7 @@ public final class ActivityThread extends ClientTransactionHandler
AutofillOptions autofillOptions,
ContentCaptureOptions contentCaptureOptions,
long[] disabledCompatChanges,
+ long[] loggableCompatChanges,
SharedMemory serializedSystemFontMap,
long startRequestedElapsedTime,
long startRequestedUptime) {
@@ -1337,6 +1339,7 @@ public final class ActivityThread extends ClientTransactionHandler
data.autofillOptions = autofillOptions;
data.contentCaptureOptions = contentCaptureOptions;
data.disabledCompatChanges = disabledCompatChanges;
+ data.mLoggableCompatChanges = loggableCompatChanges;
data.mSerializedSystemFontMap = serializedSystemFontMap;
data.startRequestedElapsedTime = startRequestedElapsedTime;
data.startRequestedUptime = startRequestedUptime;
@@ -7123,7 +7126,7 @@ public final class ActivityThread extends ClientTransactionHandler
Process.setStartTimes(SystemClock.elapsedRealtime(), SystemClock.uptimeMillis(),
data.startRequestedElapsedTime, data.startRequestedUptime);
- AppCompatCallbacks.install(data.disabledCompatChanges);
+ AppCompatCallbacks.install(data.disabledCompatChanges, data.mLoggableCompatChanges);
// Let libcore handle any compat changes after installing the list of compat changes.
AppSpecializationHooks.handleCompatChangesBeforeBindingApplication();
diff --git a/core/java/android/app/AppCompatCallbacks.java b/core/java/android/app/AppCompatCallbacks.java
index 134cef5b6bfa..f2debfcfa6b1 100644
--- a/core/java/android/app/AppCompatCallbacks.java
+++ b/core/java/android/app/AppCompatCallbacks.java
@@ -30,41 +30,59 @@ import java.util.Arrays;
*/
public final class AppCompatCallbacks implements Compatibility.BehaviorChangeDelegate {
private final long[] mDisabledChanges;
+ private final long[] mLoggableChanges;
private final ChangeReporter mChangeReporter;
/**
- * Install this class into the current process.
+ * Install this class into the current process using the disabled and loggable changes lists.
*
* @param disabledChanges Set of compatibility changes that are disabled for this process.
+ * @param loggableChanges Set of compatibility changes that we want to log.
*/
- public static void install(long[] disabledChanges) {
- Compatibility.setBehaviorChangeDelegate(new AppCompatCallbacks(disabledChanges));
+ public static void install(long[] disabledChanges, long[] loggableChanges) {
+ Compatibility.setBehaviorChangeDelegate(
+ new AppCompatCallbacks(disabledChanges, loggableChanges));
}
- private AppCompatCallbacks(long[] disabledChanges) {
+ private AppCompatCallbacks(long[] disabledChanges, long[] loggableChanges) {
mDisabledChanges = Arrays.copyOf(disabledChanges, disabledChanges.length);
+ mLoggableChanges = Arrays.copyOf(loggableChanges, loggableChanges.length);
Arrays.sort(mDisabledChanges);
- mChangeReporter = new ChangeReporter(
- ChangeReporter.SOURCE_APP_PROCESS);
+ Arrays.sort(mLoggableChanges);
+ mChangeReporter = new ChangeReporter(ChangeReporter.SOURCE_APP_PROCESS);
+ }
+
+ /**
+ * Helper to determine if a list contains a changeId.
+ *
+ * @param list to search through
+ * @param changeId for which to search in the list
+ * @return true if the given changeId is found in the provided array.
+ */
+ private boolean changeIdInChangeList(long[] list, long changeId) {
+ return Arrays.binarySearch(list, changeId) >= 0;
}
public void onChangeReported(long changeId) {
- reportChange(changeId, ChangeReporter.STATE_LOGGED);
+ boolean isLoggable = changeIdInChangeList(mLoggableChanges, changeId);
+ reportChange(changeId, ChangeReporter.STATE_LOGGED, isLoggable);
}
public boolean isChangeEnabled(long changeId) {
- if (Arrays.binarySearch(mDisabledChanges, changeId) < 0) {
- // Not present in the disabled array
- reportChange(changeId, ChangeReporter.STATE_ENABLED);
+ boolean isEnabled = !changeIdInChangeList(mDisabledChanges, changeId);
+ boolean isLoggable = changeIdInChangeList(mLoggableChanges, changeId);
+ if (isEnabled) {
+ // Not present in the disabled changeId array
+ reportChange(changeId, ChangeReporter.STATE_ENABLED, isLoggable);
return true;
}
- reportChange(changeId, ChangeReporter.STATE_DISABLED);
+ reportChange(changeId, ChangeReporter.STATE_DISABLED, isLoggable);
return false;
}
- private void reportChange(long changeId, int state) {
+ private void reportChange(long changeId, int state, boolean isLoggable) {
int uid = Process.myUid();
- mChangeReporter.reportChange(uid, changeId, state);
+ mChangeReporter.reportChange(uid, changeId, state, isLoggable);
}
}
diff --git a/core/java/android/app/IApplicationThread.aidl b/core/java/android/app/IApplicationThread.aidl
index a04620cafd75..251e4e8ad834 100644
--- a/core/java/android/app/IApplicationThread.aidl
+++ b/core/java/android/app/IApplicationThread.aidl
@@ -90,7 +90,7 @@ oneway interface IApplicationThread {
in CompatibilityInfo compatInfo, in Map services,
in Bundle coreSettings, in String buildSerial, in AutofillOptions autofillOptions,
in ContentCaptureOptions contentCaptureOptions, in long[] disabledCompatChanges,
- in SharedMemory serializedSystemFontMap,
+ in long[] loggableCompatChanges, in SharedMemory serializedSystemFontMap,
long startRequestedElapsedTime, long startRequestedUptime);
void runIsolatedEntryPoint(in String entryPoint, in String[] entryPointArgs);
void scheduleExit();
diff --git a/core/java/com/android/internal/compat/Android.bp b/core/java/com/android/internal/compat/Android.bp
new file mode 100644
index 000000000000..9ff05a6589b7
--- /dev/null
+++ b/core/java/com/android/internal/compat/Android.bp
@@ -0,0 +1,7 @@
+aconfig_declarations {
+ name: "compat_logging_flags",
+ package: "com.android.internal.compat.flags",
+ srcs: [
+ "compat_logging_flags.aconfig",
+ ],
+}
diff --git a/core/java/com/android/internal/compat/ChangeReporter.java b/core/java/com/android/internal/compat/ChangeReporter.java
index b9d3df678a91..6ff546fd77f8 100644
--- a/core/java/com/android/internal/compat/ChangeReporter.java
+++ b/core/java/com/android/internal/compat/ChangeReporter.java
@@ -24,6 +24,7 @@ import android.util.Slog;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.compat.flags.Flags;
import com.android.internal.util.FrameworkStatsLog;
import java.lang.annotation.Retention;
@@ -40,7 +41,7 @@ import java.util.Set;
* @hide
*/
public final class ChangeReporter {
- private static final String TAG = "CompatibilityChangeReporter";
+ private static final String TAG = "CompatChangeReporter";
private int mSource;
private static final class ChangeReport {
@@ -84,21 +85,36 @@ public final class ChangeReporter {
* Report the change to stats log and to the debug log if the change was not previously
* logged already.
*
- * @param uid affected by the change
- * @param changeId the reported change id
- * @param state of the reported change - enabled/disabled/only logged
+ * @param uid affected by the change
+ * @param changeId the reported change id
+ * @param state of the reported change - enabled/disabled/only logged
+ * @param isLoggableBySdk whether debug logging is allowed for this change based on target
+ * SDK version. This is combined with other logic to determine whether to
+ * actually log. If the sdk version does not matter, should be true.
*/
- public void reportChange(int uid, long changeId, int state) {
+ public void reportChange(int uid, long changeId, int state, boolean isLoggableBySdk) {
if (shouldWriteToStatsLog(uid, changeId, state)) {
FrameworkStatsLog.write(FrameworkStatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid,
changeId, state, mSource);
}
- if (shouldWriteToDebug(uid, changeId, state)) {
+ if (shouldWriteToDebug(uid, changeId, state, isLoggableBySdk)) {
debugLog(uid, changeId, state);
}
markAsReported(uid, new ChangeReport(changeId, state));
}
+ /**
+ * Report the change to stats log and to the debug log if the change was not previously
+ * logged already.
+ *
+ * @param uid affected by the change
+ * @param changeId the reported change id
+ * @param state of the reported change - enabled/disabled/only logged
+ */
+ public void reportChange(int uid, long changeId, int state) {
+ reportChange(uid, changeId, state, true);
+ }
+
/**
* Start logging all the time to logcat.
*/
@@ -130,14 +146,43 @@ public final class ChangeReporter {
/**
* Returns whether the next report should be logged to logcat.
*
- * @param uid affected by the change
- * @param changeId the reported change id
- * @param state of the reported change - enabled/disabled/only logged
+ * @param uid affected by the change
+ * @param changeId the reported change id
+ * @param state of the reported change - enabled/disabled/only logged
+ * @param isLoggableBySdk whether debug logging is allowed for this change based on target
+ * SDK version. This is combined with other logic to determine whether to
+ * actually log. If the sdk version does not matter, should be true.
+ * @return true if the report should be logged
+ */
+ @VisibleForTesting
+ public boolean shouldWriteToDebug(
+ int uid, long changeId, int state, boolean isLoggableBySdk) {
+ // If log all bit is on, always return true.
+ if (mDebugLogAll) return true;
+ // If the change has already been reported, do not write.
+ if (isAlreadyReported(uid, new ChangeReport(changeId, state))) return false;
+
+ // If the flag is turned off or the TAG's logging is forced to debug level with
+ // `adb setprop log.tag.CompatChangeReporter=DEBUG`, write to debug since the above checks
+ // have already passed.
+ boolean skipLoggingFlag = Flags.skipOldAndDisabledCompatLogging();
+ if (!skipLoggingFlag || Log.isLoggable(TAG, Log.DEBUG)) return true;
+
+ // Log if the change is enabled and targets the latest sdk version.
+ return isLoggableBySdk && state != STATE_DISABLED;
+ }
+
+ /**
+ * Returns whether the next report should be logged to logcat.
+ *
+ * @param uid affected by the change
+ * @param changeId the reported change id
+ * @param state of the reported change - enabled/disabled/only logged
* @return true if the report should be logged
*/
@VisibleForTesting
public boolean shouldWriteToDebug(int uid, long changeId, int state) {
- return mDebugLogAll || !isAlreadyReported(uid, new ChangeReport(changeId, state));
+ return shouldWriteToDebug(uid, changeId, state, true);
}
private boolean isAlreadyReported(int uid, ChangeReport report) {
diff --git a/core/java/com/android/internal/compat/compat_logging_flags.aconfig b/core/java/com/android/internal/compat/compat_logging_flags.aconfig
new file mode 100644
index 000000000000..fab3856daca7
--- /dev/null
+++ b/core/java/com/android/internal/compat/compat_logging_flags.aconfig
@@ -0,0 +1,9 @@
+package: "com.android.internal.compat.flags"
+
+flag {
+ name: "skip_old_and_disabled_compat_logging"
+ namespace: "platform_compat"
+ description: "Feature flag for skipping debug logging for changes that do not target the latest sdk or are disabled"
+ bug: "323949942"
+ is_fixed_read_only: true
+}
\ No newline at end of file
diff --git a/core/tests/PlatformCompatFramework/Android.bp b/core/tests/PlatformCompatFramework/Android.bp
index 95e23ad396af..2621d280bd9d 100644
--- a/core/tests/PlatformCompatFramework/Android.bp
+++ b/core/tests/PlatformCompatFramework/Android.bp
@@ -18,6 +18,7 @@ android_test {
static_libs: [
"junit",
"androidx.test.rules",
+ "flag-junit",
],
platform_apis: true,
}
diff --git a/core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java b/core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java
index a052543c6446..12a42f975cd7 100644
--- a/core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java
+++ b/core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java
@@ -19,9 +19,17 @@ package com.android.internal.compat;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import android.platform.test.flag.junit.SetFlagsRule;
+
+import com.android.internal.compat.flags.Flags;
+
+import org.junit.Rule;
import org.junit.Test;
public class ChangeReporterTest {
+
+ @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();
+
@Test
public void testStatsLogOnce() {
ChangeReporter reporter = new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);
@@ -63,7 +71,7 @@ public class ChangeReporterTest {
ChangeReporter reporter = new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);
int myUid = 1022, otherUid = 1023;
long myChangeId = 500L, otherChangeId = 600L;
- int myState = ChangeReporter.STATE_ENABLED, otherState = ChangeReporter.STATE_DISABLED;
+ int myState = ChangeReporter.STATE_ENABLED, otherState = ChangeReporter.STATE_LOGGED;
assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myState));
reporter.reportChange(myUid, myChangeId, myState);
@@ -112,4 +120,80 @@ public class ChangeReporterTest {
reporter.stopDebugLogAll();
assertFalse(reporter.shouldWriteToDebug(myUid, myChangeId, myState));
}
+
+ @Test
+ public void testDebugLogWithFlagOnAndOldSdk() {
+ mSetFlagsRule.enableFlags(Flags.FLAG_SKIP_OLD_AND_DISABLED_COMPAT_LOGGING);
+ ChangeReporter reporter = new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);
+ int myUid = 1022;
+ long myChangeId = 500L;
+ int myEnabledState = ChangeReporter.STATE_ENABLED;
+ int myDisabledState = ChangeReporter.STATE_DISABLED;
+
+ // Report will not log if target sdk is before the previous version.
+ assertFalse(reporter.shouldWriteToDebug(myUid, myChangeId, myEnabledState, false));
+
+ reporter.resetReportedChanges(myUid);
+
+ // Report will be logged if target sdk is the latest version.
+ assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myEnabledState, true));
+
+ reporter.resetReportedChanges(myUid);
+
+ // If the report is disabled, the sdk version shouldn't matter.
+ assertFalse(reporter.shouldWriteToDebug(myUid, myChangeId, myDisabledState, true));
+ }
+
+ @Test
+ public void testDebugLogWithFlagOnAndDisabledChange() {
+ mSetFlagsRule.enableFlags(Flags.FLAG_SKIP_OLD_AND_DISABLED_COMPAT_LOGGING);
+ ChangeReporter reporter = new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);
+ int myUid = 1022;
+ long myChangeId = 500L;
+ int myEnabledState = ChangeReporter.STATE_ENABLED;
+ int myDisabledState = ChangeReporter.STATE_DISABLED;
+
+ // Report will not log if the change is disabled.
+ assertFalse(reporter.shouldWriteToDebug(myUid, myChangeId, myDisabledState, true));
+
+ reporter.resetReportedChanges(myUid);
+
+ // Report will be logged if the change is enabled.
+ assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myEnabledState, true));
+
+ reporter.resetReportedChanges(myUid);
+
+ // If the report is not the latest version, the disabled state doesn't matter.
+ assertFalse(reporter.shouldWriteToDebug(myUid, myChangeId, myEnabledState, false));
+ }
+
+ @Test
+ public void testDebugLogWithFlagOff() {
+ mSetFlagsRule.disableFlags(Flags.FLAG_SKIP_OLD_AND_DISABLED_COMPAT_LOGGING);
+ ChangeReporter reporter = new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);
+ int myUid = 1022;
+ long myChangeId = 500L;
+ int myEnabledState = ChangeReporter.STATE_ENABLED;
+ int myDisabledState = ChangeReporter.STATE_DISABLED;
+
+ // Report will be logged even if the change is not the latest sdk but the flag is off.
+ assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myEnabledState, false));
+
+ reporter.resetReportedChanges(myUid);
+
+ // Report will be logged if the change is enabled and the latest sdk but the flag is off.
+ assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myEnabledState, true));
+
+ reporter.resetReportedChanges(myUid);
+
+ // Report will be logged if the change is disabled and the latest sdk but the flag is
+ // off.
+ assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myDisabledState, true));
+
+ reporter.resetReportedChanges(myUid);
+
+ // Report will be logged if the change is disabled and not the latest sdk but the flag is
+ // off.
+ assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myDisabledState, false));
+ }
}
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 60bfc637225f..5e6ff55f4e94 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -4755,6 +4755,7 @@ public class ActivityManagerService extends IActivityManager.Stub
autofillOptions,
contentCaptureOptions,
app.getDisabledCompatChanges(),
+ app.getLoggableCompatChanges(),
serializedSystemFontMap,
app.getStartElapsedTime(),
app.getStartUptime());
diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java
index 27d6c608cf6c..48a9d6af0df4 100644
--- a/services/core/java/com/android/server/am/ProcessList.java
+++ b/services/core/java/com/android/server/am/ProcessList.java
@@ -2088,8 +2088,10 @@ public final class ProcessList {
+ " with non-zero pid:" + app.getPid());
}
app.setDisabledCompatChanges(null);
+ app.setLoggableCompatChanges(null);
if (mPlatformCompat != null) {
app.setDisabledCompatChanges(mPlatformCompat.getDisabledChanges(app.info));
+ app.setLoggableCompatChanges(mPlatformCompat.getLoggableChanges(app.info));
}
final long startSeq = ++mProcStartSeqCounter;
app.setStartSeq(startSeq);
diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java
index 9fa3a8bf6f3b..b93908974a42 100644
--- a/services/core/java/com/android/server/am/ProcessRecord.java
+++ b/services/core/java/com/android/server/am/ProcessRecord.java
@@ -258,6 +258,12 @@ class ProcessRecord implements WindowProcessListener {
@GuardedBy("mService")
private long[] mDisabledCompatChanges;
+ /**
+ * Set of compat changes for the process that are intended to be logged to logcat.
+ */
+ @GuardedBy("mService")
+ private long[] mLoggableCompatChanges;
+
/**
* Who is watching for the death.
*/
@@ -934,11 +940,21 @@ class ProcessRecord implements WindowProcessListener {
return mDisabledCompatChanges;
}
+ @GuardedBy("mService")
+ long[] getLoggableCompatChanges() {
+ return mLoggableCompatChanges;
+ }
+
@GuardedBy("mService")
void setDisabledCompatChanges(long[] disabledCompatChanges) {
mDisabledCompatChanges = disabledCompatChanges;
}
+ @GuardedBy("mService")
+ void setLoggableCompatChanges(long[] loggableCompatChanges) {
+ mLoggableCompatChanges = loggableCompatChanges;
+ }
+
@GuardedBy("mService")
void unlinkDeathRecipient() {
if (mDeathRecipient != null && mThread != null) {
diff --git a/services/core/java/com/android/server/am/SettingsToPropertiesMapper.java b/services/core/java/com/android/server/am/SettingsToPropertiesMapper.java
index d1bda7991f45..7df5fdd282c3 100644
--- a/services/core/java/com/android/server/am/SettingsToPropertiesMapper.java
+++ b/services/core/java/com/android/server/am/SettingsToPropertiesMapper.java
@@ -170,6 +170,7 @@ public class SettingsToPropertiesMapper {
"pixel_connectivity_gps",
"pixel_system_sw_video",
"pixel_watch",
+ "platform_compat",
"platform_security",
"pmw",
"power",
diff --git a/services/core/java/com/android/server/compat/CompatConfig.java b/services/core/java/com/android/server/compat/CompatConfig.java
index 9102cfd0d426..79025d00d128 100644
--- a/services/core/java/com/android/server/compat/CompatConfig.java
+++ b/services/core/java/com/android/server/compat/CompatConfig.java
@@ -25,6 +25,7 @@ import android.compat.Compatibility.ChangeConfig;
import android.content.Context;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
+import android.os.Build;
import android.os.Environment;
import android.text.TextUtils;
import android.util.LongArray;
@@ -72,7 +73,6 @@ import javax.xml.datatype.DatatypeConfigurationException;
* been configured.
*/
final class CompatConfig {
-
private static final String TAG = "CompatConfig";
private static final String APP_COMPAT_DATA_DIR = "/data/misc/appcompat";
private static final String STATIC_OVERRIDES_PRODUCT_DIR = "/product/etc/appcompat";
@@ -148,6 +148,56 @@ final class CompatConfig {
return sortedChanges;
}
+ /**
+ * Retrieves the set of changes that are intended to be logged. This includes changes that
+ * target the most recent SDK version and are not disabled.
+ *
+ * @param app the app in question
+ * @return a sorted long array of change IDs
+ */
+ long[] getLoggableChanges(ApplicationInfo app) {
+ LongArray loggable = new LongArray(mChanges.size());
+ for (CompatChange c : mChanges.values()) {
+ long changeId = c.getId();
+ boolean isLatestSdk = isChangeTargetingLatestSdk(c, app.targetSdkVersion);
+ if (c.isEnabled(app, mAndroidBuildClassifier) && isLatestSdk) {
+ loggable.add(changeId);
+ }
+ }
+ final long[] sortedChanges = loggable.toArray();
+ Arrays.sort(sortedChanges);
+ return sortedChanges;
+ }
+
+ /**
+ * Whether the change indicated by the given changeId is targeting the latest SDK version.
+ * @param c the change for which to check the target SDK version
+ * @param appSdkVersion the target sdk version of the app
+ * @return true if the changeId targets the current sdk version or the current development
+ * version.
+ */
+ boolean isChangeTargetingLatestSdk(CompatChange c, int appSdkVersion) {
+ int maxTargetSdk = maxTargetSdkForCompatChange(c) + 1;
+ if (maxTargetSdk <= 0) {
+ // No max target sdk found.
+ return false;
+ }
+
+ return maxTargetSdk == Build.VERSION_CODES.CUR_DEVELOPMENT || maxTargetSdk == appSdkVersion;
+ }
+
+ /**
+ * Retrieves the CompatChange associated with the given changeId. Will return null if the
+ * changeId is not found. Used only for performance improvement purposes, in order to reduce
+ * lookups.
+ *
+ * @param changeId for which to look up the CompatChange
+ * @return the found compat change, or null if not found.
+ */
+ CompatChange getCompatChange(long changeId) {
+ return mChanges.get(changeId);
+ }
+
/**
* Looks up a change ID by name.
*
@@ -164,7 +214,7 @@ final class CompatConfig {
}
/**
- * Checks if a given change is enabled for a given application.
+ * Checks if a given change id is enabled for a given application.
*
* @param changeId the ID of the change in question
* @param app app to check for
@@ -173,6 +223,18 @@ final class CompatConfig {
*/
boolean isChangeEnabled(long changeId, ApplicationInfo app) {
CompatChange c = mChanges.get(changeId);
+ return isChangeEnabled(c, app);
+ }
+
+ /**
+ * Checks if a given change is enabled for a given application.
+ *
+ * @param c the CompatChange in question
+ * @param app the app to check for
+ * @return {@code true} if the change is enabled for this app. Also returns {@code true} if the
+ * change ID is not known, as unknown changes are enabled by default.
+ */
+ boolean isChangeEnabled(CompatChange c, ApplicationInfo app) {
if (c == null) {
// we know nothing about this change: default behaviour is enabled.
return true;
@@ -301,9 +363,21 @@ final class CompatConfig {
/**
* Returns the maximum SDK version for which this change can be opted in (or -1 if it is not
* target SDK gated).
+ *
+ * @param changeId the id of the CompatChange to check for the max target sdk
*/
int maxTargetSdkForChangeIdOptIn(long changeId) {
CompatChange c = mChanges.get(changeId);
+ return maxTargetSdkForCompatChange(c);
+ }
+
+ /**
+ * Returns the maximum SDK version for which this change can be opted in (or -1 if it is not
+ * target SDK gated).
+ *
+ * @param c the CompatChange to check for the max target sdk
+ */
+ int maxTargetSdkForCompatChange(CompatChange c) {
if (c != null && c.getEnableSinceTargetSdk() != -1) {
return c.getEnableSinceTargetSdk() - 1;
}
diff --git a/services/core/java/com/android/server/compat/PlatformCompat.java b/services/core/java/com/android/server/compat/PlatformCompat.java
index 6cca130af35d..f8fd0a0790e0 100644
--- a/services/core/java/com/android/server/compat/PlatformCompat.java
+++ b/services/core/java/com/android/server/compat/PlatformCompat.java
@@ -120,8 +120,16 @@ public class PlatformCompat extends IPlatformCompat.Stub {
reportChangeInternal(changeId, uid, ChangeReporter.STATE_LOGGED);
}
+ /**
+ * Report the change, but skip over the sdk target version check. This can be used to force the
+ * debug logs.
+ *
+ * @param changeId of the change to report
+ * @param uid of the user
+ * @param state of the change - enabled/disabled/logged
+ */
private void reportChangeInternal(long changeId, int uid, int state) {
- mChangeReporter.reportChange(uid, changeId, state);
+ mChangeReporter.reportChange(uid, changeId, state, true);
}
@Override
@@ -164,15 +172,25 @@ public class PlatformCompat extends IPlatformCompat.Stub {
}
/**
- * Internal version of {@link #isChangeEnabled(long, ApplicationInfo)}.
+ * Internal version of {@link #isChangeEnabled(long, ApplicationInfo)}. If the provided appInfo
+ * is not null, also reports the change.
+ *
+ * @param changeId of the change to report
+ * @param appInfo the app to check
*
* <p>Does not perform costly permission check.
*/
public boolean isChangeEnabledInternal(long changeId, ApplicationInfo appInfo) {
- boolean enabled = isChangeEnabledInternalNoLogging(changeId, appInfo);
+ // Fetch the CompatChange. This is done here instead of in mCompatConfig to avoid multiple
+ // fetches.
+ CompatChange c = mCompatConfig.getCompatChange(changeId);
+
+ boolean enabled = mCompatConfig.isChangeEnabled(c, appInfo);
+ int state = enabled ? ChangeReporter.STATE_ENABLED : ChangeReporter.STATE_DISABLED;
if (appInfo != null) {
- reportChangeInternal(changeId, appInfo.uid,
- enabled ? ChangeReporter.STATE_ENABLED : ChangeReporter.STATE_DISABLED);
+ boolean isTargetingLatestSdk =
+ mCompatConfig.isChangeTargetingLatestSdk(c, appInfo.targetSdkVersion);
+ mChangeReporter.reportChange(appInfo.uid, changeId, state, isTargetingLatestSdk);
}
return enabled;
}
@@ -398,6 +416,19 @@ public class PlatformCompat extends IPlatformCompat.Stub {
return mCompatConfig.getDisabledChanges(appInfo);
}
+ /**
+ * Retrieves the set of changes that should be logged for a given app. Any change ID not in the
+ * returned array is ignored for logging purposes.
+ *
+ * @param appInfo The app in question
+ * @return A sorted long array of change IDs. We use a primitive array to minimize memory
+ * footprint: Every app process will store this array statically so we aim to reduce
+ * overhead as much as possible.
+ */
+ public long[] getLoggableChanges(ApplicationInfo appInfo) {
+ return mCompatConfig.getLoggableChanges(appInfo);
+ }
+
/**
* Look up a change ID by name.
*
diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java
index 2112dae0f311..73d830dcde33 100644
--- a/services/java/com/android/server/SystemServer.java
+++ b/services/java/com/android/server/SystemServer.java
@@ -1134,7 +1134,7 @@ public final class SystemServer implements Dumpable {
ServiceManager.addService(Context.PLATFORM_COMPAT_SERVICE, platformCompat);
ServiceManager.addService(Context.PLATFORM_COMPAT_NATIVE_SERVICE,
new PlatformCompatNative(platformCompat));
- AppCompatCallbacks.install(new long[0]);
+ AppCompatCallbacks.install(new long[0], new long[0]);
t.traceEnd();
// FileIntegrityService responds to requests from apps and the system. It needs to run after
diff --git a/services/tests/mockingservicestests/src/com/android/server/am/AsyncProcessStartTest.java b/services/tests/mockingservicestests/src/com/android/server/am/AsyncProcessStartTest.java
index caa08647628e..a8b792e30485 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/AsyncProcessStartTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/AsyncProcessStartTest.java
@@ -213,7 +213,7 @@ public class AsyncProcessStartTest {
any(), any(), any(),
any(), any(),
any(), any(),
- any(),
+ any(), any(),
anyLong(), anyLong());
final ProcessRecord r = spy(new ProcessRecord(mAms, ai, ai.processName, ai.uid));
@@ -277,7 +277,7 @@ public class AsyncProcessStartTest {
null, null,
null,
null, null, null,
- null, null,
+ null, null, null,
0, 0);
// Sleep until timeout should have triggered
diff --git a/services/tests/mockingservicestests/src/com/android/server/am/ProcessObserverTest.java b/services/tests/mockingservicestests/src/com/android/server/am/ProcessObserverTest.java
index fcf761fb6607..67be93b3b49f 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/ProcessObserverTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/ProcessObserverTest.java
@@ -215,7 +215,7 @@ public class ProcessObserverTest {
any(), any(), any(),
any(), any(),
any(), any(),
- any(),
+ any(), any(),
anyLong(), anyLong());
final ProcessRecord r = spy(new ProcessRecord(mAms, ai, ai.processName, ai.uid));
r.setPid(myPid());
@@ -263,7 +263,7 @@ public class ProcessObserverTest {
null, null,
null,
null, null, null,
- null, null,
+ null, null, null,
0, 0);
return app;
}
diff --git a/services/tests/servicestests/src/com/android/server/compat/CompatConfigTest.java b/services/tests/servicestests/src/com/android/server/compat/CompatConfigTest.java
index ef15f60101d4..36b163ec84b6 100644
--- a/services/tests/servicestests/src/com/android/server/compat/CompatConfigTest.java
+++ b/services/tests/servicestests/src/com/android/server/compat/CompatConfigTest.java
@@ -172,6 +172,25 @@ public class CompatConfigTest {
.asList().containsExactly(12L, 123L, 1234L);
}
+ @Test
+ public void testGetLoggableChanges() throws Exception {
+ final long disabledChangeId = 1234L;
+ final long enabledLatestChangeId = 2345L;
+ final long enabledOlderChangeId = 3456L;
+ CompatConfig compatConfig = CompatConfigBuilder.create(mBuildClassifier, mContext)
+ // Disabled changes should not be logged.
+ .addDisabledChangeWithId(disabledChangeId)
+ // A change targeting the latest sdk should be logged.
+ .addEnableSinceSdkChangeWithId(3, enabledLatestChangeId)
+ // A change targeting an old sdk should not be logged.
+ .addEnableSinceSdkChangeWithId(1, enabledOlderChangeId)
+ .build();
+
+ assertThat(compatConfig.getLoggableChanges(
+ ApplicationInfoBuilder.create().withTargetSdk(3).build()))
+ .asList().containsExactly(enabledLatestChangeId);
+ }
+
@Test
public void testPackageOverrideEnabled() throws Exception {
CompatConfig compatConfig = CompatConfigBuilder.create(mBuildClassifier, mContext)
--
GitLab