From 9f5a33afb76877a9c5e9cbadf500c7312d0baa06 Mon Sep 17 00:00:00 2001 From: Behnam Heydarshahi Date: Fri, 30 Jun 2023 21:09:43 +0000 Subject: [PATCH] Tear down VolumeDialogImplTest cleanly VolumeDialogImpl tests used to run a method on a mock InteractionJankMonitor, a fault which resulted in a nullpointer exception that caused the subsequent tests to crash. This CL avoids the fault by preventing the abovementioned method from running by allowing the VolumeDialogImpl class to not have an animator jank listener, when under test. Note: for long-term, instead of not listening to animation events, either the lingering animation calls should be omitted or the mock InteractionJankMonitor should not react to those calls with a null pointer exception. Bug: b/289228716 Test: atest VolumeDialogImplTest QuickAccessWalletControllerTest Change-Id: I0677b6b6ab958dc9939f55851c00dc67d32b828d --- .../systemui/volume/VolumeDialogImpl.java | 15 ++++++++++- .../systemui/volume/dagger/VolumeModule.java | 1 + .../systemui/volume/VolumeDialogImplTest.java | 27 ++++++++++++++----- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java b/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java index 93622200ad46b..349f3684659cd 100644 --- a/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java +++ b/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java @@ -168,6 +168,13 @@ public class VolumeDialogImpl implements VolumeDialog, Dumpable, /** Volume dialog slider animation. */ private static final String TYPE_UPDATE = "update"; + /** + * TODO(b/290612381): remove lingering animations or tolerate them + * When false, this will cause this class to not listen to animator events and not record jank + * events. This should never be false in production code, and only is false for unit tests for + * this class. This flag should be true in Scenario/Integration tests. + */ + private final boolean mShouldListenForJank; private final int mDialogShowAnimationDurationMs; private final int mDialogHideAnimationDurationMs; private int mDialogWidth; @@ -304,6 +311,7 @@ public class VolumeDialogImpl implements VolumeDialog, Dumpable, VolumePanelFactory volumePanelFactory, ActivityStarter activityStarter, InteractionJankMonitor interactionJankMonitor, + boolean shouldListenForJank, CsdWarningDialog.Factory csdWarningDialogFactory, DevicePostureController devicePostureController, Looper looper, @@ -311,6 +319,8 @@ public class VolumeDialogImpl implements VolumeDialog, Dumpable, mContext = new ContextThemeWrapper(context, R.style.volume_dialog_theme); mHandler = new H(looper); + + mShouldListenForJank = shouldListenForJank; mController = volumeDialogController; mKeyguard = (KeyguardManager) mContext.getSystemService(Context.KEYGUARD_SERVICE); mActivityManager = (ActivityManager) mContext.getSystemService(Context.ACTIVITY_SERVICE); @@ -1368,7 +1378,10 @@ public class VolumeDialogImpl implements VolumeDialog, Dumpable, } private Animator.AnimatorListener getJankListener(View v, String type, long timeout) { - return new Animator.AnimatorListener() { + if (!mShouldListenForJank) { + // TODO(b/290612381): temporary fix to prevent null pointers on leftover JankMonitors + return null; + } else return new Animator.AnimatorListener() { @Override public void onAnimationStart(@NonNull Animator animation) { if (!v.isAttachedToWindow()) { diff --git a/packages/SystemUI/src/com/android/systemui/volume/dagger/VolumeModule.java b/packages/SystemUI/src/com/android/systemui/volume/dagger/VolumeModule.java index aa4ee545a5004..d0edc6e7ce4c2 100644 --- a/packages/SystemUI/src/com/android/systemui/volume/dagger/VolumeModule.java +++ b/packages/SystemUI/src/com/android/systemui/volume/dagger/VolumeModule.java @@ -72,6 +72,7 @@ public interface VolumeModule { volumePanelFactory, activityStarter, interactionJankMonitor, + true, /* should listen for jank */ csdFactory, devicePostureController, Looper.getMainLooper(), diff --git a/packages/SystemUI/tests/src/com/android/systemui/volume/VolumeDialogImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/volume/VolumeDialogImplTest.java index 8f725bebfb16c..0c77529377aba 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/volume/VolumeDialogImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/volume/VolumeDialogImplTest.java @@ -142,6 +142,7 @@ public class VolumeDialogImplTest extends SysuiTestCase { mVolumePanelFactory, mActivityStarter, mInteractionJankMonitor, + false, mCsdWarningDialogFactory, mPostureController, mTestableLooper.getLooper(), @@ -378,6 +379,7 @@ public class VolumeDialogImplTest extends SysuiTestCase { mVolumePanelFactory, mActivityStarter, mInteractionJankMonitor, + false, mCsdWarningDialogFactory, devicePostureController, mTestableLooper.getLooper(), @@ -397,6 +399,8 @@ public class VolumeDialogImplTest extends SysuiTestCase { int gravity = dialog.getWindowGravity(); assertEquals(Gravity.TOP, gravity & Gravity.VERTICAL_GRAVITY_MASK); + + cleanUp(dialog); } @Test @@ -415,6 +419,7 @@ public class VolumeDialogImplTest extends SysuiTestCase { mVolumePanelFactory, mActivityStarter, mInteractionJankMonitor, + false, mCsdWarningDialogFactory, devicePostureController, mTestableLooper.getLooper(), @@ -433,6 +438,8 @@ public class VolumeDialogImplTest extends SysuiTestCase { int gravity = dialog.getWindowGravity(); assertEquals(Gravity.CENTER_VERTICAL, gravity & Gravity.VERTICAL_GRAVITY_MASK); + + cleanUp(dialog); } @Test @@ -451,6 +458,7 @@ public class VolumeDialogImplTest extends SysuiTestCase { mVolumePanelFactory, mActivityStarter, mInteractionJankMonitor, + false, mCsdWarningDialogFactory, devicePostureController, mTestableLooper.getLooper(), @@ -469,6 +477,8 @@ public class VolumeDialogImplTest extends SysuiTestCase { int gravity = dialog.getWindowGravity(); assertEquals(Gravity.CENTER_VERTICAL, gravity & Gravity.VERTICAL_GRAVITY_MASK); + + cleanUp(dialog); } @Test @@ -489,18 +499,19 @@ public class VolumeDialogImplTest extends SysuiTestCase { mVolumePanelFactory, mActivityStarter, mInteractionJankMonitor, + false, mCsdWarningDialogFactory, mPostureController, mTestableLooper.getLooper(), - mDumpManager - ); + mDumpManager); dialog.init(0, null); verify(mPostureController, never()).removeCallback(any()); - dialog.destroy(); verify(mPostureController).removeCallback(any()); + + cleanUp(dialog); } private void setOrientation(int orientation) { @@ -513,14 +524,18 @@ public class VolumeDialogImplTest extends SysuiTestCase { @After public void teardown() { - if (mDialog != null) { - mDialog.clearInternalHandlerAfterTest(); - } + cleanUp(mDialog); setOrientation(mOriginalOrientation); mTestableLooper.processAllMessages(); reset(mPostureController); } + private void cleanUp(VolumeDialogImpl dialog) { + if (dialog != null) { + dialog.clearInternalHandlerAfterTest(); + } + } + /* @Test public void testContentDescriptions() {