From 1ab32a7942e52b9b198c835d0cbbe68932bb5249 Mon Sep 17 00:00:00 2001
From: Jeremy Sim <jeremysim@google.com>
Date: Fri, 24 May 2024 16:48:14 -0700
Subject: [PATCH] Address jank and memory issues with OverviewActionsView

Attempt to improve jank and memory regressions by simplifying the view layout of OverviewActionsView.

Bug: 339688091
Bug: 339853741
Test: testSaveAppPairMenuItemOrActionExistsOnSplitPair()
Flag: EXEMPT bugfix
Change-Id: Ib4a7c5a813b221c0a083144e7fb1e1c5f3fedb1c
---
 .../res/layout/overview_actions_container.xml | 26 +++++++------------
 .../res/layout/overview_actions_container.xml | 24 +++++++----------
 .../quickstep/views/OverviewActionsView.java  | 24 +++++++++--------
 .../quickstep/TaplTestsSplitscreen.java       |  2 --
 .../android/launcher3/tapl/BaseOverview.java  | 19 ++------------
 .../launcher3/tapl/OverviewActions.java       |  9 -------
 6 files changed, 33 insertions(+), 71 deletions(-)

diff --git a/go/quickstep/res/layout/overview_actions_container.xml b/go/quickstep/res/layout/overview_actions_container.xml
index b1a62022e9..e31f462562 100644
--- a/go/quickstep/res/layout/overview_actions_container.xml
+++ b/go/quickstep/res/layout/overview_actions_container.xml
@@ -124,23 +124,15 @@
     </LinearLayout>
 
     <!-- Unused. Included only for compatibility with parent class. -->
-    <LinearLayout
-        android:id="@+id/group_action_buttons"
-        android:layout_width="match_parent"
-        android:layout_height="@dimen/overview_actions_height"
+    <Button
+        android:id="@+id/action_save_app_pair"
+        style="@style/GoOverviewActionButton"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
         android:layout_gravity="top|center_horizontal"
-        android:orientation="horizontal"
-        android:visibility="gone">
-
-        <Button
-            android:id="@+id/action_save_app_pair"
-            style="@style/GoOverviewActionButton"
-            android:layout_width="wrap_content"
-            android:layout_height="wrap_content"
-            android:drawableStart="@drawable/ic_save_app_pair_up_down"
-            android:text="@string/action_save_app_pair"
-            android:theme="@style/ThemeControlHighlightWorkspaceColor" />
-
-    </LinearLayout>
+        android:drawableStart="@drawable/ic_save_app_pair_up_down"
+        android:text="@string/action_save_app_pair"
+        android:theme="@style/ThemeControlHighlightWorkspaceColor"
+        android:visibility="gone" />
 
 </com.android.quickstep.views.GoOverviewActionsView>
\ No newline at end of file
diff --git a/quickstep/res/layout/overview_actions_container.xml b/quickstep/res/layout/overview_actions_container.xml
index 7aaf7441a1..fcd2e5495a 100644
--- a/quickstep/res/layout/overview_actions_container.xml
+++ b/quickstep/res/layout/overview_actions_container.xml
@@ -47,22 +47,16 @@
 
     </LinearLayout>
 
-    <LinearLayout
-        android:id="@+id/group_action_buttons"
+    <!-- Currently, the only "group action button" is this save app pair button. If more are added,
+    a new LinearLayout may be needed to contain them, but beware of increased memory usage. -->
+    <Button
+        android:id="@+id/action_save_app_pair"
+        style="@style/OverviewActionButton"
         android:layout_width="wrap_content"
-        android:layout_height="@dimen/overview_actions_height"
+        android:layout_height="wrap_content"
+        android:text="@string/action_save_app_pair"
+        android:theme="@style/ThemeControlHighlightWorkspaceColor"
         android:layout_gravity="bottom|center_horizontal"
