From e9f3426a780eb59b03bb7a775d5486c04107a486 Mon Sep 17 00:00:00 2001 From: Caitlin Cassidy Date: Tue, 29 Jun 2021 20:47:00 +0000 Subject: [PATCH] [Ongoing call] Don't show the time if the notification's `when` value isn't valid. Test: manual + new unit tests (OngoingCallControllerTest, OngoingCallChronometerTest) Fixes: 192379214 Bug: 183229367 Change-Id: Idc5097de017ec662cf6c54b5afc196343eb0e3dc --- .../ongoingcall/OngoingCallChronometer.kt | 12 +++++-- .../ongoingcall/OngoingCallController.kt | 26 ++++++++++++---- .../ongoingcall/OngoingCallChronometerTest.kt | 20 ++++++++++++ .../ongoingcall/OngoingCallControllerTest.kt | 31 +++++++++++++++++++ 4 files changed, 81 insertions(+), 8 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallChronometer.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallChronometer.kt index 6e27caec93651..bb7ba4c4174fe 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallChronometer.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallChronometer.kt @@ -20,6 +20,7 @@ import android.content.Context import android.util.AttributeSet import android.widget.Chronometer +import androidx.annotation.UiThread /** * A [Chronometer] specifically for the ongoing call chip in the status bar. @@ -46,10 +47,10 @@ class OngoingCallChronometer @JvmOverloads constructor( // Minimum width that the text view can be. Corresponds with the largest number width seen so // far. - var minimumTextWidth: Int = 0 + private var minimumTextWidth: Int = 0 // True if the text is too long for the space available, so the text should be hidden. - var shouldHideText: Boolean = false + private var shouldHideText: Boolean = false override fun setBase(base: Long) { // These variables may have changed during the previous call, so re-set them before the new @@ -60,6 +61,13 @@ class OngoingCallChronometer @JvmOverloads constructor( super.setBase(base) } + /** Sets whether this view should hide its text or not. */ + @UiThread + fun setShouldHideText(shouldHideText: Boolean) { + this.shouldHideText = shouldHideText + requestLayout() + } + override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) { if (shouldHideText) { setMeasuredDimension(0, 0) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt index c20730e1b4a4f..16fa5da9e9794 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt @@ -172,10 +172,16 @@ class OngoingCallController @Inject constructor( currentChipView?.findViewById(R.id.ongoing_call_chip_background) if (currentChipView != null && timeView != null && backgroundView != null) { - timeView.base = currentCallNotificationInfo.callStartTime - - System.currentTimeMillis() + - systemClock.elapsedRealtime() - timeView.start() + if (currentCallNotificationInfo.hasValidStartTime()) { + timeView.setShouldHideText(false) + timeView.base = currentCallNotificationInfo.callStartTime - + systemClock.currentTimeMillis() + + systemClock.elapsedRealtime() + timeView.start() + } else { + timeView.setShouldHideText(true) + timeView.stop() + } currentCallNotificationInfo.intent?.let { intent -> currentChipView.setOnClickListener { @@ -260,7 +266,9 @@ class OngoingCallController @Inject constructor( @VisibleForTesting fun tearDownChipView() = chipView?.getTimeView()?.stop() - private fun View.getTimeView(): Chronometer? = this.findViewById(R.id.ongoing_call_chip_time) + private fun View.getTimeView(): OngoingCallChronometer? { + return this.findViewById(R.id.ongoing_call_chip_time) + } private data class CallNotificationInfo( val key: String, @@ -269,7 +277,13 @@ class OngoingCallController @Inject constructor( val uid: Int, /** True if the call is currently ongoing (as opposed to incoming, screening, etc.). */ val isOngoing: Boolean - ) + ) { + /** + * Returns true if the notification information has a valid call start time. + * See b/192379214. + */ + fun hasValidStartTime(): Boolean = callStartTime > 0 + } } private fun isCallNotification(entry: NotificationEntry): Boolean { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallChronometerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallChronometerTest.kt index e32af605955ba..c3326b2bb7a27 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallChronometerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallChronometerTest.kt @@ -132,6 +132,26 @@ class OngoingCallChronometerTest : SysuiTestCase() { assertThat(textView.measuredWidth).isGreaterThan(0) } + @Test + fun setShouldHideText_true_textHidden() { + textView.setShouldHideText(true) + measureTextView() + + assertThat(textView.measuredWidth).isEqualTo(0) + } + + @Test + fun setShouldHideText_false_textShown() { + // First, set to true so that setting it to false will definitely have an effect. + textView.setShouldHideText(true) + measureTextView() + + textView.setShouldHideText(false) + measureTextView() + + assertThat(textView.measuredWidth).isGreaterThan(0) + } + private fun setTextAndMeasure(text: String) { textView.text = text measureTextView() diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt index c81d46898ff93..d36cb0b3a7177 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt @@ -153,6 +153,37 @@ class OngoingCallControllerTest : SysuiTestCase() { createCallNotifEntry(ongoingCallStyle, nullContentIntent = true)) } + /** Regression test for b/192379214. */ + @Test + fun onEntryUpdated_notificationWhenIsZero_timeHidden() { + val notification = NotificationEntryBuilder(createOngoingCallNotifEntry()) + notification.modifyNotification(context).setWhen(0) + + notifCollectionListener.onEntryUpdated(notification.build()) + chipView.measure( + View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED), + View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED) + ) + + assertThat(chipView.findViewById(R.id.ongoing_call_chip_time)?.measuredWidth) + .isEqualTo(0) + } + + @Test + fun onEntryUpdated_notificationWhenIsValid_timeShown() { + val notification = NotificationEntryBuilder(createOngoingCallNotifEntry()) + notification.modifyNotification(context).setWhen(clock.currentTimeMillis()) + + notifCollectionListener.onEntryUpdated(notification.build()) + chipView.measure( + View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED), + View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED) + ) + + assertThat(chipView.findViewById(R.id.ongoing_call_chip_time)?.measuredWidth) + .isGreaterThan(0) + } + /** * If a call notification is never added before #onEntryRemoved is called, then the listener * should never be notified.