Merge "Avoid logging change reports to statsd from system apps" into main (6f9beb9c) · Commits · e / os / android_frameworks_base

core/java/android/app/AppCompatCallbacks.java

+1 −1

Original line number	Diff line number	Diff line
		@@ -82,7 +82,7 @@ public final class AppCompatCallbacks implements Compatibility.BehaviorChangeDel

		private void reportChange(long changeId, int state, boolean isLoggable) {
		int uid = Process.myUid();
		mChangeReporter.reportChange(uid, changeId, state, isLoggable);
		mChangeReporter.reportChange(uid, changeId, state, false, isLoggable);
		}

		}

core/java/com/android/internal/compat/ChangeReporter.java

+32 −15

Original line number	Diff line number	Diff line
		@@ -42,7 +42,7 @@ import java.util.function.Function;
		*
		* @hide
		*/
		public final class ChangeReporter {
		public class ChangeReporter {
		private static final String TAG = "CompatChangeReporter";
		private static final Function<Integer, Set<ChangeReport>> NEW_CHANGE_REPORT_SET =
		uid -> Collections.synchronizedSet(new HashSet<>());
		@@ -91,14 +91,17 @@ public final class ChangeReporter {
		* @param uid affected by the change
		* @param changeId the reported change id
		* @param state of the reported change - enabled/disabled/only logged
		* @param isKnownSystemApp do we know that the affected app is a system app? (if true is
		* definitely a system app, if false it may or may not be a system app)
		* @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.
		* 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, boolean isLoggableBySdk) {
		public void reportChange(int uid, long changeId, int state, boolean isKnownSystemApp,
		boolean isLoggableBySdk) {
		boolean isAlreadyReported =
		checkAndSetIsAlreadyReported(uid, new ChangeReport(changeId, state));
		if (!isAlreadyReported) {
		if (shouldWriteToStatsLog(isKnownSystemApp, isAlreadyReported)) {
		FrameworkStatsLog.write(FrameworkStatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid,
		changeId, state, mSource);
		}
		@@ -116,7 +119,7 @@ public final class ChangeReporter {
		* @param state of the reported change - enabled/disabled/only logged
		*/
		public void reportChange(int uid, long changeId, int state) {
		reportChange(uid, changeId, state, true);
		reportChange(uid, changeId, state, false, true);
		}

		/**
		@@ -136,14 +139,15 @@ public final class ChangeReporter {
		/**
		* Returns whether the next report should be logged to FrameworkStatsLog.
		*
		* @param uid affected by the change
		* @param changeId the reported change id
		* @param state of the reported change - enabled/disabled/only logged
		* @param isKnownSystemApp do we know that the affected app is a system app? (if true is
		* definitely a system app, if false it may or may not be a system app)
		* @param isAlreadyReported is the change already reported
		* @return true if the report should be logged
		*/
		@VisibleForTesting
		boolean shouldWriteToStatsLog(int uid, long changeId, int state) {
		return !isAlreadyReported(uid, new ChangeReport(changeId, state));
		boolean shouldWriteToStatsLog(boolean isKnownSystemApp, boolean isAlreadyReported) {
		// We don't log where we know the source is a system app or is already reported
		return !isKnownSystemApp && !isAlreadyReported;
		}

		/**
		@@ -224,6 +228,19 @@ public final class ChangeReporter {
		return mReportedChanges.getOrDefault(uid, EMPTY_SET).contains(report);
		}

		/**
		* Returns whether the next report should be logged.
		*
		* @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
		boolean isAlreadyReported(int uid, long changeId, int state) {
		return isAlreadyReported(uid, new ChangeReport(changeId, state));
		}

		private void markAsReported(int uid, ChangeReport report) {
		mReportedChanges.computeIfAbsent(uid, NEW_CHANGE_REPORT_SET).add(report);
		}

core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java

+33 −8

Original line number	Diff line number	Diff line
		@@ -37,15 +37,20 @@ public class ChangeReporterTest {
		long myChangeId = 500L, otherChangeId = 600L;
		int myState = ChangeReporter.STATE_ENABLED, otherState = ChangeReporter.STATE_DISABLED;

		assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
		assertTrue(reporter.shouldWriteToStatsLog(false,
		reporter.isAlreadyReported(myUid, myChangeId, myState)));
		reporter.reportChange(myUid, myChangeId, myState);

		// Same report will not be logged again.
		assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
		assertFalse(reporter.shouldWriteToStatsLog(false,
		reporter.isAlreadyReported(myUid, myChangeId, myState)));
		// Other reports will be logged.
		assertTrue(reporter.shouldWriteToStatsLog(otherUid, myChangeId, myState));
		assertTrue(reporter.shouldWriteToStatsLog(myUid, otherChangeId, myState));
		assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, otherState));
		assertTrue(reporter.shouldWriteToStatsLog(false,
		reporter.isAlreadyReported(otherUid, myChangeId, myState)));
		assertTrue(reporter.shouldWriteToStatsLog(false,
		reporter.isAlreadyReported(myUid, otherChangeId, myState)));
		assertTrue(reporter.shouldWriteToStatsLog(false,
		reporter.isAlreadyReported(myUid, myChangeId, otherState)));
		}

		@Test
		@@ -55,15 +60,18 @@ public class ChangeReporterTest {
		long myChangeId = 500L;
		int myState = ChangeReporter.STATE_ENABLED;

		assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
		assertTrue(reporter.shouldWriteToStatsLog(false,
		reporter.isAlreadyReported(myUid, myChangeId, myState)));
		reporter.reportChange(myUid, myChangeId, myState);

		// Same report will not be logged again.
		assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
		assertFalse(reporter.shouldWriteToStatsLog(false,
		reporter.isAlreadyReported(myUid, myChangeId, myState)));
		reporter.resetReportedChanges(myUid);

		// Same report will be logged again after reset.
		assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
		assertTrue(reporter.shouldWriteToStatsLog(false,
		reporter.isAlreadyReported(myUid, myChangeId, myState)));
		}

		@Test
		@@ -196,4 +204,21 @@ public class ChangeReporterTest {
		// off.
		assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myDisabledState, false));
		}

		@Test
		public void testDontLogSystemApps() {
		// Verify we don't log an app if we know it's a system app when source is system server.
		ChangeReporter systemServerReporter =
		new ChangeReporter(ChangeReporter.SOURCE_SYSTEM_SERVER);

		assertFalse(systemServerReporter.shouldWriteToStatsLog(true, false));
		assertFalse(systemServerReporter.shouldWriteToStatsLog(true, true));

		// Verify we don't log an app if we know it's a system app when source is unknown.
		ChangeReporter unknownReporter =
		new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);

		assertFalse(unknownReporter.shouldWriteToStatsLog(true, false));
		assertFalse(unknownReporter.shouldWriteToStatsLog(true, true));
		}
		}

services/core/java/com/android/server/compat/PlatformCompat.java

+21 −9

Original line number	Diff line number	Diff line
		@@ -83,9 +83,10 @@ public class PlatformCompat extends IPlatformCompat.Stub {

		@VisibleForTesting
		PlatformCompat(Context context, CompatConfig compatConfig,
		AndroidBuildClassifier buildClassifier) {
		AndroidBuildClassifier buildClassifier,
		ChangeReporter changeReporter) {
		mContext = context;
		mChangeReporter = new ChangeReporter(ChangeReporter.SOURCE_SYSTEM_SERVER);
		mChangeReporter = changeReporter;
		mCompatConfig = compatConfig;
		mBuildClassifier = buildClassifier;

		@@ -96,8 +97,11 @@ public class PlatformCompat extends IPlatformCompat.Stub {
		@EnforcePermission(LOG_COMPAT_CHANGE)
		public void reportChange(long changeId, ApplicationInfo appInfo) {
		super.reportChange_enforcePermission();

		reportChangeInternal(changeId, appInfo.uid, ChangeReporter.STATE_LOGGED);
		reportChangeInternal(
		changeId,
		appInfo.uid,
		appInfo.isSystemApp(),
		ChangeReporter.STATE_LOGGED);
		}

		@Override
		@@ -108,7 +112,11 @@ public class PlatformCompat extends IPlatformCompat.Stub {

		ApplicationInfo appInfo = getApplicationInfo(packageName, userId);
		if (appInfo != null) {
		reportChangeInternal(changeId, appInfo.uid, ChangeReporter.STATE_LOGGED);
		reportChangeInternal(
		changeId,
		appInfo.uid,
		appInfo.isSystemApp(),
		ChangeReporter.STATE_LOGGED);
		}
		}

		@@ -117,7 +125,7 @@ public class PlatformCompat extends IPlatformCompat.Stub {
		public void reportChangeByUid(long changeId, int uid) {
		super.reportChangeByUid_enforcePermission();

		reportChangeInternal(changeId, uid, ChangeReporter.STATE_LOGGED);
		reportChangeInternal(changeId, uid, false, ChangeReporter.STATE_LOGGED);
		}

		/**
		@@ -128,8 +136,8 @@ public class PlatformCompat extends IPlatformCompat.Stub {
		* @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, true);
		private void reportChangeInternal(long changeId, int uid, boolean isKnownSystemApp, int state) {
		mChangeReporter.reportChange(uid, changeId, state, isKnownSystemApp, true);
		}

		@Override
		@@ -190,7 +198,11 @@ public class PlatformCompat extends IPlatformCompat.Stub {
		if (appInfo != null) {
		boolean isTargetingLatestSdk =
		mCompatConfig.isChangeTargetingLatestSdk(c, appInfo.targetSdkVersion);
		mChangeReporter.reportChange(appInfo.uid, changeId, state, isTargetingLatestSdk);
		mChangeReporter.reportChange(appInfo.uid,
		changeId,
		state,
		appInfo.isSystemApp(),
		isTargetingLatestSdk);
		}
		return enabled;
		}

services/tests/servicestests/src/com/android/server/compat/ApplicationInfoBuilder.java

+16 −0

Original line number	Diff line number	Diff line
		@@ -21,8 +21,10 @@ import android.content.pm.ApplicationInfo;
		class ApplicationInfoBuilder {
		private boolean mIsDebuggable;
		private int mTargetSdk;
		private int mUid;
		private String mPackageName;
		private long mVersionCode;
		private boolean mIsSystemApp;

		private ApplicationInfoBuilder() {
		mTargetSdk = -1;
		@@ -42,6 +44,16 @@ class ApplicationInfoBuilder {
		return this;
		}

		ApplicationInfoBuilder systemApp() {
		mIsSystemApp = true;
		return this;
		}

		ApplicationInfoBuilder withUid(int uid) {
		mUid = uid;
		return this;
		}

		ApplicationInfoBuilder withPackageName(String packageName) {
		mPackageName = packageName;
		return this;
		@@ -60,6 +72,10 @@ class ApplicationInfoBuilder {
		applicationInfo.packageName = mPackageName;
		applicationInfo.targetSdkVersion = mTargetSdk;
		applicationInfo.longVersionCode = mVersionCode;
		applicationInfo.uid = mUid;
		if (mIsSystemApp) {
		applicationInfo.flags \|= ApplicationInfo.FLAG_SYSTEM;
		}
		return applicationInfo;
		}
		}