diff options
5 files changed, 91 insertions, 23 deletions
diff --git a/service/java/com/android/safetycenter/SafetyCenterRefreshTracker.java b/service/java/com/android/safetycenter/SafetyCenterRefreshTracker.java index 7133fbfc6..a9ebe185e 100644 --- a/service/java/com/android/safetycenter/SafetyCenterRefreshTracker.java +++ b/service/java/com/android/safetycenter/SafetyCenterRefreshTracker.java @@ -149,18 +149,24 @@ public final class SafetyCenterRefreshTracker { * * <p>If a source calls {@code reportSafetySourceError}, then this method is also used to mark * the refresh as completed. The {@code successful} parameter indicates whether the refresh - * completed successfully or not. + * completed successfully or not. The {@code dataChanged} parameter indicates whether this + * source's data changed or not. * * <p>Completed refreshes are logged to statsd. */ public boolean reportSourceRefreshCompleted( - String refreshBroadcastId, String sourceId, @UserIdInt int userId, boolean successful) { + String refreshBroadcastId, + String sourceId, + @UserIdInt int userId, + boolean successful, + boolean dataChanged) { if (!checkRefreshInProgress("reportSourceRefreshCompleted", refreshBroadcastId)) { return false; } SafetySourceKey sourceKey = SafetySourceKey.of(sourceId, userId); - Duration duration = mRefreshInProgress.markSourceRefreshComplete(sourceKey, successful); + Duration duration = + mRefreshInProgress.markSourceRefreshComplete(sourceKey, successful, dataChanged); int refreshReason = mRefreshInProgress.getReason(); int requestType = RefreshReasons.toRefreshRequestType(refreshReason); @@ -172,7 +178,8 @@ public final class SafetyCenterRefreshTracker { UserUtils.isManagedProfile(userId, mContext), duration, sourceResult, - refreshReason); + refreshReason, + dataChanged); } if (!mRefreshInProgress.isComplete()) { @@ -186,7 +193,8 @@ public final class SafetyCenterRefreshTracker { requestType, mRefreshInProgress.getDurationSinceStart(), wholeResult, - refreshReason); + refreshReason, + mRefreshInProgress.hasAnyTrackedSourceDataChanged()); mRefreshInProgress = null; return true; } @@ -270,7 +278,8 @@ public final class SafetyCenterRefreshTracker { UserUtils.isManagedProfile(sourceKey.getUserId(), mContext), duration, SAFETY_CENTER_SYSTEM_EVENT_REPORTED__RESULT__TIMEOUT, - refreshReason); + refreshReason, + false); } } @@ -278,7 +287,8 @@ public final class SafetyCenterRefreshTracker { requestType, clearedRefresh.getDurationSinceStart(), SAFETY_CENTER_SYSTEM_EVENT_REPORTED__RESULT__TIMEOUT, - refreshReason); + refreshReason, + clearedRefresh.hasAnyTrackedSourceDataChanged()); return timedOutSources; } @@ -348,6 +358,7 @@ public final class SafetyCenterRefreshTracker { private final ArrayMap<SafetySourceKey, Long> mSourceRefreshesInFlight = new ArrayMap<>(); private boolean mAnyTrackedSourceErrors = false; + private boolean mAnyTrackedSourceDataChanged = false; RefreshInProgress( String id, @@ -400,6 +411,11 @@ public final class SafetyCenterRefreshTracker { return mAnyTrackedSourceErrors; } + /** Returns {@code true} if any refresh of a tracked source changed that source's data. */ + private boolean hasAnyTrackedSourceDataChanged() { + return mAnyTrackedSourceDataChanged; + } + private void markSourceRefreshInFlight(SafetySourceKey safetySourceKey) { boolean tracked = isTracked(safetySourceKey); long currentElapsedMillis = SystemClock.elapsedRealtime(); @@ -425,11 +441,12 @@ public final class SafetyCenterRefreshTracker { @Nullable private Duration markSourceRefreshComplete( - SafetySourceKey safetySourceKey, boolean successful) { + SafetySourceKey safetySourceKey, boolean successful, boolean dataChanged) { Long startElapsedMillis = mSourceRefreshesInFlight.remove(safetySourceKey); boolean tracked = isTracked(safetySourceKey); mAnyTrackedSourceErrors |= (tracked && !successful); + mAnyTrackedSourceDataChanged |= dataChanged; Duration duration = (startElapsedMillis == null) ? null @@ -446,6 +463,8 @@ public final class SafetyCenterRefreshTracker { + duration + " successful:" + successful + + " dataChanged:" + + dataChanged + " & tracking:" + tracked + ", " @@ -498,6 +517,8 @@ public final class SafetyCenterRefreshTracker { + mStartElapsedMillis + ", mAnyTrackedSourceErrors=" + mAnyTrackedSourceErrors + + ", mAnyTrackedSourceDataChanged=" + + mAnyTrackedSourceDataChanged + '}'; } } diff --git a/service/java/com/android/safetycenter/data/SafetySourceDataRepository.java b/service/java/com/android/safetycenter/data/SafetySourceDataRepository.java index 809ed1e8e..1d70ddea9 100644 --- a/service/java/com/android/safetycenter/data/SafetySourceDataRepository.java +++ b/service/java/com/android/safetycenter/data/SafetySourceDataRepository.java @@ -143,9 +143,10 @@ final class SafetySourceDataRepository { refreshReason = mSafetyCenterRefreshTracker.getRefreshReason(); } - boolean eventCausedChange = processSafetyEvent(safetySourceId, safetyEvent, userId, false); - boolean removedSourceError = mSafetySourceErrors.remove(key); boolean sourceDataDiffers = !Objects.equals(safetySourceData, mSafetySourceData.get(key)); + boolean eventCausedChange = + processSafetyEvent(safetySourceId, safetyEvent, userId, false, sourceDataDiffers); + boolean removedSourceError = mSafetySourceErrors.remove(key); if (sourceDataDiffers) { setSafetySourceDataInternal(key, safetySourceData); @@ -232,7 +233,7 @@ final class SafetySourceDataRepository { Log.w(TAG, "Error reported from source: " + safetySourceId + ", for event: " + safetyEvent); boolean safetyEventChangedSafetyCenterData = - processSafetyEvent(safetySourceId, safetyEvent, userId, true); + processSafetyEvent(safetySourceId, safetyEvent, userId, true, false); int safetyEventType = safetyEvent.getType(); if (safetyEventType == SafetyEvent.SAFETY_EVENT_TYPE_RESOLVING_ACTION_FAILED || safetyEventType == SafetyEvent.SAFETY_EVENT_TYPE_RESOLVING_ACTION_SUCCEEDED) { @@ -581,7 +582,8 @@ final class SafetySourceDataRepository { String safetySourceId, SafetyEvent safetyEvent, @UserIdInt int userId, - boolean isError) { + boolean isError, + boolean sourceDataChanged) { int type = safetyEvent.getType(); switch (type) { case SafetyEvent.SAFETY_EVENT_TYPE_REFRESH_REQUESTED: @@ -591,7 +593,7 @@ final class SafetySourceDataRepository { return false; } return mSafetyCenterRefreshTracker.reportSourceRefreshCompleted( - refreshBroadcastId, safetySourceId, userId, !isError); + refreshBroadcastId, safetySourceId, userId, !isError, sourceDataChanged); case SafetyEvent.SAFETY_EVENT_TYPE_RESOLVING_ACTION_SUCCEEDED: case SafetyEvent.SAFETY_EVENT_TYPE_RESOLVING_ACTION_FAILED: String safetySourceIssueId = safetyEvent.getSafetySourceIssueId(); diff --git a/service/java/com/android/safetycenter/logging/SafetyCenterStatsdLogger.java b/service/java/com/android/safetycenter/logging/SafetyCenterStatsdLogger.java index 7872fd14e..c29a25761 100644 --- a/service/java/com/android/safetycenter/logging/SafetyCenterStatsdLogger.java +++ b/service/java/com/android/safetycenter/logging/SafetyCenterStatsdLogger.java @@ -281,7 +281,8 @@ public final class SafetyCenterStatsdLogger { boolean isManagedProfile, Duration duration, @SystemEventResult int result, - long refreshReason) { + long refreshReason, + boolean dataChanged) { if (!SafetyCenterFlags.getAllowStatsdLogging()) { return; } @@ -294,8 +295,7 @@ public final class SafetyCenterStatsdLogger { duration.toMillis(), result, refreshReason, - // TODO(b/268328334): Track refreshReason and dataChanged for system events - UNSET_DATA_CHANGED); + dataChanged); } /** @@ -306,7 +306,8 @@ public final class SafetyCenterStatsdLogger { @RefreshRequestType int refreshType, Duration duration, @SystemEventResult int result, - long refreshReason) { + long refreshReason, + boolean dataChanged) { if (!SafetyCenterFlags.getAllowStatsdLogging()) { return; } @@ -319,8 +320,7 @@ public final class SafetyCenterStatsdLogger { duration.toMillis(), result, refreshReason, - // TODO(b/268328334): Track refreshReason and dataChanged for system events - UNSET_DATA_CHANGED); + dataChanged); } /** diff --git a/tests/hostside/safetycenter/helper-app/src/android/safetycenter/hostside/device/SafetySourceStateCollectedLoggingHelperTests.kt b/tests/hostside/safetycenter/helper-app/src/android/safetycenter/hostside/device/SafetySourceStateCollectedLoggingHelperTests.kt index 9c90142e7..1010930c2 100644 --- a/tests/hostside/safetycenter/helper-app/src/android/safetycenter/hostside/device/SafetySourceStateCollectedLoggingHelperTests.kt +++ b/tests/hostside/safetycenter/helper-app/src/android/safetycenter/hostside/device/SafetySourceStateCollectedLoggingHelperTests.kt @@ -76,6 +76,17 @@ class SafetySourceStateCollectedLoggingHelperTests { } @Test + fun refreshAllSources_twiceSameData_allSuccessful() { + repeat(2) { + simulateRefresh( + Response.SetData(safetySourceTestData.information), + Response.SetData(safetySourceTestData.recommendationWithAccountIssue), + Response.SetData(safetySourceTestData.criticalWithResolvingDeviceIssue) + ) + } + } + + @Test fun refreshAllSources_reasonPageOpen_oneError() { simulateRefresh( Response.SetData(safetySourceTestData.information), @@ -88,8 +99,6 @@ class SafetySourceStateCollectedLoggingHelperTests { fun refreshAllSources_reasonPageOpen_oneSuccessOneErrorOneTimeout() { SafetyCenterFlags.setAllRefreshTimeoutsTo(Coroutines.TIMEOUT_SHORT) simulateRefresh(Response.SetData(safetySourceTestData.information), Response.Error, null) - // Give time for responses to all sources - Thread.sleep(Coroutines.TIMEOUT_SHORT.toMillis() * 2) } @Test @@ -101,8 +110,6 @@ class SafetySourceStateCollectedLoggingHelperTests { null, refreshReason = SafetyCenterManager.REFRESH_REASON_RESCAN_BUTTON_CLICK ) - // Give time for responses to all sources - Thread.sleep(Coroutines.TIMEOUT_SHORT.toMillis() * 2) } private fun simulateRefresh( @@ -121,5 +128,7 @@ class SafetySourceStateCollectedLoggingHelperTests { SafetySourceReceiver.setResponse(Request.Refresh(SOURCE_ID_3), source3Response) } safetyCenterManager.refreshSafetySourcesWithReceiverPermissionAndWait(refreshReason) + // Give time for responses to all sources + Thread.sleep(Coroutines.TIMEOUT_SHORT.toMillis()) } } diff --git a/tests/hostside/safetycenter/src/android/safetycenter/hostside/SafetyCenterSystemEventReportedLoggingHostTest.kt b/tests/hostside/safetycenter/src/android/safetycenter/hostside/SafetyCenterSystemEventReportedLoggingHostTest.kt index a01dce43d..922ffb885 100644 --- a/tests/hostside/safetycenter/src/android/safetycenter/hostside/SafetyCenterSystemEventReportedLoggingHostTest.kt +++ b/tests/hostside/safetycenter/src/android/safetycenter/hostside/SafetyCenterSystemEventReportedLoggingHostTest.kt @@ -141,9 +141,45 @@ class SafetyCenterSystemEventReportedLoggingHostTest : BaseHostJUnit4Test() { assertWithMessage("the system event atoms").that(systemEventAtoms).hasSize(4) assertWithMessage("the number of atoms with the button-click refresh reason") .that(systemEventAtoms.count { it.refreshReason == REFRESH_REASON_BUTTON_CLICK }) + } + + fun refreshAllSources_firstTime_allSourcesSuccessful_dataChangedTrueForAll() { + helperAppRule.runTest( + ".SafetySourceStateCollectedLoggingHelperTests", + "refreshAllSources_reasonPageOpen_allSuccessful" + ) + + val systemEventAtoms = + ReportUtils.getEventMetricDataList(device).mapNotNull { + it.atom.safetyCenterSystemEventReported + } + + assertWithMessage("the number of atoms with dataChanged=true") + .that(systemEventAtoms.count { it.dataChanged }) .isEqualTo(4) } + @Test + fun refreshAllSources_secondTime_allSourcesUnchanged_dataChangedFalseForAll() { + helperAppRule.runTest( + ".SafetySourceStateCollectedLoggingHelperTests", + "refreshAllSources_twiceSameData_allSuccessful" + ) + + val systemEventAtoms = + ReportUtils.getEventMetricDataList(device).mapNotNull { + it.atom.safetyCenterSystemEventReported + } + + // There are three sources in the multiple sources config, of which one is not allowed to + // refresh on page open, except when there is no data. Plus, for each refresh there is an + // overall refresh atom making seven atoms in total. + assertWithMessage("the number of atoms").that(systemEventAtoms).hasSize(7) + assertWithMessage("the number of atoms with dataChanged=false") + .that(systemEventAtoms.count { !it.dataChanged }) + .isEqualTo(3) + } + companion object { private const val REFRESH_REASON_PAGE_OPEN = 100L private const val REFRESH_REASON_BUTTON_CLICK = 200L |