From b4935a25caccfb4021c8546209b5db7219747792 Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Tue, 1 Jun 2021 15:12:15 -0400 Subject: [PATCH] Don't destroy the FalsingManager in Wallet. When FalsingManager#cleanupInternal is called, it no longer produces valid results. With this change, we check that the FalsingManager is not used after being destroyed, and also avoid destroying it in WalletScreenController. Fixes: 188174214 Test: manual Change-Id: I0ce67de5a326b56dee11c1d63c1d592640c0713d --- packages/SystemUI/Android.bp | 14 +++++++++-- .../systemui/plugins/FalsingManager.java | 11 ++++++-- .../classifier/BrightLineFalsingManager.java | 19 +++++++++++++- .../classifier/FalsingManagerProxy.java | 8 +++--- .../wallet/ui/WalletScreenController.java | 1 - .../classifier/BrightLineClassifierTest.java | 2 +- .../classifier/FalsingManagerFake.java | 25 ++++++++++++++----- 7 files changed, 63 insertions(+), 17 deletions(-) rename packages/SystemUI/{ => tests}/src/com/android/systemui/classifier/FalsingManagerFake.java (85%) diff --git a/packages/SystemUI/Android.bp b/packages/SystemUI/Android.bp index 4f587ebba89cd..b357a9478ab61 100644 --- a/packages/SystemUI/Android.bp +++ b/packages/SystemUI/Android.bp @@ -105,11 +105,21 @@ android_library { filegroup { name: "SystemUI-tests-utils", srcs: [ + "tests/src/com/android/systemui/SysuiTestCase.java", + "tests/src/com/android/systemui/TestableDependency.java", + "tests/src/com/android/systemui/classifier/FalsingManagerFake.java", "tests/src/com/android/systemui/statusbar/notification/collection/NotificationEntryBuilder.java", "tests/src/com/android/systemui/statusbar/RankingBuilder.java", "tests/src/com/android/systemui/statusbar/SbnBuilder.java", - "tests/src/com/android/systemui/util/concurrency/FakeExecutor.java", - "tests/src/com/android/systemui/util/time/FakeSystemClock.java", + "tests/src/com/android/systemui/SysuiTestableContext.java", + "tests/src/com/android/systemui/utils/leaks/BaseLeakChecker.java", + "tests/src/com/android/systemui/utils/leaks/LeakCheckedTest.java", + "tests/src/com/android/systemui/**/Fake*.java", + "tests/src/com/android/systemui/**/Fake*.kt", + ], + exclude_srcs: [ + "tests/src/com/android/systemui/**/*Test.java", + "tests/src/com/android/systemui/**/*Test.kt", ], path: "tests/src", } diff --git a/packages/SystemUI/plugin/src/com/android/systemui/plugins/FalsingManager.java b/packages/SystemUI/plugin/src/com/android/systemui/plugins/FalsingManager.java index 5ac8961aceebc..b4fac5cbb6ab4 100644 --- a/packages/SystemUI/plugin/src/com/android/systemui/plugins/FalsingManager.java +++ b/packages/SystemUI/plugin/src/com/android/systemui/plugins/FalsingManager.java @@ -114,7 +114,12 @@ public interface FalsingManager { /** From com.android.systemui.Dumpable. */ void dump(FileDescriptor fd, PrintWriter pw, String[] args); - void cleanup(); + /** + * Don't call this. It's meant for internal use to allow switching between implementations. + * + * Tests may also call it. + **/ + void cleanupInternal(); /** Call to report a ProximityEvent to the FalsingManager. */ void onProximityEvent(ProximityEvent proximityEvent); @@ -136,7 +141,9 @@ public interface FalsingManager { void onFalse(); } - /** Listener that is alerted when a double tap is required to confirm a single tap. */ + /** + * Listener that is alerted when a double tap is required to confirm a single tap. + **/ interface FalsingTapListener { void onDoubleTapRequired(); } diff --git a/packages/SystemUI/src/com/android/systemui/classifier/BrightLineFalsingManager.java b/packages/SystemUI/src/com/android/systemui/classifier/BrightLineFalsingManager.java index c821d100f5534..020401ecd2f82 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/BrightLineFalsingManager.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/BrightLineFalsingManager.java @@ -82,6 +82,8 @@ public class BrightLineFalsingManager implements FalsingManager { private final List mFalsingBeliefListeners = new ArrayList<>(); private List mFalsingTapListeners = new ArrayList<>(); + private boolean mDestroyed; + private final SessionListener mSessionListener = new SessionListener() { @Override public void onSessionEnded() { @@ -196,6 +198,8 @@ public class BrightLineFalsingManager implements FalsingManager { @Override public boolean isFalseTouch(@Classifier.InteractionType int interactionType) { + checkDestroyed(); + mPriorInteractionType = interactionType; if (skipFalsing(interactionType)) { mPriorResults = getPassedResult(1); @@ -221,6 +225,8 @@ public class BrightLineFalsingManager implements FalsingManager { @Override public boolean isSimpleTap() { + checkDestroyed(); + FalsingClassifier.Result result = mSingleTapClassifier.isTap( mDataProvider.getRecentMotionEvents(), 0); mPriorResults = Collections.singleton(result); @@ -228,8 +234,16 @@ public class BrightLineFalsingManager implements FalsingManager { return !result.isFalse(); } + private void checkDestroyed() { + if (mDestroyed) { + Log.wtf(TAG, "Tried to use FalsingManager after being destroyed!"); + } + } + @Override public boolean isFalseTap(@Penalty int penalty) { + checkDestroyed(); + if (skipFalsing(GENERIC)) { mPriorResults = getPassedResult(1); logDebug("Skipped falsing"); @@ -292,6 +306,8 @@ public class BrightLineFalsingManager implements FalsingManager { @Override public boolean isFalseDoubleTap() { + checkDestroyed(); + if (skipFalsing(GENERIC)) { mPriorResults = getPassedResult(1); logDebug("Skipped falsing"); @@ -406,7 +422,8 @@ public class BrightLineFalsingManager implements FalsingManager { } @Override - public void cleanup() { + public void cleanupInternal() { + mDestroyed = true; mDataProvider.removeSessionListener(mSessionListener); mDataProvider.removeGestureCompleteListener(mGestureFinalizedListener); mClassifiers.forEach(FalsingClassifier::cleanup); diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingManagerProxy.java b/packages/SystemUI/src/com/android/systemui/classifier/FalsingManagerProxy.java index ee0dba0a50873..5a24f354eaf65 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingManagerProxy.java +++ b/packages/SystemUI/src/com/android/systemui/classifier/FalsingManagerProxy.java @@ -79,7 +79,7 @@ public class FalsingManagerProxy implements FalsingManager, Dumpable { public void onPluginConnected(FalsingPlugin plugin, Context context) { FalsingManager pluginFalsingManager = plugin.getFalsingManager(context); if (pluginFalsingManager != null) { - mInternalFalsingManager.cleanup(); + mInternalFalsingManager.cleanupInternal(); mInternalFalsingManager = pluginFalsingManager; } } @@ -109,7 +109,7 @@ public class FalsingManagerProxy implements FalsingManager, Dumpable { */ private void setupFalsingManager() { if (mInternalFalsingManager != null) { - mInternalFalsingManager.cleanup(); + mInternalFalsingManager.cleanupInternal(); } mInternalFalsingManager = mBrightLineFalsingManagerProvider.get(); } @@ -195,10 +195,10 @@ public class FalsingManagerProxy implements FalsingManager, Dumpable { } @Override - public void cleanup() { + public void cleanupInternal() { mDeviceConfig.removeOnPropertiesChangedListener(mDeviceConfigListener); mPluginManager.removePluginListener(mPluginListener); mDumpManager.unregisterDumpable(DUMPABLE_TAG); - mInternalFalsingManager.cleanup(); + mInternalFalsingManager.cleanupInternal(); } } diff --git a/packages/SystemUI/src/com/android/systemui/wallet/ui/WalletScreenController.java b/packages/SystemUI/src/com/android/systemui/wallet/ui/WalletScreenController.java index d0662e7301d86..8da80caefdd37 100644 --- a/packages/SystemUI/src/com/android/systemui/wallet/ui/WalletScreenController.java +++ b/packages/SystemUI/src/com/android/systemui/wallet/ui/WalletScreenController.java @@ -260,7 +260,6 @@ public class WalletScreenController implements mIsDismissed = true; mSelectedCardId = null; mHandler.removeCallbacks(mSelectionRunnable); - mFalsingManager.cleanup(); mWalletClient.notifyWalletDismissed(); mWalletClient.removeWalletServiceEventListener(this); mWalletView.animateDismissal(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/classifier/BrightLineClassifierTest.java b/packages/SystemUI/tests/src/com/android/systemui/classifier/BrightLineClassifierTest.java index a7f9fe4e0a2c2..3eb1a9e624c8e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/classifier/BrightLineClassifierTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/classifier/BrightLineClassifierTest.java @@ -118,7 +118,7 @@ public class BrightLineClassifierTest extends SysuiTestCase { verify(mFalsingDataProvider).addSessionListener( any(FalsingDataProvider.SessionListener.class)); - mBrightLineFalsingManager.cleanup(); + mBrightLineFalsingManager.cleanupInternal(); verify(mFalsingDataProvider).removeSessionListener( any(FalsingDataProvider.SessionListener.class)); } diff --git a/packages/SystemUI/src/com/android/systemui/classifier/FalsingManagerFake.java b/packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingManagerFake.java similarity index 85% rename from packages/SystemUI/src/com/android/systemui/classifier/FalsingManagerFake.java rename to packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingManagerFake.java index dba530edc27fc..87d1b6b8cb303 100644 --- a/packages/SystemUI/src/com/android/systemui/classifier/FalsingManagerFake.java +++ b/packages/SystemUI/tests/src/com/android/systemui/classifier/FalsingManagerFake.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2019 The Android Open Source Project + * 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. @@ -16,6 +16,8 @@ package com.android.systemui.classifier; +import static com.google.common.truth.Truth.assertWithMessage; + import android.net.Uri; import com.android.internal.annotations.VisibleForTesting; @@ -34,10 +36,11 @@ public class FalsingManagerFake implements FalsingManager { private boolean mIsSimpleTap; private boolean mIsFalseDoubleTap; private boolean mIsUnlockingDisabled; - private boolean mIsClassiferEnabled; + private boolean mIsClassifierEnabled; private boolean mShouldEnforceBouncer; private boolean mIsReportingEnabled; private boolean mIsFalseRobustTap; + private boolean mDestroyed; private final List mFalsingBeliefListeners = new ArrayList<>(); private final List mTapListeners = new ArrayList<>(); @@ -64,6 +67,7 @@ public class FalsingManagerFake implements FalsingManager { @Override public boolean isFalseTouch(@Classifier.InteractionType int interactionType) { + checkDestroyed(); return mIsFalseTouch; } @@ -81,27 +85,30 @@ public class FalsingManagerFake implements FalsingManager { @Override public boolean isSimpleTap() { + checkDestroyed(); return mIsSimpleTap; } @Override public boolean isFalseTap(@Penalty int penalty) { + checkDestroyed(); return mIsFalseRobustTap; } @Override public boolean isFalseDoubleTap() { + checkDestroyed(); return mIsFalseDoubleTap; } @VisibleForTesting - public void setIsClassiferEnabled(boolean isClassiferEnabled) { - mIsClassiferEnabled = isClassiferEnabled; + public void setIsClassifierEnabled(boolean isClassifierEnabled) { + mIsClassifierEnabled = isClassifierEnabled; } @Override public boolean isClassifierEnabled() { - return mIsClassiferEnabled; + return mIsClassifierEnabled; } @Override @@ -129,7 +136,13 @@ public class FalsingManagerFake implements FalsingManager { } @Override - public void cleanup() { + public void cleanupInternal() { + mDestroyed = true; + } + + private void checkDestroyed() { + assertWithMessage("FakeFasingManager has been destroyed") + .that(mDestroyed).isFalse(); } @Override