Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 84ee5e0e authored by Mark White's avatar Mark White
Browse files

Don't log change reports to statsd from system server

Test: presubmit
Fix: 343941691
Flag: EXEMPT bugfix

Change-Id: Id95aa9d1cafc00fa71852d79834793e728df2a10
parent dc295013
Loading
Loading
Loading
Loading
+21 −8
Original line number Diff line number Diff line
@@ -98,7 +98,7 @@ public final class ChangeReporter {
    public void reportChange(int uid, long changeId, int state, boolean isLoggableBySdk) {
        boolean isAlreadyReported =
                checkAndSetIsAlreadyReported(uid, new ChangeReport(changeId, state));
        if (!isAlreadyReported) {
        if (shouldWriteToStatsLog(isAlreadyReported)) {
            FrameworkStatsLog.write(FrameworkStatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid,
                    changeId, state, mSource);
        }
@@ -136,14 +136,16 @@ 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 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 isAlreadyReported) {
        // We don't log for system server
        if (mSource == SOURCE_SYSTEM_SERVER) return false;

        // Don't log if already reported
        return !isAlreadyReported;
    }

    /**
@@ -160,8 +162,6 @@ public final class ChangeReporter {
            boolean isAlreadyReported, 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) 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
@@ -224,6 +224,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);
    }
+26 −9
Original line number Diff line number Diff line
@@ -31,21 +31,21 @@ public class ChangeReporterTest {
    @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();

    @Test
    public void testStatsLogOnce() {
    public void testIsAlreadyReportedOnce() {
        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;

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

        // Same report will not be logged again.
        assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState));
        assertTrue(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));
        assertFalse(reporter.isAlreadyReported(otherUid, myChangeId, myState));
        assertFalse(reporter.isAlreadyReported(myUid, otherChangeId, myState));
        assertFalse(reporter.isAlreadyReported(myUid, myChangeId, otherState));
    }

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

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

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

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

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

    @Test
    public void testDontLogSystemServer() {
        ChangeReporter systemServerReporter =
                new ChangeReporter(ChangeReporter.SOURCE_SYSTEM_SERVER);

        // We never log from system server, even if already reported.
        assertFalse(systemServerReporter.shouldWriteToStatsLog(false));
        assertFalse(systemServerReporter.shouldWriteToStatsLog(true));

        ChangeReporter unknownSourceReporter =
                new ChangeReporter(ChangeReporter.SOURCE_UNKNOWN_SOURCE);

        // Otherwise we log if it's not already been reported.
        assertTrue(unknownSourceReporter.shouldWriteToStatsLog(false));
        assertFalse(unknownSourceReporter.shouldWriteToStatsLog(true));
    }
}