From 8b8aa7d66e16d0dc5fd7f25b9b43bcd34e86f3bb Mon Sep 17 00:00:00 2001 From: Michal Brzezinski Date: Tue, 6 Dec 2022 18:10:44 +0000 Subject: [PATCH] Optimistic fix for tile not getting redistributed in split shade The issue is likely caused by QS tiles not getting redistributed in PagedTileLayout#onConfigurationChanged which does it only when orientation changes. One fix would be to remove that condition at all but then we would be redistributing for every configuration change or we'd need to make PagedTileLayout aware of split shade - none of these options seem good. So the solution here is to force redistribution from QSPanelControllerBase which is already aware of split shade and can listen to changes there. I'll also add more logs (in case bug is not fixed) in follow-up CL. Bug: 255208946 Test: there is no reliable way to reproduce the issue, we'll see if it stops happening Test: QSPanelControllerTest Change-Id: I81630b6eec16237a1a665982553dfc5b9dbac54b --- .../android/systemui/qs/PagedTileLayout.java | 8 +++++++ .../systemui/qs/QSPanelController.java | 5 ++++ .../systemui/qs/QSPanelControllerBase.java | 14 +++++------ .../systemui/qs/QSPanelControllerTest.kt | 24 +++++++++++++++++-- 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/qs/PagedTileLayout.java b/packages/SystemUI/src/com/android/systemui/qs/PagedTileLayout.java index f92bbf75d0273..8ceee1a950eae 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/PagedTileLayout.java +++ b/packages/SystemUI/src/com/android/systemui/qs/PagedTileLayout.java @@ -741,6 +741,14 @@ public class PagedTileLayout extends ViewPager implements QSTileLayout { } }; + /** + * Force all tiles to be redistributed across pages. + * Should be called when one of the following changes: rows, columns, number of tiles. + */ + public void forceTilesRedistribution() { + mDistributeTiles = true; + } + public interface PageListener { int INVALID_PAGE = -1; diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSPanelController.java b/packages/SystemUI/src/com/android/systemui/qs/QSPanelController.java index 1827eaf3fad12..b2ca6b728113d 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSPanelController.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSPanelController.java @@ -148,6 +148,11 @@ public class QSPanelController extends QSPanelControllerBase { } } + @Override + protected void onSplitShadeChanged() { + ((PagedTileLayout) mView.getOrCreateTileLayout()).forceTilesRedistribution(); + } + /** */ public void setVisibility(int visibility) { mView.setVisibility(visibility); diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java b/packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java index dd88c83949fbb..60d2c177c7cd3 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSPanelControllerBase.java @@ -96,17 +96,23 @@ public abstract class QSPanelControllerBase extends ViewContr /* newOrientation= */ newConfig.orientation, /* containerName= */ mView.getDumpableTag()); + boolean previousSplitShadeState = mShouldUseSplitNotificationShade; mShouldUseSplitNotificationShade = LargeScreenUtils.shouldUseSplitNotificationShade(getResources()); mLastOrientation = newConfig.orientation; switchTileLayoutIfNeeded(); onConfigurationChanged(); + if (previousSplitShadeState != mShouldUseSplitNotificationShade) { + onSplitShadeChanged(); + } } }; protected void onConfigurationChanged() { } + protected void onSplitShadeChanged() { } + private final Function1 mMediaHostVisibilityListener = (visible) -> { if (mMediaVisibilityChangedListener != null) { mMediaVisibilityChangedListener.accept(visible); @@ -264,14 +270,6 @@ public abstract class QSPanelControllerBase extends ViewContr } } } - protected QSTile getTile(String subPanel) { - for (int i = 0; i < mRecords.size(); i++) { - if (subPanel.equals(mRecords.get(i).tile.getTileSpec())) { - return mRecords.get(i).tile; - } - } - return mHost.createTile(subPanel); - } boolean areThereTiles() { return !mRecords.isEmpty(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.kt index 9f28708a388e1..5e082f686ea3e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelControllerTest.kt @@ -1,9 +1,12 @@ package com.android.systemui.qs +import android.content.res.Configuration import android.test.suitebuilder.annotation.SmallTest import android.testing.AndroidTestingRunner +import android.testing.TestableResources import com.android.internal.logging.MetricsLogger import com.android.internal.logging.UiEventLogger +import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager import com.android.systemui.flags.FeatureFlags @@ -26,10 +29,11 @@ import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.Mock import org.mockito.Mockito import org.mockito.Mockito.any +import org.mockito.Mockito.never import org.mockito.Mockito.reset import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` as whenever import org.mockito.MockitoAnnotations +import org.mockito.Mockito.`when` as whenever @SmallTest @RunWith(AndroidTestingRunner::class) @@ -54,8 +58,11 @@ class QSPanelControllerTest : SysuiTestCase() { @Mock private lateinit var otherTile: QSTile @Mock private lateinit var statusBarKeyguardViewManager: StatusBarKeyguardViewManager @Mock private lateinit var featureFlags: FeatureFlags + @Mock private lateinit var configuration: Configuration + @Mock private lateinit var pagedTileLayout: PagedTileLayout private lateinit var controller: QSPanelController + private val testableResources: TestableResources = mContext.orCreateTestableResources @Before fun setUp() { @@ -63,7 +70,9 @@ class QSPanelControllerTest : SysuiTestCase() { whenever(brightnessSliderFactory.create(any(), any())).thenReturn(brightnessSlider) whenever(brightnessControllerFactory.create(any())).thenReturn(brightnessController) - whenever(qsPanel.resources).thenReturn(mContext.orCreateTestableResources.resources) + testableResources.addOverride(R.bool.config_use_split_notification_shade, false) + whenever(qsPanel.resources).thenReturn(testableResources.resources) + whenever(qsPanel.getOrCreateTileLayout()).thenReturn(pagedTileLayout) whenever(statusBarKeyguardViewManager.isPrimaryBouncerInTransit()).thenReturn(false) whenever(qsPanel.setListening(anyBoolean())).then { whenever(qsPanel.isListening).thenReturn(it.getArgument(0)) @@ -121,4 +130,15 @@ class QSPanelControllerTest : SysuiTestCase() { whenever(statusBarKeyguardViewManager.isPrimaryBouncerInTransit()).thenReturn(false) assertThat(controller.isBouncerInTransit()).isEqualTo(false) } + + @Test + fun configurationChange_onlySplitShadeConfigChanges_tileAreRedistributed() { + testableResources.addOverride(R.bool.config_use_split_notification_shade, false) + controller.mOnConfigurationChangedListener.onConfigurationChange(configuration) + verify(pagedTileLayout, never()).forceTilesRedistribution() + + testableResources.addOverride(R.bool.config_use_split_notification_shade, true) + controller.mOnConfigurationChangedListener.onConfigurationChange(configuration) + verify(pagedTileLayout).forceTilesRedistribution() + } }