[SB Refactor] Add verbose logging about values received by the binder.

More debug logging for b/267236367: Add logs to see if the binder has received values from the view model (and if yes, which values). Bug: 267236367 Test: `adb shell dumpsys activity service com.android.systemui/.SystemUIService VerboseMobileViewLog` -> see binder logs Change-Id: I88baa0eb9aa5f302a197a0ee1c2ddfc51648e6e5
2023-03-02 18:46:31 +00:00
parent 3fac1f3366
commit f1214c529b
10 changed files with 146 additions and 7 deletions
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt
@@ -155,5 +155,12 @@ abstract class StatusBarPipelineModule {
        fun provideMobileViewLogBuffer(factory: LogBufferFactory): LogBuffer {
            return factory.create("MobileViewLog", 100)
        }
+
+        @Provides
+        @SysUISingleton
+        @VerboseMobileViewLog
+        fun provideVerboseMobileViewLogBuffer(factory: LogBufferFactory): LogBuffer {
+            return factory.create("VerboseMobileViewLog", 100)
+        }
    }
 }
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/VerboseMobileViewLog.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/VerboseMobileViewLog.kt
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.statusbar.pipeline.dagger
+
+import javax.inject.Qualifier
+
+/** Logs for **verbose** changes with the new mobile views. */
+@Qualifier
+@MustBeDocumented
+@kotlin.annotation.Retention(AnnotationRetention.RUNTIME)
+annotation class VerboseMobileViewLog
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/MobileViewLogger.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/MobileViewLogger.kt
@@ -17,7 +17,6 @@
 package com.android.systemui.statusbar.pipeline.mobile.ui

 import android.view.View
-import androidx.annotation.VisibleForTesting
 import com.android.systemui.Dumpable
 import com.android.systemui.dagger.SysUISingleton
 import com.android.systemui.dump.DumpManager
