From 692e1096459cb80e26bfb106594fea6c5f2b9422 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Go=CC=88llner?= Date: Mon, 15 Aug 2022 12:18:31 +0200 Subject: [PATCH] Fix media having wrong expansion on QQS/landscape after fold/unfold The boolean to decide whether media should be collapsed on qqs landscape comes from a boolean resource config. This resource has different values for different resource qualifiers. The boolean resource was being read once through Dagger during initialization. The problem is that the dagger component for QS is not being recreated during fold/unfold, and we keep using the out of date resource value. The solution is to inject a provider of that boolean, instead of the boolean itself, to make sure we always get the latest value. Test: QuickQSPanelControllerTest.kt Test: Manually Fixes: 234574920 Change-Id: I22fef5d865b2eeb6bf4421ff1e5cbfde592f21ce --- .../systemui/qs/QuickQSPanelController.java | 12 +- .../systemui/qs/QuickQSPanelControllerTest.kt | 104 ++++++++++-------- 2 files changed, 67 insertions(+), 49 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/QuickQSPanelController.java b/packages/SystemUI/src/com/android/systemui/qs/QuickQSPanelController.java index 833573dd38730..be4420234c294 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QuickQSPanelController.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QuickQSPanelController.java @@ -39,6 +39,7 @@ import java.util.List; import javax.inject.Inject; import javax.inject.Named; +import javax.inject.Provider; /** Controller for {@link QuickQSPanel}. */ @QSScope @@ -52,20 +53,21 @@ public class QuickQSPanelController extends QSPanelControllerBase } }; - private final boolean mUsingCollapsedLandscapeMedia; + private final Provider mUsingCollapsedLandscapeMediaProvider; @Inject QuickQSPanelController(QuickQSPanel view, QSTileHost qsTileHost, QSCustomizerController qsCustomizerController, @Named(QS_USING_MEDIA_PLAYER) boolean usingMediaPlayer, @Named(QUICK_QS_PANEL) MediaHost mediaHost, - @Named(QS_USING_COLLAPSED_LANDSCAPE_MEDIA) boolean usingCollapsedLandscapeMedia, + @Named(QS_USING_COLLAPSED_LANDSCAPE_MEDIA) + Provider usingCollapsedLandscapeMediaProvider, MetricsLogger metricsLogger, UiEventLogger uiEventLogger, QSLogger qsLogger, DumpManager dumpManager ) { super(view, qsTileHost, qsCustomizerController, usingMediaPlayer, mediaHost, metricsLogger, uiEventLogger, qsLogger, dumpManager); - mUsingCollapsedLandscapeMedia = usingCollapsedLandscapeMedia; + mUsingCollapsedLandscapeMediaProvider = usingCollapsedLandscapeMediaProvider; } @Override @@ -80,7 +82,8 @@ public class QuickQSPanelController extends QSPanelControllerBase int rotation = getRotation(); boolean isLandscape = rotation == RotationUtils.ROTATION_LANDSCAPE || rotation == RotationUtils.ROTATION_SEASCAPE; - if (!mUsingCollapsedLandscapeMedia || !isLandscape) { + boolean usingCollapsedLandscapeMedia = mUsingCollapsedLandscapeMediaProvider.get(); + if (!usingCollapsedLandscapeMedia || !isLandscape) { mMediaHost.setExpansion(MediaHost.EXPANDED); } else { mMediaHost.setExpansion(MediaHost.COLLAPSED); @@ -126,7 +129,6 @@ public class QuickQSPanelController extends QSPanelControllerBase super.setTiles(tiles, /* collapsedView */ true); } - /** */ public void setContentMargins(int marginStart, int marginEnd) { mView.setContentMargins(marginStart, marginEnd, mMediaHost.getHostView()); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QuickQSPanelControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/QuickQSPanelControllerTest.kt index 1f28210acc64a..e4f47fd16be7d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QuickQSPanelControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QuickQSPanelControllerTest.kt @@ -17,8 +17,8 @@ package com.android.systemui.qs import android.content.res.Configuration -import android.test.suitebuilder.annotation.SmallTest import android.testing.AndroidTestingRunner +import androidx.test.filters.SmallTest import com.android.internal.logging.MetricsLogger import com.android.internal.logging.testing.UiEventLoggerFake import com.android.systemui.SysuiTestCase @@ -38,38 +38,32 @@ import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.Captor import org.mockito.Mock -import org.mockito.Mockito.`when` import org.mockito.Mockito.any +import org.mockito.Mockito.reset import org.mockito.Mockito.times import org.mockito.Mockito.verify +import org.mockito.Mockito.`when` as whenever import org.mockito.MockitoAnnotations @SmallTest @RunWith(AndroidTestingRunner::class) class QuickQSPanelControllerTest : SysuiTestCase() { - @Mock - private lateinit var quickQSPanel: QuickQSPanel - @Mock - private lateinit var qsTileHost: QSTileHost - @Mock - private lateinit var qsCustomizerController: QSCustomizerController - @Mock - private lateinit var mediaHost: MediaHost - @Mock - private lateinit var metricsLogger: MetricsLogger + @Mock private lateinit var quickQSPanel: QuickQSPanel + @Mock private lateinit var qsTileHost: QSTileHost + @Mock private lateinit var qsCustomizerController: QSCustomizerController + @Mock private lateinit var mediaHost: MediaHost + @Mock private lateinit var metricsLogger: MetricsLogger + @Mock private lateinit var qsLogger: QSLogger + @Mock private lateinit var tile: QSTile + @Mock private lateinit var tileLayout: TileLayout + @Mock private lateinit var tileView: QSTileView + @Captor private lateinit var captor: ArgumentCaptor + private val uiEventLogger = UiEventLoggerFake() - @Mock - private lateinit var qsLogger: QSLogger private val dumpManager = DumpManager() - @Mock - private lateinit var tile: QSTile - @Mock - private lateinit var tileLayout: TileLayout - @Mock - private lateinit var tileView: QSTileView - @Captor - private lateinit var captor: ArgumentCaptor + + private var usingCollapsedLandscapeMedia = true private lateinit var controller: TestQuickQSPanelController @@ -77,24 +71,24 @@ class QuickQSPanelControllerTest : SysuiTestCase() { fun setUp() { MockitoAnnotations.initMocks(this) - `when`(quickQSPanel.tileLayout).thenReturn(tileLayout) - `when`(quickQSPanel.isAttachedToWindow).thenReturn(true) - `when`(quickQSPanel.dumpableTag).thenReturn("") - `when`(quickQSPanel.resources).thenReturn(mContext.resources) - `when`(qsTileHost.createTileView(any(), any(), anyBoolean())).thenReturn(tileView) + whenever(quickQSPanel.tileLayout).thenReturn(tileLayout) + whenever(quickQSPanel.isAttachedToWindow).thenReturn(true) + whenever(quickQSPanel.dumpableTag).thenReturn("") + whenever(quickQSPanel.resources).thenReturn(mContext.resources) + whenever(qsTileHost.createTileView(any(), any(), anyBoolean())).thenReturn(tileView) - controller = TestQuickQSPanelController( + controller = + TestQuickQSPanelController( quickQSPanel, qsTileHost, qsCustomizerController, - false, + /* usingMediaPlayer = */ false, mediaHost, - true, + { usingCollapsedLandscapeMedia }, metricsLogger, uiEventLogger, qsLogger, - dumpManager - ) + dumpManager) controller.init() } @@ -106,9 +100,9 @@ class QuickQSPanelControllerTest : SysuiTestCase() { @Test fun testTileSublistWithFewerTiles_noCrash() { - `when`(quickQSPanel.numQuickTiles).thenReturn(3) + whenever(quickQSPanel.numQuickTiles).thenReturn(3) - `when`(qsTileHost.tiles).thenReturn(listOf(tile, tile)) + whenever(qsTileHost.tiles).thenReturn(listOf(tile, tile)) controller.setTiles() } @@ -116,8 +110,8 @@ class QuickQSPanelControllerTest : SysuiTestCase() { @Test fun testTileSublistWithTooManyTiles() { val limit = 3 - `when`(quickQSPanel.numQuickTiles).thenReturn(limit) - `when`(qsTileHost.tiles).thenReturn(listOf(tile, tile, tile, tile)) + whenever(quickQSPanel.numQuickTiles).thenReturn(limit) + whenever(qsTileHost.tiles).thenReturn(listOf(tile, tile, tile, tile)) controller.setTiles() @@ -125,39 +119,61 @@ class QuickQSPanelControllerTest : SysuiTestCase() { } @Test - fun testMediaExpansionUpdatedWhenConfigurationChanged() { + fun mediaExpansion_afterConfigChange_inLandscape_collapsedInLandscapeTrue_updatesToCollapsed() { // times(2) because both controller and base controller are registering their listeners verify(quickQSPanel, times(2)).addOnConfigurationChangedListener(captor.capture()) // verify that media starts in the expanded state by default verify(mediaHost).expansion = MediaHostState.EXPANDED - // Rotate device, verify media size updated + // Rotate device, verify media size updated to collapsed + usingCollapsedLandscapeMedia = true controller.setRotation(RotationUtils.ROTATION_LANDSCAPE) captor.allValues.forEach { it.onConfigurationChange(Configuration.EMPTY) } verify(mediaHost).expansion = MediaHostState.COLLAPSED } + @Test + fun mediaExpansion_afterConfigChange_landscape_collapsedInLandscapeFalse_remainsExpanded() { + // times(2) because both controller and base controller are registering their listeners + verify(quickQSPanel, times(2)).addOnConfigurationChangedListener(captor.capture()) + reset(mediaHost) + + usingCollapsedLandscapeMedia = false + controller.setRotation(RotationUtils.ROTATION_LANDSCAPE) + captor.allValues.forEach { it.onConfigurationChange(Configuration.EMPTY) } + + verify(mediaHost).expansion = MediaHostState.EXPANDED + } + class TestQuickQSPanelController( view: QuickQSPanel, qsTileHost: QSTileHost, qsCustomizerController: QSCustomizerController, usingMediaPlayer: Boolean, mediaHost: MediaHost, - usingCollapsedLandscapeMedia: Boolean, + usingCollapsedLandscapeMedia: () -> Boolean, metricsLogger: MetricsLogger, uiEventLogger: UiEventLoggerFake, qsLogger: QSLogger, dumpManager: DumpManager - ) : QuickQSPanelController(view, qsTileHost, qsCustomizerController, usingMediaPlayer, - mediaHost, usingCollapsedLandscapeMedia, metricsLogger, uiEventLogger, qsLogger, - dumpManager) { + ) : + QuickQSPanelController( + view, + qsTileHost, + qsCustomizerController, + usingMediaPlayer, + mediaHost, + usingCollapsedLandscapeMedia, + metricsLogger, + uiEventLogger, + qsLogger, + dumpManager) { private var rotation = RotationUtils.ROTATION_NONE - @Override - override fun getRotation(): Int = rotation + @Override override fun getRotation(): Int = rotation fun setRotation(newRotation: Int) { rotation = newRotation