diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttLogger.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttLogger.kt index 811d47ddaa29b..b565f3c22f249 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttLogger.kt @@ -18,22 +18,21 @@ package com.android.systemui.media.taptotransfer.common import com.android.systemui.log.LogBuffer import com.android.systemui.log.LogLevel +import com.android.systemui.temporarydisplay.TemporaryViewLogger /** * A logger for media tap-to-transfer events. * - * @property deviceTypeTag the type of device triggering the logs -- "Sender" or "Receiver". + * @param deviceTypeTag the type of device triggering the logs -- "Sender" or "Receiver". */ class MediaTttLogger( - private val deviceTypeTag: String, - private val buffer: LogBuffer -){ - private val bufferTag = BASE_TAG + deviceTypeTag - + deviceTypeTag: String, + buffer: LogBuffer +) : TemporaryViewLogger(buffer, BASE_TAG + deviceTypeTag) { /** Logs a change in the chip state for the given [mediaRouteId]. */ fun logStateChange(stateName: String, mediaRouteId: String, packageName: String?) { buffer.log( - bufferTag, + tag, LogLevel.DEBUG, { str1 = stateName @@ -44,25 +43,32 @@ class MediaTttLogger( ) } - /** Logs that we removed the chip for the given [reason]. */ - fun logChipRemoval(reason: String) { - buffer.log( - bufferTag, - LogLevel.DEBUG, - { str1 = reason }, - { "Chip removed due to $str1" } - ) - } - /** Logs that we couldn't find information for [packageName]. */ fun logPackageNotFound(packageName: String) { buffer.log( - bufferTag, + tag, LogLevel.DEBUG, { str1 = packageName }, { "Package $str1 could not be found" } ) } + + /** + * Logs that a removal request has been bypassed (ignored). + * + * @param removalReason the reason that the chip removal was requested. + * @param bypassReason the reason that the request was bypassed. + */ + fun logRemovalBypass(removalReason: String, bypassReason: String) { + buffer.log( + tag, + LogLevel.DEBUG, + { + str1 = removalReason + str2 = bypassReason + }, + { "Chip removal requested due to $str1; however, removal was ignored because $str2" }) + } } private const val BASE_TAG = "MediaTtt" diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttUtils.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttUtils.kt index 5db2cdc166e05..792ae7ca60491 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttUtils.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttUtils.kt @@ -29,6 +29,7 @@ class MediaTttUtils { // Used in CTS tests UpdateMediaTapToTransferSenderDisplayTest and // UpdateMediaTapToTransferReceiverDisplayTest const val WINDOW_TITLE = "Media Transfer Chip View" + const val WAKE_REASON = "MEDIA_TRANSFER_ACTIVATED" /** * Returns the information needed to display the icon. diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiver.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiver.kt index cdb3157c3b865..dfd9e22c14b14 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiver.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiver.kt @@ -62,7 +62,7 @@ class MediaTttChipControllerReceiver @Inject constructor( powerManager: PowerManager, @Main private val mainHandler: Handler, private val uiEventLogger: MediaTttReceiverUiEventLogger, -) : TemporaryViewDisplayController( +) : TemporaryViewDisplayController( context, logger, windowManager, @@ -72,6 +72,7 @@ class MediaTttChipControllerReceiver @Inject constructor( powerManager, R.layout.media_ttt_chip_receiver, MediaTttUtils.WINDOW_TITLE, + MediaTttUtils.WAKE_REASON, ) { @SuppressLint("WrongConstant") // We're allowed to use LAYOUT_IN_DISPLAY_CUTOUT_MODE_ALWAYS override val windowLayoutParams = commonWindowLayoutParams.apply { diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt index 57e2b509df74a..e539f3fd842de 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt @@ -58,7 +58,7 @@ class MediaTttChipControllerSender @Inject constructor( configurationController: ConfigurationController, powerManager: PowerManager, private val uiEventLogger: MediaTttSenderUiEventLogger -) : TemporaryViewDisplayController( +) : TemporaryViewDisplayController( context, logger, windowManager, @@ -68,6 +68,7 @@ class MediaTttChipControllerSender @Inject constructor( powerManager, R.layout.media_ttt_chip, MediaTttUtils.WINDOW_TITLE, + MediaTttUtils.WAKE_REASON, ) { override val windowLayoutParams = commonWindowLayoutParams.apply { gravity = Gravity.TOP.or(Gravity.CENTER_HORIZONTAL) @@ -105,7 +106,7 @@ class MediaTttChipControllerSender @Inject constructor( uiEventLogger.logSenderStateChange(chipState) if (chipState == ChipStateSender.FAR_FROM_RECEIVER) { - removeView(removalReason = ChipStateSender.FAR_FROM_RECEIVER::class.simpleName!!) + removeView(removalReason = ChipStateSender.FAR_FROM_RECEIVER.name) } else { displayView(ChipSenderInfo(chipState, routeInfo, undoCallback)) } @@ -179,6 +180,9 @@ class MediaTttChipControllerSender @Inject constructor( transferStatus == TransferStatus.SUCCEEDED) && removalReason != TemporaryDisplayRemovalReason.REASON_TIMEOUT ) { + logger.logRemovalBypass( + removalReason, bypassReason = "transferStatus=${transferStatus.name}" + ) return } super.removeView(removalReason) diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt index d9c77df05440a..a52e2aff52c18 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt @@ -32,7 +32,6 @@ import android.view.accessibility.AccessibilityManager.FLAG_CONTENT_ICONS import android.view.accessibility.AccessibilityManager.FLAG_CONTENT_TEXT import androidx.annotation.CallSuper import com.android.systemui.dagger.qualifiers.Main -import com.android.systemui.media.taptotransfer.common.MediaTttLogger import com.android.systemui.statusbar.policy.ConfigurationController import com.android.systemui.util.concurrency.DelayableExecutor @@ -45,14 +44,14 @@ import com.android.systemui.util.concurrency.DelayableExecutor * The generic type T is expected to contain all the information necessary for the subclasses to * display the view in a certain state, since they receive in [updateView]. * - * TODO(b/245610654): Remove all the media-specific logic from this class. - * * @property windowTitle the title to use for the window that displays the temporary view. Should be * normally cased, like "Window Title". + * @property wakeReason a string used for logging if we needed to wake the screen in order to + * display the temporary view. Should be screaming snake cased, like WAKE_REASON. */ -abstract class TemporaryViewDisplayController( +abstract class TemporaryViewDisplayController( internal val context: Context, - internal val logger: MediaTttLogger, + internal val logger: U, internal val windowManager: WindowManager, @Main private val mainExecutor: DelayableExecutor, private val accessibilityManager: AccessibilityManager, @@ -60,6 +59,7 @@ abstract class TemporaryViewDisplayController( private val powerManager: PowerManager, @LayoutRes private val viewLayoutRes: Int, private val windowTitle: String, + private val wakeReason: String, ) { /** * Window layout params that will be used as a starting point for the [windowLayoutParams] of @@ -114,10 +114,10 @@ abstract class TemporaryViewDisplayController( powerManager.wakeUp( SystemClock.uptimeMillis(), PowerManager.WAKE_REASON_APPLICATION, - "com.android.systemui:media_tap_to_transfer_activated" + "com.android.systemui:$wakeReason", ) } - + logger.logChipAddition() inflateAndUpdateView(newInfo) } diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewLogger.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewLogger.kt new file mode 100644 index 0000000000000..606a11a846860 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewLogger.kt @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2022 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.temporarydisplay + +import com.android.systemui.log.LogBuffer +import com.android.systemui.log.LogLevel + +/** A logger for temporary view changes -- see [TemporaryViewDisplayController]. */ +open class TemporaryViewLogger( + internal val buffer: LogBuffer, + internal val tag: String, +) { + /** Logs that we added the chip to a new window. */ + fun logChipAddition() { + buffer.log(tag, LogLevel.DEBUG, {}, { "Chip added" }) + } + + /** Logs that we removed the chip for the given [reason]. */ + fun logChipRemoval(reason: String) { + buffer.log(tag, LogLevel.DEBUG, { str1 = reason }, { "Chip removed due to $str1" }) + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/common/MediaTttLoggerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/common/MediaTttLoggerTest.kt index 940681544d4ef..1078cdaa57c4a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/common/MediaTttLoggerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/common/MediaTttLoggerTest.kt @@ -50,40 +50,39 @@ class MediaTttLoggerTest : SysuiTestCase() { logger.logStateChange(stateName, id, packageName) - val stringWriter = StringWriter() - buffer.dump(PrintWriter(stringWriter), tailLength = 0) - val actualString = stringWriter.toString() - + val actualString = getStringFromBuffer() assertThat(actualString).contains(DEVICE_TYPE_TAG) assertThat(actualString).contains(stateName) assertThat(actualString).contains(id) assertThat(actualString).contains(packageName) } - @Test - fun logChipRemoval_bufferHasDeviceTypeAndReason() { - val reason = "test reason" - logger.logChipRemoval(reason) - - val stringWriter = StringWriter() - buffer.dump(PrintWriter(stringWriter), tailLength = 0) - val actualString = stringWriter.toString() - - assertThat(actualString).contains(DEVICE_TYPE_TAG) - assertThat(actualString).contains(reason) - } - @Test fun logPackageNotFound_bufferHasPackageName() { val packageName = "this.is.a.package" logger.logPackageNotFound(packageName) + val actualString = getStringFromBuffer() + assertThat(actualString).contains(packageName) + } + + @Test + fun logRemovalBypass_bufferHasReasons() { + val removalReason = "fakeRemovalReason" + val bypassReason = "fakeBypassReason" + + logger.logRemovalBypass(removalReason, bypassReason) + + val actualString = getStringFromBuffer() + assertThat(actualString).contains(removalReason) + assertThat(actualString).contains(bypassReason) + } + + private fun getStringFromBuffer(): String { val stringWriter = StringWriter() buffer.dump(PrintWriter(stringWriter), tailLength = 0) - val actualString = stringWriter.toString() - - assertThat(actualString).contains(packageName) + return stringWriter.toString() } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSenderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSenderTest.kt index 05440548c754f..ff0faf98fe1ef 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSenderTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSenderTest.kt @@ -587,6 +587,7 @@ class MediaTttChipControllerSenderTest : SysuiTestCase() { fakeExecutor.runAllReady() verify(windowManager, never()).removeView(any()) + verify(logger).logRemovalBypass(any(), any()) } @Test @@ -601,6 +602,7 @@ class MediaTttChipControllerSenderTest : SysuiTestCase() { fakeExecutor.runAllReady() verify(windowManager, never()).removeView(any()) + verify(logger).logRemovalBypass(any(), any()) } @Test @@ -622,6 +624,7 @@ class MediaTttChipControllerSenderTest : SysuiTestCase() { fakeExecutor.runAllReady() verify(windowManager, never()).removeView(any()) + verify(logger).logRemovalBypass(any(), any()) } @Test @@ -646,6 +649,7 @@ class MediaTttChipControllerSenderTest : SysuiTestCase() { fakeExecutor.runAllReady() verify(windowManager, never()).removeView(any()) + verify(logger).logRemovalBypass(any(), any()) } @Test @@ -656,6 +660,7 @@ class MediaTttChipControllerSenderTest : SysuiTestCase() { fakeExecutor.runAllReady() verify(windowManager, never()).removeView(any()) + verify(logger).logRemovalBypass(any(), any()) } @Test @@ -680,6 +685,7 @@ class MediaTttChipControllerSenderTest : SysuiTestCase() { fakeExecutor.runAllReady() verify(windowManager, never()).removeView(any()) + verify(logger).logRemovalBypass(any(), any()) } @Test @@ -690,6 +696,7 @@ class MediaTttChipControllerSenderTest : SysuiTestCase() { fakeExecutor.runAllReady() verify(windowManager, never()).removeView(any()) + verify(logger).logRemovalBypass(any(), any()) } @Test @@ -714,6 +721,7 @@ class MediaTttChipControllerSenderTest : SysuiTestCase() { fakeExecutor.runAllReady() verify(windowManager, never()).removeView(any()) + verify(logger).logRemovalBypass(any(), any()) } private fun ViewGroup.getAppIconView() = this.requireViewById(R.id.app_icon) diff --git a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt index fbc4fee4fc079..921b7efc38eba 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayControllerTest.kt @@ -25,7 +25,6 @@ import androidx.test.filters.SmallTest import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.dagger.qualifiers.Main -import com.android.systemui.media.taptotransfer.common.MediaTttLogger import com.android.systemui.statusbar.policy.ConfigurationController import com.android.systemui.statusbar.policy.ConfigurationController.ConfigurationListener import com.android.systemui.util.concurrency.DelayableExecutor @@ -52,7 +51,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { private lateinit var fakeExecutor: FakeExecutor @Mock - private lateinit var logger: MediaTttLogger + private lateinit var logger: TemporaryViewLogger @Mock private lateinit var accessibilityManager: AccessibilityManager @Mock @@ -216,13 +215,13 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { inner class TestController( context: Context, - logger: MediaTttLogger, + logger: TemporaryViewLogger, windowManager: WindowManager, @Main mainExecutor: DelayableExecutor, accessibilityManager: AccessibilityManager, configurationController: ConfigurationController, powerManager: PowerManager, - ) : TemporaryViewDisplayController( + ) : TemporaryViewDisplayController( context, logger, windowManager, @@ -232,6 +231,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() { powerManager, R.layout.media_ttt_chip, "Window Title", + "WAKE_REASON", ) { var mostRecentViewInfo: ViewInfo? = null diff --git a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewLoggerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewLoggerTest.kt new file mode 100644 index 0000000000000..c9f2b4db81ef5 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/TemporaryViewLoggerTest.kt @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2022 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.temporarydisplay + +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.dump.DumpManager +import com.android.systemui.log.LogBuffer +import com.android.systemui.log.LogBufferFactory +import com.android.systemui.log.LogcatEchoTracker +import com.google.common.truth.Truth.assertThat +import java.io.PrintWriter +import java.io.StringWriter +import org.junit.Before +import org.junit.Test +import org.mockito.Mockito + +@SmallTest +class TemporaryViewLoggerTest : SysuiTestCase() { + private lateinit var buffer: LogBuffer + private lateinit var logger: TemporaryViewLogger + + @Before + fun setUp() { + buffer = + LogBufferFactory(DumpManager(), Mockito.mock(LogcatEchoTracker::class.java)) + .create("buffer", 10) + logger = TemporaryViewLogger(buffer, TAG) + } + + @Test + fun logChipAddition_bufferHasLog() { + logger.logChipAddition() + + val stringWriter = StringWriter() + buffer.dump(PrintWriter(stringWriter), tailLength = 0) + val actualString = stringWriter.toString() + + assertThat(actualString).contains(TAG) + } + + @Test + fun logChipRemoval_bufferHasTagAndReason() { + val reason = "test reason" + logger.logChipRemoval(reason) + + val stringWriter = StringWriter() + buffer.dump(PrintWriter(stringWriter), tailLength = 0) + val actualString = stringWriter.toString() + + assertThat(actualString).contains(TAG) + assertThat(actualString).contains(reason) + } +} + +private const val TAG = "TestTag"