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
diff --git a/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java b/packages/SystemUI/src/com/android/systemui/volume/VolumeDialogImpl.java
index 9362220..349f368 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 @@
/** 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 @@
VolumePanelFactory volumePanelFactory,
ActivityStarter activityStarter,
InteractionJankMonitor interactionJankMonitor,
+ boolean shouldListenForJank,
CsdWarningDialog.Factory csdWarningDialogFactory,
DevicePostureController devicePostureController,
Looper looper,
@@ -311,6 +319,8 @@
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 @@
}
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 aa4ee54..d0edc6e 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 @@
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 8f725be..0c77529 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 @@
mVolumePanelFactory,
mActivityStarter,
mInteractionJankMonitor,
+ false,
mCsdWarningDialogFactory,
mPostureController,
mTestableLooper.getLooper(),
@@ -378,6 +379,7 @@
mVolumePanelFactory,
mActivityStarter,
mInteractionJankMonitor,
+ false,
mCsdWarningDialogFactory,
devicePostureController,
mTestableLooper.getLooper(),
@@ -397,6 +399,8 @@
int gravity = dialog.getWindowGravity();
assertEquals(Gravity.TOP, gravity & Gravity.VERTICAL_GRAVITY_MASK);
+
+ cleanUp(dialog);
}
@Test
@@ -415,6 +419,7 @@
mVolumePanelFactory,
mActivityStarter,
mInteractionJankMonitor,
+ false,
mCsdWarningDialogFactory,
devicePostureController,
mTestableLooper.getLooper(),
@@ -433,6 +438,8 @@
int gravity = dialog.getWindowGravity();
assertEquals(Gravity.CENTER_VERTICAL, gravity & Gravity.VERTICAL_GRAVITY_MASK);
+
+ cleanUp(dialog);
}
@Test
@@ -451,6 +458,7 @@
mVolumePanelFactory,
mActivityStarter,
mInteractionJankMonitor,
+ false,
mCsdWarningDialogFactory,
devicePostureController,
mTestableLooper.getLooper(),
@@ -469,6 +477,8 @@
int gravity = dialog.getWindowGravity();
assertEquals(Gravity.CENTER_VERTICAL, gravity & Gravity.VERTICAL_GRAVITY_MASK);
+
+ cleanUp(dialog);
}
@Test
@@ -489,18 +499,19 @@
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 @@
@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() {