From 0ef21c14946f69dd4e69310783fbec982075c6f4 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Fri, 30 Jul 2021 15:01:14 -0400 Subject: [PATCH] Fix index when moving SecurityFooter When moving the security footer in the same container, correct the index if it's before the desired position (because it will be removed from the current position). Also, extract moving logic so it can be easily tested. Fixes: 195104944 Test: atest QSPanelTest QSPanelSwitchToParentTest Change-Id: Ibc85cd5501010fcf9752d8af103500479c6f28c2 Merged-In: Ibc85cd5501010fcf9752d8af103500479c6f28c2 --- .../src/com/android/systemui/qs/QSPanel.java | 41 ++++- .../systemui/qs/QSPanelSwitchToParentTest.kt | 162 ++++++++++++++++++ .../com/android/systemui/qs/QSPanelTest.java | 81 +++++++-- 3 files changed, 267 insertions(+), 17 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelSwitchToParentTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java b/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java index 425bdc24fe1b1..1b4ee734a4911 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QSPanel.java @@ -27,12 +27,15 @@ import android.os.Bundle; import android.os.Handler; import android.os.Message; import android.util.AttributeSet; +import android.util.Log; import android.view.Gravity; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; import android.widget.LinearLayout; +import androidx.annotation.VisibleForTesting; + import com.android.internal.logging.UiEventLogger; import com.android.internal.widget.RemeasuringLinearLayout; import com.android.systemui.R; @@ -386,19 +389,18 @@ public class QSPanel extends LinearLayout implements Tunable { if (mediaView != null) { index = indexOfChild(mediaView); } + if (mSecurityFooter.getParent() == this && indexOfChild(mSecurityFooter) < index) { + // When we remove the securityFooter to rearrange, the index of media will go + // down by one, so we correct it + index--; + } switchToParent(mSecurityFooter, this, index); } } } private void switchToParent(View child, ViewGroup parent, int index) { - ViewGroup currentParent = (ViewGroup) child.getParent(); - if (currentParent != parent || currentParent.indexOfChild(child) != index) { - if (currentParent != null) { - currentParent.removeView(child); - } - parent.addView(child, index); - } + switchToParent(child, parent, index, getDumpableTag()); } /** Call when orientation has changed and MediaHost needs to be adjusted. */ @@ -767,4 +769,29 @@ public class QSPanel extends LinearLayout implements Tunable { interface OnConfigurationChangedListener { void onConfigurationChange(Configuration newConfig); } + + @VisibleForTesting + static void switchToParent(View child, ViewGroup parent, int index, String tag) { + if (parent == null) { + Log.w(tag, "Trying to move view to null parent", + new IllegalStateException()); + return; + } + ViewGroup currentParent = (ViewGroup) child.getParent(); + if (currentParent != parent) { + if (currentParent != null) { + currentParent.removeView(child); + } + parent.addView(child, index); + return; + } + // Same parent, we are just changing indices + int currentIndex = parent.indexOfChild(child); + if (currentIndex == index) { + // We want to be in the same place. Nothing to do here + return; + } + parent.removeView(child); + parent.addView(child, index); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelSwitchToParentTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelSwitchToParentTest.kt new file mode 100644 index 0000000000000..56f2905e834f6 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelSwitchToParentTest.kt @@ -0,0 +1,162 @@ +/* + * Copyright (C) 2021 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.qs + +import com.google.common.truth.Truth.assertThat + +import androidx.test.filters.SmallTest + +import android.testing.AndroidTestingRunner +import android.view.View +import android.view.ViewGroup +import android.widget.FrameLayout +import com.android.systemui.SysuiTestCase +import com.android.systemui.util.children +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidTestingRunner::class) +@SmallTest +class QSPanelSwitchToParentTest : SysuiTestCase() { + + private lateinit var parent1: FrameLayout + private lateinit var parent2: FrameLayout + + private lateinit var movingView: View + + private lateinit var view1A: View + private lateinit var view1B: View + private lateinit var view1C: View + + private lateinit var view2A: View + private lateinit var view2B: View + private lateinit var view2C: View + + @Before + fun setUp() { + parent1 = FrameLayout(mContext) + parent2 = FrameLayout(mContext) + + movingView = View(mContext) + + view1A = View(mContext) + parent1.addView(view1A) + view1B = View(mContext) + parent1.addView(view1B) + view1C = View(mContext) + parent1.addView(view1C) + + view2A = View(mContext) + parent2.addView(view2A) + view2B = View(mContext) + parent2.addView(view2B) + view2C = View(mContext) + parent2.addView(view2C) + } + + @Test + fun testNullTargetNoInteractions() { + QSPanel.switchToParent(movingView, null, -1, "") + + assertThat(movingView.parent).isNull() + } + + @Test + fun testMoveToEndNoParent() { + QSPanel.switchToParent(movingView, parent2, -1, "") + + assertThat(parent1.childrenList).containsExactly( + view1A, view1B, view1C + ) + + assertThat(parent2.childrenList).containsExactly( + view2A, view2B, view2C, movingView + ) + } + + @Test + fun testMoveToEndDifferentParent() { + parent1.addView(movingView, 0) + + QSPanel.switchToParent(movingView, parent2, -1, "") + + assertThat(parent1.childrenList).containsExactly( + view1A, view1B, view1C + ) + assertThat(parent2.childrenList).containsExactly( + view2A, view2B, view2C, movingView + ) + } + + @Test + fun testMoveToEndSameParent() { + parent2.addView(movingView, 0) + + QSPanel.switchToParent(movingView, parent2, -1, "") + + assertThat(parent1.childrenList).containsExactly( + view1A, view1B, view1C + ) + assertThat(parent2.childrenList).containsExactly( + view2A, view2B, view2C, movingView + ) + } + + @Test + fun testMoveToMiddleFromNoParent() { + QSPanel.switchToParent(movingView, parent2, 1, "") + + assertThat(parent1.childrenList).containsExactly( + view1A, view1B, view1C + ) + assertThat(parent2.childrenList).containsExactly( + view2A, movingView, view2B, view2C + ) + } + + @Test + fun testMoveToMiddleDifferentParent() { + parent1.addView(movingView, 1) + + QSPanel.switchToParent(movingView, parent2, 2, "") + + assertThat(parent1.childrenList).containsExactly( + view1A, view1B, view1C + ) + assertThat(parent2.childrenList).containsExactly( + view2A, view2B, movingView, view2C + ) + } + + @Test + fun testMoveToMiddleSameParent() { + parent2.addView(movingView, 0) + + QSPanel.switchToParent(movingView, parent2, 1, "") + + assertThat(parent1.childrenList).containsExactly( + view1A, view1B, view1C + ) + assertThat(parent2.childrenList).containsExactly( + view2A, movingView, view2B, view2C + ) + } + + private val ViewGroup.childrenList: List + get() = children.toList() +} \ No newline at end of file diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.java index 4f8859927d06a..16d4dddd67aab 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QSPanelTest.java @@ -14,6 +14,8 @@ package com.android.systemui.qs; +import static junit.framework.Assert.assertEquals; + import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; @@ -21,19 +23,20 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.content.res.Configuration; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; +import android.view.View; import android.view.ViewGroup; import android.widget.FrameLayout; import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; -import com.android.systemui.plugins.ActivityStarter; import com.android.systemui.plugins.qs.QSTileView; -import com.android.systemui.qs.logging.QSLogger; import com.android.systemui.qs.tileimpl.QSTileImpl; +import com.android.systemui.util.animation.UniqueObjectHostView; import org.junit.Before; import org.junit.Test; @@ -56,29 +59,28 @@ public class QSPanelTest extends SysuiTestCase { private QSTileImpl dndTile; @Mock private QSPanelControllerBase.TileRecord mDndTileRecord; - @Mock - private QSLogger mQSLogger; private ViewGroup mParentView; @Mock private QSDetail.Callback mCallback; @Mock private QSTileView mQSTileView; + + private UniqueObjectHostView mMediaView; + private View mSecurityFooter; @Mock - private ActivityStarter mActivityStarter; + private FrameLayout mHeaderContainer; @Before public void setup() throws Exception { MockitoAnnotations.initMocks(this); mTestableLooper = TestableLooper.get(this); -// // Dependencies for QSSecurityFooter -// mDependency.injectTestDependency(ActivityStarter.class, mActivityStarter); -// mDependency.injectMockDependency(SecurityController.class); -// mDependency.injectTestDependency(Dependency.BG_LOOPER, mTestableLooper.getLooper()); -// mContext.addMockSystemService(Context.USER_SERVICE, mock(UserManager.class)); mDndTileRecord.tile = dndTile; mDndTileRecord.tileView = mQSTileView; + mMediaView = new UniqueObjectHostView(mContext); + mSecurityFooter = new View(mContext); + mTestableLooper.runWithLooper(() -> { mQsPanel = new QSPanel(mContext, null); mQsPanel.initialize(); @@ -92,6 +94,7 @@ public class QSPanelTest extends SysuiTestCase { when(mHost.createTileView(any(), any(), anyBoolean())).thenReturn(mQSTileView); mQsPanel.addTile(mDndTileRecord); mQsPanel.setCallback(mCallback); + mQsPanel.setHeaderContainer(mHeaderContainer); }); } @@ -112,4 +115,62 @@ public class QSPanelTest extends SysuiTestCase { verify(mCallback, never()).onShowingDetail(any(), anyInt(), anyInt()); } + + @Test + public void testSecurityFooterAtEndNoMedia_portrait() { + mTestableLooper.processAllMessages(); + + mContext.getResources().getConfiguration().orientation = Configuration.ORIENTATION_PORTRAIT; + + mQsPanel.setSecurityFooter(mSecurityFooter); + + int children = mQsPanel.getChildCount(); + assertEquals(children - 1, mQsPanel.indexOfChild(mSecurityFooter)); + } + + @Test + public void testSecurityFooterRightBeforeMedia_portrait() { + mTestableLooper.processAllMessages(); + + mContext.getResources().getConfiguration().orientation = Configuration.ORIENTATION_PORTRAIT; + + mQsPanel.addView(mMediaView); + + mQsPanel.setSecurityFooter(mSecurityFooter); + + int securityFooterIndex = mQsPanel.indexOfChild(mSecurityFooter); + int mediaIndex = mQsPanel.indexOfChild(mMediaView); + + assertEquals(mediaIndex - 1, securityFooterIndex); + } + + @Test + public void testSecurityFooterRightBeforeMedia_portrait_configChange() { + mTestableLooper.processAllMessages(); + + mContext.getResources().getConfiguration().orientation = Configuration.ORIENTATION_PORTRAIT; + + mQsPanel.addView(mMediaView); + + mQsPanel.setSecurityFooter(mSecurityFooter); + + mQsPanel.onConfigurationChanged(mContext.getResources().getConfiguration()); + + int securityFooterIndex = mQsPanel.indexOfChild(mSecurityFooter); + int mediaIndex = mQsPanel.indexOfChild(mMediaView); + + assertEquals(mediaIndex - 1, securityFooterIndex); + } + + @Test + public void testSecurityFooterInHeader_landscape() { + mTestableLooper.processAllMessages(); + + mContext.getResources().getConfiguration().orientation = + Configuration.ORIENTATION_LANDSCAPE; + + mQsPanel.setSecurityFooter(mSecurityFooter); + + verify(mHeaderContainer).addView(mSecurityFooter, 0); + } }