From 5fffa47cac0815329b3e95a24eb0e9d8b4b81339 Mon Sep 17 00:00:00 2001 From: Sunny Goyal Date: Fri, 7 Feb 2020 10:47:03 -0800 Subject: [PATCH] Fixing race condition on test information provider Activity tracker is accessed on a non-UI thread, which can cause a non-initialized Launcher to be treated as initialized Bug: 149022794 Change-Id: I6634a6aff891592369c16469bbe95a9ea611819c --- .../QuickstepTestInformationHandler.java | 41 +----- .../android/quickstep/views/RecentsView.java | 11 -- .../testing/TestInformationHandler.java | 139 ++++++++++++------ .../launcher3/testing/TestProtocol.java | 3 +- .../android/launcher3/tapl/BaseOverview.java | 10 +- .../tapl/LauncherInstrumentation.java | 6 + 6 files changed, 106 insertions(+), 104 deletions(-) diff --git a/quickstep/recents_ui_overrides/src/com/android/quickstep/QuickstepTestInformationHandler.java b/quickstep/recents_ui_overrides/src/com/android/quickstep/QuickstepTestInformationHandler.java index 3e6def3dfa..a538fffe81 100644 --- a/quickstep/recents_ui_overrides/src/com/android/quickstep/QuickstepTestInformationHandler.java +++ b/quickstep/recents_ui_overrides/src/com/android/quickstep/QuickstepTestInformationHandler.java @@ -1,23 +1,18 @@ package com.android.quickstep; -import static com.android.launcher3.util.Executors.MAIN_EXECUTOR; - +import android.app.Activity; import android.content.Context; import android.os.Bundle; import android.util.Log; -import com.android.launcher3.Launcher; -import com.android.launcher3.LauncherAppState; import com.android.launcher3.testing.TestInformationHandler; import com.android.launcher3.testing.TestProtocol; import com.android.launcher3.uioverrides.touchcontrollers.PortraitStatesTouchController; import com.android.quickstep.util.LayoutUtils; -import com.android.quickstep.views.RecentsView; import com.android.systemui.shared.recents.model.Task; import java.util.ArrayList; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; public class QuickstepTestInformationHandler extends TestInformationHandler { @@ -46,33 +41,8 @@ public class QuickstepTestInformationHandler extends TestInformationHandler { } case TestProtocol.REQUEST_HOTSEAT_TOP: { - if (mLauncher == null) return null; - - response.putInt(TestProtocol.TEST_INFO_RESPONSE_FIELD, - PortraitStatesTouchController.getHotseatTop(mLauncher)); - return response; - } - - case TestProtocol.REQUEST_OVERVIEW_LEFT_GESTURE_MARGIN: { - try { - final int leftMargin = MAIN_EXECUTOR.submit(() -> - getRecentsView().getLeftGestureMargin()).get(); - response.putInt(TestProtocol.TEST_INFO_RESPONSE_FIELD, leftMargin); - } catch (ExecutionException | InterruptedException e) { - throw new RuntimeException(e); - } - return response; - } - - case TestProtocol.REQUEST_OVERVIEW_RIGHT_GESTURE_MARGIN: { - try { - final int rightMargin = MAIN_EXECUTOR.submit(() -> - getRecentsView().getRightGestureMargin()).get(); - response.putInt(TestProtocol.TEST_INFO_RESPONSE_FIELD, rightMargin); - } catch (ExecutionException | InterruptedException e) { - throw new RuntimeException(e); - } - return response; + return getLauncherUIProperty( + Bundle::putInt, PortraitStatesTouchController::getHotseatTop); } case TestProtocol.REQUEST_RECENT_TASKS_LIST: { @@ -99,11 +69,12 @@ public class QuickstepTestInformationHandler extends TestInformationHandler { return super.call(method); } - private RecentsView getRecentsView() { + @Override + protected Activity getCurrentActivity() { OverviewComponentObserver observer = new OverviewComponentObserver(mContext, new RecentsAnimationDeviceState(mContext)); try { - return observer.getActivityInterface().getCreatedActivity().getOverviewPanel(); + return observer.getActivityInterface().getCreatedActivity(); } finally { observer.onDestroy(); } diff --git a/quickstep/recents_ui_overrides/src/com/android/quickstep/views/RecentsView.java b/quickstep/recents_ui_overrides/src/com/android/quickstep/views/RecentsView.java index aaba308c0e..321af6c7ba 100644 --- a/quickstep/recents_ui_overrides/src/com/android/quickstep/views/RecentsView.java +++ b/quickstep/recents_ui_overrides/src/com/android/quickstep/views/RecentsView.java @@ -74,7 +74,6 @@ import android.view.MotionEvent; import android.view.View; import android.view.ViewDebug; import android.view.ViewGroup; -import android.view.WindowInsets; import android.view.accessibility.AccessibilityEvent; import android.view.accessibility.AccessibilityNodeInfo; import android.widget.ListView; @@ -1890,16 +1889,6 @@ public abstract class RecentsView extends PagedView impl } } - public int getLeftGestureMargin() { - final WindowInsets insets = getRootWindowInsets(); - return Math.max(insets.getSystemGestureInsets().left, insets.getSystemWindowInsetLeft()); - } - - public int getRightGestureMargin() { - final WindowInsets insets = getRootWindowInsets(); - return Math.max(insets.getSystemGestureInsets().right, insets.getSystemWindowInsetRight()); - } - /** If it's in the live tile mode, switch the running task into screenshot mode. */ public void switchToScreenshot(ThumbnailData thumbnailData, Runnable onFinishRunnable) { TaskView taskView = getRunningTaskView(); diff --git a/src/com/android/launcher3/testing/TestInformationHandler.java b/src/com/android/launcher3/testing/TestInformationHandler.java index 4af5e0a2ef..994912b97a 100644 --- a/src/com/android/launcher3/testing/TestInformationHandler.java +++ b/src/com/android/launcher3/testing/TestInformationHandler.java @@ -17,16 +17,22 @@ package com.android.launcher3.testing; import static android.graphics.Bitmap.Config.ARGB_8888; +import static com.android.launcher3.allapps.AllAppsStore.DEFER_UPDATES_TEST; import static com.android.launcher3.util.Executors.MAIN_EXECUTOR; +import android.annotation.TargetApi; +import android.app.Activity; import android.content.Context; import android.graphics.Bitmap; import android.graphics.Color; +import android.graphics.Insets; +import android.os.Build; import android.os.Bundle; import android.os.Debug; import android.system.Os; import android.util.Log; import android.view.View; +import android.view.WindowInsets; import androidx.annotation.Keep; @@ -36,14 +42,19 @@ import com.android.launcher3.Launcher; import com.android.launcher3.LauncherAppState; import com.android.launcher3.LauncherState; import com.android.launcher3.R; -import com.android.launcher3.allapps.AllAppsStore; import com.android.launcher3.util.ResourceBasedOverride; import java.util.LinkedList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import java.util.function.Supplier; +/** + * Class to handle requests from tests + */ +@TargetApi(Build.VERSION_CODES.Q) public class TestInformationHandler implements ResourceBasedOverride { public static TestInformationHandler newInstance(Context context) { @@ -54,7 +65,6 @@ public class TestInformationHandler implements ResourceBasedOverride { protected Context mContext; protected DeviceProfile mDeviceProfile; protected LauncherAppState mLauncherAppState; - protected Launcher mLauncher; private static LinkedList mLeaks; public void init(Context context) { @@ -62,35 +72,31 @@ public class TestInformationHandler implements ResourceBasedOverride { mDeviceProfile = InvariantDeviceProfile.INSTANCE. get(context).getDeviceProfile(context); mLauncherAppState = LauncherAppState.getInstanceNoCreate(); - mLauncher = Launcher.ACTIVITY_TRACKER.getCreatedActivity(); } public Bundle call(String method) { final Bundle response = new Bundle(); switch (method) { case TestProtocol.REQUEST_ALL_APPS_TO_OVERVIEW_SWIPE_HEIGHT: { - if (mLauncher == null) return null; - - final float progress = LauncherState.OVERVIEW.getVerticalProgress(mLauncher) - - LauncherState.ALL_APPS.getVerticalProgress(mLauncher); - final float distance = mLauncher.getAllAppsController().getShiftRange() * progress; - response.putInt(TestProtocol.TEST_INFO_RESPONSE_FIELD, (int) distance); - break; + return getLauncherUIProperty(Bundle::putInt, l -> { + final float progress = LauncherState.OVERVIEW.getVerticalProgress(l) + - LauncherState.ALL_APPS.getVerticalProgress(l); + final float distance = l.getAllAppsController().getShiftRange() * progress; + return (int) distance; + }); } case TestProtocol.REQUEST_HOME_TO_ALL_APPS_SWIPE_HEIGHT: { - if (mLauncher == null) return null; - - final float progress = LauncherState.NORMAL.getVerticalProgress(mLauncher) - - LauncherState.ALL_APPS.getVerticalProgress(mLauncher); - final float distance = mLauncher.getAllAppsController().getShiftRange() * progress; - response.putInt(TestProtocol.TEST_INFO_RESPONSE_FIELD, (int) distance); - break; + return getLauncherUIProperty(Bundle::putInt, l -> { + final float progress = LauncherState.NORMAL.getVerticalProgress(l) + - LauncherState.ALL_APPS.getVerticalProgress(l); + final float distance = l.getAllAppsController().getShiftRange() * progress; + return (int) distance; + }); } case TestProtocol.REQUEST_IS_LAUNCHER_INITIALIZED: { - response.putBoolean(TestProtocol.TEST_INFO_RESPONSE_FIELD, isLauncherInitialized()); - break; + return getUIProperty(Bundle::putBoolean, t -> isLauncherInitialized(), () -> true); } case TestProtocol.REQUEST_ENABLE_DEBUG_TRACING: @@ -102,40 +108,33 @@ public class TestInformationHandler implements ResourceBasedOverride { break; case TestProtocol.REQUEST_FREEZE_APP_LIST: - MAIN_EXECUTOR.execute(() -> - mLauncher.getAppsView().getAppsStore().enableDeferUpdates( - AllAppsStore.DEFER_UPDATES_TEST)); - break; - + return getLauncherUIProperty(Bundle::putBoolean, l -> { + l.getAppsView().getAppsStore().enableDeferUpdates(DEFER_UPDATES_TEST); + return true; + }); case TestProtocol.REQUEST_UNFREEZE_APP_LIST: - MAIN_EXECUTOR.execute(() -> - mLauncher.getAppsView().getAppsStore().disableDeferUpdates( - AllAppsStore.DEFER_UPDATES_TEST)); - break; + return getLauncherUIProperty(Bundle::putBoolean, l -> { + l.getAppsView().getAppsStore().disableDeferUpdates(DEFER_UPDATES_TEST); + return true; + }); case TestProtocol.REQUEST_APP_LIST_FREEZE_FLAGS: { - try { - final int deferUpdatesFlags = MAIN_EXECUTOR.submit(() -> - mLauncher.getAppsView().getAppsStore().getDeferUpdatesFlags()).get(); - response.putInt(TestProtocol.TEST_INFO_RESPONSE_FIELD, - deferUpdatesFlags); - } catch (ExecutionException | InterruptedException e) { - throw new RuntimeException(e); - } - break; + return getLauncherUIProperty(Bundle::putInt, + l -> l.getAppsView().getAppsStore().getDeferUpdatesFlags()); } case TestProtocol.REQUEST_APPS_LIST_SCROLL_Y: { - try { - final int scroll = MAIN_EXECUTOR.submit(() -> - mLauncher.getAppsView().getActiveRecyclerView().getCurrentScrollY()) - .get(); - response.putInt(TestProtocol.TEST_INFO_RESPONSE_FIELD, - scroll); - } catch (ExecutionException | InterruptedException e) { - throw new RuntimeException(e); - } - break; + return getLauncherUIProperty(Bundle::putInt, + l -> l.getAppsView().getActiveRecyclerView().getCurrentScrollY()); + } + + case TestProtocol.REQUEST_WINDOW_INSETS: { + return getUIProperty(Bundle::putParcelable, a -> { + WindowInsets insets = a.getWindow() + .getDecorView().getRootWindowInsets(); + return Insets.max( + insets.getSystemGestureInsets(), insets.getSystemWindowInsets()); + }, this::getCurrentActivity); } case TestProtocol.REQUEST_PID: { @@ -176,7 +175,6 @@ public class TestInformationHandler implements ResourceBasedOverride { case TestProtocol.REQUEST_VIEW_LEAK: { if (mLeaks == null) mLeaks = new LinkedList(); - mLeaks.add(new View(mContext)); break; } @@ -200,6 +198,10 @@ public class TestInformationHandler implements ResourceBasedOverride { || LauncherAppState.getInstance(mContext).getModel().isModelLoaded(); } + protected Activity getCurrentActivity() { + return Launcher.ACTIVITY_TRACKER.getCreatedActivity(); + } + private static void runGcAndFinalizersSync() { Runtime.getRuntime().gc(); Runtime.getRuntime().runFinalization(); @@ -216,6 +218,47 @@ public class TestInformationHandler implements ResourceBasedOverride { } } + /** + * Returns the result by getting a Launcher property on UI thread + */ + public static Bundle getLauncherUIProperty( + BundleSetter bundleSetter, Function provider) { + return getUIProperty(bundleSetter, provider, Launcher.ACTIVITY_TRACKER::getCreatedActivity); + } + + /** + * Returns the result by getting a generic property on UI thread + */ + private static Bundle getUIProperty( + BundleSetter bundleSetter, Function provider, Supplier targetSupplier) { + try { + return MAIN_EXECUTOR.submit(() -> { + S target = targetSupplier.get(); + if (target == null) { + return null; + } + T value = provider.apply(target); + Bundle response = new Bundle(); + bundleSetter.set(response, TestProtocol.TEST_INFO_RESPONSE_FIELD, value); + return response; + }).get(); + } catch (ExecutionException | InterruptedException e) { + throw new RuntimeException(e); + } + } + + /** + * Generic interface for setting a fiend in bundle + * @param the type of value being set + */ + public interface BundleSetter { + + /** + * Sets any generic property to the bundle + */ + void set(Bundle b, String key, T value); + } + // Create the observer in the scope of a method to minimize the chance that // it remains live in a DEX/machine register at the point of the fence guard. // This must be kept to avoid R8 inlining it. diff --git a/src/com/android/launcher3/testing/TestProtocol.java b/src/com/android/launcher3/testing/TestProtocol.java index 2f053c99e8..6e5379086c 100644 --- a/src/com/android/launcher3/testing/TestProtocol.java +++ b/src/com/android/launcher3/testing/TestProtocol.java @@ -75,8 +75,7 @@ public final class TestProtocol { public static final String REQUEST_UNFREEZE_APP_LIST = "unfreeze-app-list"; public static final String REQUEST_APP_LIST_FREEZE_FLAGS = "app-list-freeze-flags"; public static final String REQUEST_APPS_LIST_SCROLL_Y = "apps-list-scroll-y"; - public static final String REQUEST_OVERVIEW_LEFT_GESTURE_MARGIN = "overview-left-margin"; - public static final String REQUEST_OVERVIEW_RIGHT_GESTURE_MARGIN = "overview-right-margin"; + public static final String REQUEST_WINDOW_INSETS = "window-insets"; public static final String REQUEST_PID = "pid"; public static final String REQUEST_TOTAL_PSS_KB = "total_pss"; public static final String REQUEST_JAVA_LEAK = "java-leak"; diff --git a/tests/tapl/com/android/launcher3/tapl/BaseOverview.java b/tests/tapl/com/android/launcher3/tapl/BaseOverview.java index e5c83e2c3c..a769acfc35 100644 --- a/tests/tapl/com/android/launcher3/tapl/BaseOverview.java +++ b/tests/tapl/com/android/launcher3/tapl/BaseOverview.java @@ -23,8 +23,6 @@ import androidx.test.uiautomator.BySelector; import androidx.test.uiautomator.Direction; import androidx.test.uiautomator.UiObject2; -import com.android.launcher3.testing.TestProtocol; - import java.util.Collections; import java.util.List; @@ -58,9 +56,7 @@ public class BaseOverview extends LauncherInstrumentation.VisibleContainer { mLauncher.addContextLayer("want to fling forward in overview")) { LauncherInstrumentation.log("Overview.flingForward before fling"); final UiObject2 overview = verifyActiveContainer(); - final int leftMargin = mLauncher.getTestInfo( - TestProtocol.REQUEST_OVERVIEW_LEFT_GESTURE_MARGIN). - getInt(TestProtocol.TEST_INFO_RESPONSE_FIELD); + final int leftMargin = mLauncher.getTargetInsets().left; mLauncher.scroll( overview, Direction.LEFT, new Rect(leftMargin + 1, 0, 0, 0), 20, false); verifyActiveContainer(); @@ -96,9 +92,7 @@ public class BaseOverview extends LauncherInstrumentation.VisibleContainer { mLauncher.addContextLayer("want to fling backward in overview")) { LauncherInstrumentation.log("Overview.flingBackward before fling"); final UiObject2 overview = verifyActiveContainer(); - final int rightMargin = mLauncher.getTestInfo( - TestProtocol.REQUEST_OVERVIEW_RIGHT_GESTURE_MARGIN). - getInt(TestProtocol.TEST_INFO_RESPONSE_FIELD); + final int rightMargin = mLauncher.getTargetInsets().right; mLauncher.scroll( overview, Direction.RIGHT, new Rect(0, 0, rightMargin + 1, 0), 20, false); verifyActiveContainer(); diff --git a/tests/tapl/com/android/launcher3/tapl/LauncherInstrumentation.java b/tests/tapl/com/android/launcher3/tapl/LauncherInstrumentation.java index 5eb164ecd4..534b0d4c18 100644 --- a/tests/tapl/com/android/launcher3/tapl/LauncherInstrumentation.java +++ b/tests/tapl/com/android/launcher3/tapl/LauncherInstrumentation.java @@ -33,6 +33,7 @@ import android.content.Context; import android.content.pm.PackageManager; import android.content.pm.ProviderInfo; import android.content.res.Resources; +import android.graphics.Insets; import android.graphics.Point; import android.graphics.Rect; import android.net.Uri; @@ -257,6 +258,11 @@ public final class LauncherInstrumentation { return getContext().getContentResolver().call(mTestProviderUri, request, null, null); } + Insets getTargetInsets() { + return getTestInfo(TestProtocol.REQUEST_WINDOW_INSETS) + .getParcelable(TestProtocol.TEST_INFO_RESPONSE_FIELD); + } + void setActiveContainer(VisibleContainer container) { sActiveContainer = new WeakReference<>(container); }