Merge "Don't duplicate issue impressions per session." into udc-dev

4 files changed, 57 insertions, 33 deletions

diff --git a/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/InteractionLogger.kt b/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/InteractionLogger.kt
index 29edd8778..58bec87b8 100644
--- a/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/InteractionLogger.kt
+++ b/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/InteractionLogger.kt
@@ -27,6 +27,8 @@ import android.safetycenter.SafetyCenterEntry
 import android.safetycenter.SafetyCenterIssue
 import android.safetycenter.SafetyCenterManager.EXTRA_SAFETY_SOURCE_ISSUE_ID
 import android.safetycenter.SafetyCenterStatus
+import android.safetycenter.config.SafetyCenterConfig
+import android.safetycenter.config.SafetySource
 import androidx.annotation.RequiresApi
 import com.android.permissioncontroller.Constants
 import com.android.permissioncontroller.PermissionControllerStatsLog
@@ -41,18 +43,39 @@ import java.math.BigInteger
 import java.security.MessageDigest
 
 @RequiresApi(Build.VERSION_CODES.TIRAMISU)
-class InteractionLogger(
-    private val noLogSourceIds: Set<String?>,
-    var sessionId: Long = Constants.INVALID_SESSION_ID,
-    var viewType: ViewType = ViewType.UNKNOWN,
-    var navigationSource: NavigationSource = NavigationSource.UNKNOWN,
-    var navigationSensor: Sensor = Sensor.UNKNOWN,
-    var groupId: String? = null
+class InteractionLogger
+private constructor(
+    private val noLogSourceIds: Set<String?>
 ) {
+    var sessionId: Long = Constants.INVALID_SESSION_ID
+    var viewType: ViewType = ViewType.UNKNOWN
+    var navigationSource: NavigationSource = NavigationSource.UNKNOWN
+    var navigationSensor: Sensor = Sensor.UNKNOWN
+    var groupId: String? = null
+
+    private val viewedIssueIds: MutableSet<String> = mutableSetOf()
+
+    constructor(
+        safetyCenterConfig: SafetyCenterConfig?
+    ) : this(extractNoLogSourceIds(safetyCenterConfig))
+
     fun record(action: Action) {
         writeAtom(action)
     }
 
+    fun recordIssueViewed(issue: SafetyCenterIssue, isDismissed: Boolean) {
+        if (viewedIssueIds.contains(issue.id)) {
+            return
+        }
+
+        recordForIssue(Action.SAFETY_ISSUE_VIEWED, issue, isDismissed)
+        viewedIssueIds.add(issue.id)
+    }
+
+    fun clearViewedIssues() {
+        viewedIssueIds.clear()
+    }
+
     fun recordForIssue(action: Action, issue: SafetyCenterIssue, isDismissed: Boolean) {
         val decodedId = SafetyCenterIds.issueIdFromString(issue.id)
         writeAtom(
@@ -133,6 +156,26 @@ class InteractionLogger(
             // Truncate to the size of a long
             return BigInteger(digest.digest()).toLong()
         }
+
+        private fun extractNoLogSourceIds(safetyCenterConfig: SafetyCenterConfig?): Set<String?> {
+            if (safetyCenterConfig == null) return setOf()
+
+            return safetyCenterConfig.safetySourcesGroups
+                .asSequence()
+                .flatMap { it.safetySources }
+                .filterNot { it.isLoggable() }
+                .map { it.id }
+                .toSet()
+        }
+
+        private fun SafetySource.isLoggable(): Boolean =
+            try {
+                isLoggingAllowed
+            } catch (ex: UnsupportedOperationException) {
+                // isLoggingAllowed will throw if you call it on a static source :(
+                // Default to logging all sources that don't support this config value.
+                true
+            }
     }
 }
 
diff --git a/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/IssueCardPreference.java b/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/IssueCardPreference.java
index 16cd05dac..f6eaa319d 100644
--- a/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/IssueCardPreference.java
+++ b/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/IssueCardPreference.java
@@ -136,7 +136,7 @@ public class IssueCardPreference extends Preference implements ComparablePrefere
 
         mSafetyCenterViewModel
                 .getInteractionLogger()
-                .recordForIssue(Action.SAFETY_ISSUE_VIEWED, mIssue, mIsDismissed);
+                .recordIssueViewed(mIssue, mIsDismissed);
     }
 
     private void maybeDisplayText(@Nullable CharSequence maybeText, TextView textView) {
diff --git a/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/SafetyCenterFragment.kt b/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/SafetyCenterFragment.kt
index 9ff8bee1a..7d5dbb3cb 100644
--- a/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/SafetyCenterFragment.kt
+++ b/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/SafetyCenterFragment.kt
@@ -134,6 +134,11 @@ abstract class SafetyCenterFragment : PreferenceFragmentCompat() {
         highlightManager.saveState(outState)
     }
 
+    override fun onStop() {
+        super.onStop()
+        safetyCenterViewModel.interactionLogger.clearViewedIssues()
+    }
+
     override fun onDestroy() {
         super.onDestroy()
         if (activity?.isChangingConfigurations == true) {
diff --git a/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/model/LiveSafetyCenterViewModel.kt b/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/model/LiveSafetyCenterViewModel.kt
index 65a7fced0..4714d42cb 100644
--- a/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/model/LiveSafetyCenterViewModel.kt
+++ b/PermissionController/src/com/android/permissioncontroller/safetycenter/ui/model/LiveSafetyCenterViewModel.kt
@@ -27,7 +27,6 @@ import android.safetycenter.SafetyCenterErrorDetails
 import android.safetycenter.SafetyCenterIssue
 import android.safetycenter.SafetyCenterManager
 import android.safetycenter.SafetyCenterStatus
-import android.safetycenter.config.SafetySource
 import android.util.Log
 import androidx.annotation.MainThread
 import androidx.annotation.RequiresApi
@@ -56,32 +55,9 @@ class LiveSafetyCenterViewModel(app: Application) : SafetyCenterViewModel(app) {
     private val _errorLiveData = MutableLiveData<SafetyCenterErrorDetails>()
 
     override val interactionLogger: InteractionLogger by lazy {
-        fun isLoggable(safetySource: SafetySource): Boolean {
-            return try {
-                safetySource.isLoggingAllowed
-            } catch (ex: UnsupportedOperationException) {
-                // isLoggingAllowed will throw if you call it on a static source :(
-                // Default to logging all sources that don't support this config value.
-                true
-            }
-        }
-
         // Fetching the config to build this set of source IDs requires IPC, so we do this
         // initialization lazily.
-        val safetyCenterConfig = safetyCenterManager.safetyCenterConfig
-
-        InteractionLogger(
-            if (safetyCenterConfig != null) {
-                safetyCenterConfig.safetySourcesGroups
-                    .asSequence()
-                    .flatMap { it.safetySources }
-                    .filterNot(::isLoggable)
-                    .map { it.id }
-                    .toSet()
-            } else {
-                setOf()
-            }
-        )
+        InteractionLogger(safetyCenterManager.safetyCenterConfig)
     }
 
     private var changingConfigurations = false