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

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);
+    }
 }

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<View>
+        get() = children.toList()
+}

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);
+    }
 }