-        android:orientation="horizontal"
-        android:visibility="gone">
-
-        <Button
-            android:id="@+id/action_save_app_pair"
-            style="@style/OverviewActionButton"
-            android:layout_width="wrap_content"
-            android:layout_height="wrap_content"
-            android:text="@string/action_save_app_pair"
-            android:theme="@style/ThemeControlHighlightWorkspaceColor" />
-
-    </LinearLayout>
+        android:visibility="gone" />
 
 </com.android.quickstep.views.OverviewActionsView>
\ No newline at end of file
diff --git a/quickstep/src/com/android/quickstep/views/OverviewActionsView.java b/quickstep/src/com/android/quickstep/views/OverviewActionsView.java
index 83a2cebeb4..d729bdc2e2 100644
--- a/quickstep/src/com/android/quickstep/views/OverviewActionsView.java
+++ b/quickstep/src/com/android/quickstep/views/OverviewActionsView.java
@@ -110,9 +110,11 @@ public class OverviewActionsView<T extends OverlayUICallbacks> extends FrameLayo
 
     /** Container for the action buttons below a focused, non-split Overview tile. */
     protected LinearLayout mActionButtons;
-    /** Container for the action buttons below a focused, split Overview tile. */
-    protected LinearLayout mGroupActionButtons;
     private Button mSplitButton;
+    /**
+     * The "save app pair" button. Currently this is the only button that is not contained in
+     * mActionButtons, since it is the sole button that appears for a grouped task.
+     */
     private Button mSaveAppPairButton;
 
     @ActionsHiddenFlags
@@ -150,15 +152,16 @@ public class OverviewActionsView<T extends OverlayUICallbacks> extends FrameLayo
         super.onFinishInflate();
         // Initialize 2 view containers: one for single tasks, one for grouped tasks.
         // These will take up the same space on the screen and alternate visibility as needed.
+        // Currently, the only grouped task action is "save app pairs".
         mActionButtons = findViewById(R.id.action_buttons);
-        mGroupActionButtons = findViewById(R.id.group_action_buttons);
-        // Initialize a list to hold alphas for mActionButtons and mGroupActionButtons.
+        mSaveAppPairButton = findViewById(R.id.action_save_app_pair);
+        // Initialize a list to hold alphas for mActionButtons and any group action buttons.
         mMultiValueAlphas[ACTIONS_ALPHAS] = new MultiValueAlpha(mActionButtons, NUM_ALPHAS);
         mMultiValueAlphas[GROUP_ACTIONS_ALPHAS] =
-                new MultiValueAlpha(mGroupActionButtons, NUM_ALPHAS);
+                new MultiValueAlpha(mSaveAppPairButton, NUM_ALPHAS);
         Arrays.stream(mMultiValueAlphas).forEach(a -> a.setUpdateVisibility(true));
