Merge changes Idb59c0c2,Idb29d25d,I07f68742 into tm-qpr-dev
* changes: [Media TTT] Improve our logs around chip removal. [Chipbar] Replace the media-specific logger with a generic TemporaryViewLogger. [Chipbar] Remove the media-specific wake reason from TemporaryViewDisplayController.
This commit is contained in:
committed by
Android (Google) Code Review
commit
a527bb977d
@@ -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"
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -62,7 +62,7 @@ class MediaTttChipControllerReceiver @Inject constructor(
|
||||
powerManager: PowerManager,
|
||||
@Main private val mainHandler: Handler,
|
||||
private val uiEventLogger: MediaTttReceiverUiEventLogger,
|
||||
) : TemporaryViewDisplayController<ChipReceiverInfo>(
|
||||
) : TemporaryViewDisplayController<ChipReceiverInfo, MediaTttLogger>(
|
||||
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 {
|
||||
|
||||
@@ -58,7 +58,7 @@ class MediaTttChipControllerSender @Inject constructor(
|
||||
configurationController: ConfigurationController,
|
||||
powerManager: PowerManager,
|
||||
private val uiEventLogger: MediaTttSenderUiEventLogger
|
||||
) : TemporaryViewDisplayController<ChipSenderInfo>(
|
||||
) : TemporaryViewDisplayController<ChipSenderInfo, MediaTttLogger>(
|
||||
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)
|
||||
|
||||
@@ -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 <T> 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<T : TemporaryViewInfo>(
|
||||
abstract class TemporaryViewDisplayController<T : TemporaryViewInfo, U : TemporaryViewLogger>(
|
||||
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<T : TemporaryViewInfo>(
|
||||
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<T : TemporaryViewInfo>(
|
||||
powerManager.wakeUp(
|
||||
SystemClock.uptimeMillis(),
|
||||
PowerManager.WAKE_REASON_APPLICATION,
|
||||
"com.android.systemui:media_tap_to_transfer_activated"
|
||||
"com.android.systemui:$wakeReason",
|
||||
)
|
||||
}
|
||||
|
||||
logger.logChipAddition()
|
||||
inflateAndUpdateView(newInfo)
|
||||
}
|
||||
|
||||
|
||||
@@ -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" })
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<ImageView>(R.id.app_icon)
|
||||
|
||||
@@ -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<ViewInfo>(
|
||||
) : TemporaryViewDisplayController<ViewInfo, TemporaryViewLogger>(
|
||||
context,
|
||||
logger,
|
||||
windowManager,
|
||||
@@ -232,6 +231,7 @@ class TemporaryViewDisplayControllerTest : SysuiTestCase() {
|
||||
powerManager,
|
||||
R.layout.media_ttt_chip,
|
||||
"Window Title",
|
||||
"WAKE_REASON",
|
||||
) {
|
||||
var mostRecentViewInfo: ViewInfo? = null
|
||||
|
||||
|
||||
@@ -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"
|
||||
Reference in New Issue
Block a user