@@ -109,8 +108,7 @@ constructor(
    }

    companion object {
-        @VisibleForTesting
-        internal fun Any.getIdForLogging(): String {
+        fun Any.getIdForLogging(): String {
            // The identityHashCode is guaranteed to be constant for the lifetime of the object.
            return Integer.toHexString(System.identityHashCode(this))
        }
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/VerboseMobileViewLogger.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/VerboseMobileViewLogger.kt
@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.statusbar.pipeline.mobile.ui
+
+import android.view.View
+import com.android.systemui.common.shared.model.Icon
+import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.plugins.log.LogBuffer
+import com.android.systemui.plugins.log.LogLevel
+import com.android.systemui.statusbar.pipeline.dagger.VerboseMobileViewLog
+import com.android.systemui.statusbar.pipeline.mobile.ui.MobileViewLogger.Companion.getIdForLogging
+import com.android.systemui.statusbar.pipeline.mobile.ui.model.SignalIconModel
+import javax.inject.Inject
+
+/**
+ * Logs for **verbose** changes with the new mobile views.
+ *
+ * This is a hopefully temporary log until we resolve some open bugs (b/267236367, b/269565345,
+ * b/270300839).
+ */
+@SysUISingleton
+class VerboseMobileViewLogger
+@Inject
+constructor(
+    @VerboseMobileViewLog private val buffer: LogBuffer,
+) {
+    fun logBinderReceivedSignalIcon(parentView: View, subId: Int, icon: SignalIconModel) {
+        buffer.log(
+            TAG,
+            LogLevel.VERBOSE,
+            {
+                str1 = parentView.getIdForLogging()
+                int1 = subId
+                int2 = icon.level
+                bool1 = icon.showExclamationMark
+            },
+            {
+                "Binder[subId=$int1, viewId=$str1] received new signal icon: " +
+                    "level=$int2 showExclamation=$bool1"
+            },
+        )
+    }
+
+    fun logBinderReceivedNetworkTypeIcon(parentView: View, subId: Int, icon: Icon.Resource?) {
+        buffer.log(
+            TAG,
+            LogLevel.VERBOSE,
+            {
+                str1 = parentView.getIdForLogging()
+                int1 = subId
+                bool1 = icon != null
+                int2 = icon?.res ?: -1
+            },
+            {
+                "Binder[subId=$int1, viewId=$str1] received new network type icon: " +
+                    if (bool1) "resId=$int2" else "null"
+            },
+        )
+    }
+}
+
+private const val TAG = "VerboseMobileViewLogger"
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt
@@ -104,6 +104,11 @@ object MobileIconBinder {
                // Set the icon for the triangle
                launch {
                    viewModel.icon.distinctUntilChanged().collect { icon ->
+                        viewModel.verboseLogger?.logBinderReceivedSignalIcon(
+                            view,
+                            viewModel.subscriptionId,
+                            icon,
+                        )
                        mobileDrawable.level =
                            SignalDrawable.getState(
                                icon.level,
@@ -122,6 +127,11 @@ object MobileIconBinder {
                // Set the network type icon
                launch {
                    viewModel.networkTypeIcon.distinctUntilChanged().collect { dataTypeId ->
+                        viewModel.verboseLogger?.logBinderReceivedNetworkTypeIcon(
+                            view,
+                            viewModel.subscriptionId,
+                            dataTypeId,
+                        )
                        dataTypeId?.let { IconViewBinder.bind(dataTypeId, networkTypeView) }
                        networkTypeView.visibility = if (dataTypeId != null) VISIBLE else GONE
                    }
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt
@@ -19,6 +19,7 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel
 import android.graphics.Color
 import com.android.systemui.statusbar.phone.StatusBarLocation
 import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags
+import com.android.systemui.statusbar.pipeline.mobile.ui.VerboseMobileViewLogger

 /**
 * A view model for an individual mobile icon that embeds the notion of a [StatusBarLocation]. This
@@ -27,12 +28,14 @@ import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags
 * @param commonImpl for convenience, this class wraps a base interface that can provides all of the
 *   common implementations between locations. See [MobileIconViewModel]
 * @property locationName the name of the location of this VM, used for logging.
+ * @property verboseLogger an optional logger to log extremely verbose view updates.
 */
 abstract class LocationBasedMobileViewModel(
    val commonImpl: MobileIconViewModelCommon,
    statusBarPipelineFlags: StatusBarPipelineFlags,
    debugTint: Int,
    val locationName: String,
+    val verboseLogger: VerboseMobileViewLogger?,
 ) : MobileIconViewModelCommon by commonImpl {
    val useDebugColoring: Boolean = statusBarPipelineFlags.useDebugColoring()

@@ -47,11 +50,16 @@ abstract class LocationBasedMobileViewModel(
        fun viewModelForLocation(
            commonImpl: MobileIconViewModelCommon,
            statusBarPipelineFlags: StatusBarPipelineFlags,
+            verboseMobileViewLogger: VerboseMobileViewLogger,
            loc: StatusBarLocation,
        ): LocationBasedMobileViewModel =
            when (loc) {
                StatusBarLocation.HOME ->
-                    HomeMobileIconViewModel(commonImpl, statusBarPipelineFlags)
+                    HomeMobileIconViewModel(
+                        commonImpl,
+                        statusBarPipelineFlags,
+                        verboseMobileViewLogger,
+                    )
                StatusBarLocation.KEYGUARD ->
                    KeyguardMobileIconViewModel(commonImpl, statusBarPipelineFlags)
                StatusBarLocation.QS -> QsMobileIconViewModel(commonImpl, statusBarPipelineFlags)
@@ -62,6 +70,7 @@ abstract class LocationBasedMobileViewModel(
 class HomeMobileIconViewModel(
    commonImpl: MobileIconViewModelCommon,
    statusBarPipelineFlags: StatusBarPipelineFlags,
+    verboseMobileViewLogger: VerboseMobileViewLogger,
 ) :
    MobileIconViewModelCommon,
    LocationBasedMobileViewModel(
@@ -69,6 +78,7 @@ class HomeMobileIconViewModel(
        statusBarPipelineFlags,
        debugTint = Color.CYAN,
        locationName = "Home",
+        verboseMobileViewLogger,
    )

 class QsMobileIconViewModel(
@@ -81,6 +91,8 @@ class QsMobileIconViewModel(
        statusBarPipelineFlags,
        debugTint = Color.GREEN,
        locationName = "QS",
+        // Only do verbose logging for the Home location.
+        verboseLogger = null,
    )

 class KeyguardMobileIconViewModel(
@@ -93,4 +105,6 @@ class KeyguardMobileIconViewModel(
        statusBarPipelineFlags,
        debugTint = Color.MAGENTA,
        locationName = "Keyguard",
+        // Only do verbose logging for the Home location.
+        verboseLogger = null,
    )
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt
@@ -49,7 +49,7 @@ interface MobileIconViewModelCommon {
    val contentDescription: Flow<ContentDescription>
    val roaming: Flow<Boolean>
    /** The RAT icon (LTE, 3G, 5G, etc) to be displayed. Null if we shouldn't show anything */
-    val networkTypeIcon: Flow<Icon?>
+    val networkTypeIcon: Flow<Icon.Resource?>
    val activityInVisible: Flow<Boolean>
    val activityOutVisible: Flow<Boolean>
    val activityContainerVisible: Flow<Boolean>
@@ -161,7 +161,7 @@ constructor(
            )
            .stateIn(scope, SharingStarted.WhileSubscribed(), false)

-    override val networkTypeIcon: Flow<Icon?> =
+    override val networkTypeIcon: Flow<Icon.Resource?> =
        combine(
                iconInteractor.networkTypeIconGroup,
                showNetworkTypeIcon,
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt
@@ -24,6 +24,7 @@ import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags
 import com.android.systemui.statusbar.pipeline.airplane.domain.interactor.AirplaneModeInteractor
 import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIconsInteractor
 import com.android.systemui.statusbar.pipeline.mobile.ui.MobileViewLogger
+import com.android.systemui.statusbar.pipeline.mobile.ui.VerboseMobileViewLogger
 import com.android.systemui.statusbar.pipeline.mobile.ui.view.ModernStatusBarMobileView
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
 import javax.inject.Inject
@@ -41,6 +42,7 @@ class MobileIconsViewModel
 constructor(
    val subscriptionIdsFlow: StateFlow<List<Int>>,
    val logger: MobileViewLogger,
+    private val verboseLogger: VerboseMobileViewLogger,
    private val interactor: MobileIconsInteractor,
    private val airplaneModeInteractor: AirplaneModeInteractor,
    private val constants: ConnectivityConstants,
@@ -68,6 +70,7 @@ constructor(
        return LocationBasedMobileViewModel.viewModelForLocation(
            common,
            statusBarPipelineFlags,
+            verboseLogger,
            location,
        )
    }
@@ -82,6 +85,7 @@ constructor(
    @Inject
    constructor(
        private val logger: MobileViewLogger,
+        private val verboseLogger: VerboseMobileViewLogger,
        private val interactor: MobileIconsInteractor,
        private val airplaneModeInteractor: AirplaneModeInteractor,
        private val constants: ConnectivityConstants,
@@ -92,6 +96,7 @@ constructor(
            return MobileIconsViewModel(
                subscriptionIdsFlow,
                logger,
+                verboseLogger,
                interactor,
                airplaneModeInteractor,
                constants,
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt
@@ -28,6 +28,7 @@ import com.android.systemui.statusbar.pipeline.mobile.ui.model.SignalIconModel
 import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconViewModelTest.Companion.defaultSignal
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
 import com.android.systemui.statusbar.pipeline.shared.data.repository.FakeConnectivityRepository
+import com.android.systemui.util.mockito.mock
 import com.google.common.truth.Truth.assertThat
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.launchIn
@@ -84,7 +85,7 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() {
                testScope.backgroundScope,
            )

-        homeIcon = HomeMobileIconViewModel(commonImpl, statusBarPipelineFlags)
+        homeIcon = HomeMobileIconViewModel(commonImpl, statusBarPipelineFlags, mock())
        qsIcon = QsMobileIconViewModel(commonImpl, statusBarPipelineFlags)
        keyguardIcon = KeyguardMobileIconViewModel(commonImpl, statusBarPipelineFlags)
    }
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModelTest.kt
@@ -25,6 +25,7 @@ import com.android.systemui.statusbar.pipeline.airplane.domain.interactor.Airpla
 import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel
 import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.FakeMobileIconsInteractor
 import com.android.systemui.statusbar.pipeline.mobile.ui.MobileViewLogger
+import com.android.systemui.statusbar.pipeline.mobile.ui.VerboseMobileViewLogger
 import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
 import com.android.systemui.statusbar.pipeline.shared.data.repository.FakeConnectivityRepository
@@ -53,6 +54,7 @@ class MobileIconsViewModelTest : SysuiTestCase() {
    @Mock private lateinit var statusBarPipelineFlags: StatusBarPipelineFlags
    @Mock private lateinit var constants: ConnectivityConstants
    @Mock private lateinit var logger: MobileViewLogger
+    @Mock private lateinit var verboseLogger: VerboseMobileViewLogger

    private val testDispatcher = UnconfinedTestDispatcher()
    private val testScope = TestScope(testDispatcher)
@@ -76,6 +78,7 @@ class MobileIconsViewModelTest : SysuiTestCase() {
            MobileIconsViewModel(
                subscriptionIdsFlow,
                logger,
+                verboseLogger,
                interactor,
                airplaneModeInteractor,
                constants,