-        // To control alpha simultaneously on mActionButtons and mGroupActionButtons, we set up an
-        // AnimatedFloat for each alpha property.
+        // To control alpha simultaneously on mActionButtons and any group action buttons, we set up
+        // an AnimatedFloat for each alpha property.
         for (int i = 0; i < NUM_ALPHAS; i++) {
             final int index = i;
             mAlphaProperties[index] = new AnimatedFloat(() -> {
@@ -175,7 +178,6 @@ public class OverviewActionsView<T extends OverlayUICallbacks> extends FrameLayo
         screenshotButton.setOnClickListener(this);
         mSplitButton = findViewById(R.id.action_split);
         mSplitButton.setOnClickListener(this);
-        mSaveAppPairButton = findViewById(R.id.action_save_app_pair);
         mSaveAppPairButton.setOnClickListener(this);
     }
 
@@ -336,7 +338,7 @@ public class OverviewActionsView<T extends OverlayUICallbacks> extends FrameLayo
      */
     public boolean areActionsButtonsVisible() {
         return mActionButtons.getVisibility() == View.VISIBLE
-                || mGroupActionButtons.getVisibility() == View.VISIBLE;
+                || mSaveAppPairButton.getVisibility() == View.VISIBLE;
     }
 
     /**
@@ -350,11 +352,11 @@ public class OverviewActionsView<T extends OverlayUICallbacks> extends FrameLayo
     /** Updates vertical margins for different navigation mode or configuration changes. */
     public void updateVerticalMargin(NavigationMode mode) {
         updateActionBarPosition(mActionButtons);
-        updateActionBarPosition(mGroupActionButtons);
+        updateActionBarPosition(mSaveAppPairButton);
     }
 
     /** Positions actions buttons according to device settings and insets. */
-    private void updateActionBarPosition(LinearLayout actionBar) {
+    private void updateActionBarPosition(View actionBar) {
         if (mDp == null) {
             return;
         }
diff --git a/quickstep/tests/src/com/android/quickstep/TaplTestsSplitscreen.java b/quickstep/tests/src/com/android/quickstep/TaplTestsSplitscreen.java
index bfd7bdba41..6be082ace4 100644
--- a/quickstep/tests/src/com/android/quickstep/TaplTestsSplitscreen.java
+++ b/quickstep/tests/src/com/android/quickstep/TaplTestsSplitscreen.java
@@ -129,8 +129,6 @@ public class TaplTestsSplitscreen extends AbstractQuickStepTest {
                     overview.getCurrentTask()
                             .tapMenu()
                             .hasMenuItem("Save app pair"));
-        } else {
-            overview.getOverviewGroupActions().assertHasAction("Save app pair");
         }
     }
 
diff --git a/tests/tapl/com/android/launcher3/tapl/BaseOverview.java b/tests/tapl/com/android/launcher3/tapl/BaseOverview.java
index 2e3944d666..e10893eea5 100644
--- a/tests/tapl/com/android/launcher3/tapl/BaseOverview.java
+++ b/tests/tapl/com/android/launcher3/tapl/BaseOverview.java
@@ -358,21 +358,6 @@ public class BaseOverview extends LauncherInstrumentation.VisibleContainer {
         }
     }
 
-    /**
-     * Gets Overview Actions specific to grouped tasks.
-     *
-     * @return The Overview group actions bar
-     */
-    @NonNull
-    public OverviewActions getOverviewGroupActions() {
-        try (LauncherInstrumentation.Closable c = mLauncher.addContextLayer(
-                "want to get overview group actions")) {
-            verifyActiveContainer();
-            UiObject2 groupActions = mLauncher.waitForOverviewObject("group_action_buttons");
-            return new OverviewActions(groupActions, mLauncher);
-        }
-    }
-
     /**
      * Returns if clear all button is visible.
      */
@@ -469,13 +454,13 @@ public class BaseOverview extends LauncherInstrumentation.VisibleContainer {
 
             if (isActionsViewVisible()) {
                 if (task.isTaskSplit()) {
-                    mLauncher.waitForOverviewObject("group_action_buttons");
+                    mLauncher.waitForOverviewObject("action_save_app_pair");
                 } else {
                     mLauncher.waitForOverviewObject("action_buttons");
                 }
             } else {
                 mLauncher.waitUntilOverviewObjectGone("action_buttons");
-                mLauncher.waitUntilOverviewObjectGone("group_action_buttons");
+                mLauncher.waitUntilOverviewObjectGone("action_save_app_pair");
             }
         }
     }
diff --git a/tests/tapl/com/android/launcher3/tapl/OverviewActions.java b/tests/tapl/com/android/launcher3/tapl/OverviewActions.java
index 486a63b454..d7c40a0859 100644
--- a/tests/tapl/com/android/launcher3/tapl/OverviewActions.java
+++ b/tests/tapl/com/android/launcher3/tapl/OverviewActions.java
@@ -17,7 +17,6 @@
 package com.android.launcher3.tapl;
 
 import androidx.annotation.NonNull;
-import androidx.test.uiautomator.By;
 import androidx.test.uiautomator.UiObject2;
 
 /**
@@ -111,12 +110,4 @@ public class OverviewActions {
             }
         }
     }
-
-    /** Asserts that an item matching the given string is present in the overview actions. */
-    public void assertHasAction(String text) {
-        try (LauncherInstrumentation.Closable c = mLauncher.addContextLayer(
-                "want to check if the action [" + text + "] is present")) {
-            mLauncher.waitForObjectInContainer(mOverviewActions, By.text(text));
-        }
-    }
 }