From d68e60ddfaf6576732637ed35d79ba3d8e237506 Mon Sep 17 00:00:00 2001 From: Bill Lin Date: Mon, 22 Nov 2021 14:14:17 +0800 Subject: [PATCH] 3/ Inject InteractionJankMonitor for StatusBarStateControllerImpl Scope : StatusBar & NotificationShade The purpose of InteractionJankMonitor.begin() and InteractionJankMonitor.end() is used for e2e tests (scenario / latency / benchmark tests) SysUI had setup dagger for InteractionJankMonitor, it would be helpful on unittest if inject instance in SysUI code directly. Test: atest SystemUITests Test: m ArcSystemUI-core Test: m CarSystemUI-core Bug: 206720526 Change-Id: Ic17b8f990c353bd4723b79bbfbed40c21016e29c --- .../StatusBarStateControllerImpl.java | 19 +++++++++++++----- .../NotificationPanelViewController.java | 18 +++++++++++------ .../statusbar/phone/PanelViewController.java | 16 +++++++++++++-- .../StatusBarStateControllerImplTest.kt | 18 ++++++++++++++++- .../NotificationPanelViewControllerTest.java | 20 ++++++++++++++----- 5 files changed, 72 insertions(+), 19 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java index da2b85ee0b619..2dbe59e27ea68 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java @@ -96,6 +96,7 @@ public class StatusBarStateControllerImpl implements private final ArrayList mListeners = new ArrayList<>(); private final UiEventLogger mUiEventLogger; + private final InteractionJankMonitor mInteractionJankMonitor; private int mState; private int mLastState; private int mUpcomingState; @@ -149,8 +150,10 @@ public class StatusBarStateControllerImpl implements private Interpolator mDozeInterpolator = Interpolators.FAST_OUT_SLOW_IN; @Inject - public StatusBarStateControllerImpl(UiEventLogger uiEventLogger, DumpManager dumpManager) { + public StatusBarStateControllerImpl(UiEventLogger uiEventLogger, DumpManager dumpManager, + InteractionJankMonitor interactionJankMonitor) { mUiEventLogger = uiEventLogger; + mInteractionJankMonitor = interactionJankMonitor; for (int i = 0; i < HISTORY_SIZE; i++) { mHistoricalRecords[i] = new HistoricalState(); } @@ -344,17 +347,23 @@ public class StatusBarStateControllerImpl implements } private void beginInteractionJankMonitor() { - if (mView != null && mView.isAttachedToWindow()) { - InteractionJankMonitor.getInstance().begin(mView, getCujType()); + if (mInteractionJankMonitor != null && mView != null && mView.isAttachedToWindow()) { + mInteractionJankMonitor.begin(mView, getCujType()); } } private void endInteractionJankMonitor() { - InteractionJankMonitor.getInstance().end(getCujType()); + if (mInteractionJankMonitor == null) { + return; + } + mInteractionJankMonitor.end(getCujType()); } private void cancelInteractionJankMonitor() { - InteractionJankMonitor.getInstance().cancel(getCujType()); + if (mInteractionJankMonitor == null) { + return; + } + mInteractionJankMonitor.cancel(getCujType()); } private int getCujType() { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java index 16aac4d765a79..3dbc10021f583 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -288,6 +288,7 @@ public class NotificationPanelViewController extends PanelViewController { private static final Rect M_DUMMY_DIRTY_RECT = new Rect(0, 0, 1, 1); private static final Rect EMPTY_RECT = new Rect(); + private final InteractionJankMonitor mInteractionJankMonitor; private final LayoutInflater mLayoutInflater; private final FeatureFlags mFeatureFlags; private final PowerManager mPowerManager; @@ -769,7 +770,8 @@ public class NotificationPanelViewController extends PanelViewController { PanelExpansionStateManager panelExpansionStateManager, NotificationRemoteInputManager remoteInputManager, Optional unfoldComponent, - ControlsComponent controlsComponent) { + ControlsComponent controlsComponent, + InteractionJankMonitor interactionJankMonitor) { super(view, falsingManager, dozeLog, @@ -782,7 +784,8 @@ public class NotificationPanelViewController extends PanelViewController { statusBarTouchableRegionManager, lockscreenGestureLogger, panelExpansionStateManager, - ambientState); + ambientState, + interactionJankMonitor); mView = view; mVibratorHelper = vibratorHelper; mKeyguardMediaController = keyguardMediaController; @@ -838,6 +841,7 @@ public class NotificationPanelViewController extends PanelViewController { mTapAgainViewController = tapAgainViewController; mUiExecutor = uiExecutor; mSecureSettings = secureSettings; + mInteractionJankMonitor = interactionJankMonitor; // TODO: inject via dagger instead of Dependency mSysUiState = Dependency.get(SysUiState.class); pulseExpansionHandler.setPulseExpandAbortListener(() -> { @@ -1892,14 +1896,16 @@ public class NotificationPanelViewController extends PanelViewController { } private void traceQsJank(boolean startTracing, boolean wasCancelled) { - InteractionJankMonitor monitor = InteractionJankMonitor.getInstance(); + if (mInteractionJankMonitor == null) { + return; + } if (startTracing) { - monitor.begin(mView, CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE); + mInteractionJankMonitor.begin(mView, CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE); } else { if (wasCancelled) { - monitor.cancel(CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE); + mInteractionJankMonitor.cancel(CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE); } else { - monitor.end(CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE); + mInteractionJankMonitor.end(CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE); } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PanelViewController.java index 2823d985102f7..6a00591ea0e9b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PanelViewController.java @@ -184,6 +184,7 @@ public abstract class PanelViewController { protected final LockscreenGestureLogger mLockscreenGestureLogger; private final PanelExpansionStateManager mPanelExpansionStateManager; private final TouchHandler mTouchHandler; + private final InteractionJankMonitor mInteractionJankMonitor; protected abstract void onExpandingFinished(); @@ -222,7 +223,8 @@ public abstract class PanelViewController { StatusBarTouchableRegionManager statusBarTouchableRegionManager, LockscreenGestureLogger lockscreenGestureLogger, PanelExpansionStateManager panelExpansionStateManager, - AmbientState ambientState) { + AmbientState ambientState, + InteractionJankMonitor interactionJankMonitor) { mAmbientState = ambientState; mView = view; mStatusBarKeyguardViewManager = statusBarKeyguardViewManager; @@ -273,6 +275,7 @@ public abstract class PanelViewController { mVibratorHelper = vibratorHelper; mVibrateOnOpening = mResources.getBoolean(R.bool.config_vibrateOnIconAnimation); mStatusBarTouchableRegionManager = statusBarTouchableRegionManager; + mInteractionJankMonitor = interactionJankMonitor; } protected void loadDimens() { @@ -1411,17 +1414,26 @@ public abstract class PanelViewController { } private void beginJankMonitoring(int cuj) { + if (mInteractionJankMonitor == null) { + return; + } InteractionJankMonitor.Configuration.Builder builder = InteractionJankMonitor.Configuration.Builder.withView(cuj, mView) .setTag(isFullyCollapsed() ? "Expand" : "Collapse"); - InteractionJankMonitor.getInstance().begin(builder); + mInteractionJankMonitor.begin(builder); } private void endJankMonitoring(int cuj) { + if (mInteractionJankMonitor == null) { + return; + } InteractionJankMonitor.getInstance().end(cuj); } private void cancelJankMonitoring(int cuj) { + if (mInteractionJankMonitor == null) { + return; + } InteractionJankMonitor.getInstance().cancel(cuj); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarStateControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarStateControllerImplTest.kt index c974882ff3054..b736f389ad6c0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarStateControllerImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarStateControllerImplTest.kt @@ -19,6 +19,7 @@ package com.android.systemui.statusbar import android.testing.AndroidTestingRunner import android.testing.TestableLooper import androidx.test.filters.SmallTest +import com.android.internal.jank.InteractionJankMonitor import com.android.internal.logging.testing.UiEventLoggerFake import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager @@ -26,20 +27,35 @@ import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.Mock import org.mockito.Mockito.mock +import org.mockito.MockitoAnnotations +import org.mockito.Mockito.`when` as whenever @SmallTest @RunWith(AndroidTestingRunner::class) @TestableLooper.RunWithLooper class StatusBarStateControllerImplTest : SysuiTestCase() { + @Mock lateinit var interactionJankMonitor: InteractionJankMonitor + private lateinit var controller: StatusBarStateControllerImpl private lateinit var uiEventLogger: UiEventLoggerFake @Before fun setUp() { + MockitoAnnotations.initMocks(this) + whenever(interactionJankMonitor.begin(any(), anyInt())).thenReturn(true) + whenever(interactionJankMonitor.end(anyInt())).thenReturn(true) + uiEventLogger = UiEventLoggerFake() - controller = StatusBarStateControllerImpl(uiEventLogger, mock(DumpManager::class.java)) + controller = StatusBarStateControllerImpl( + uiEventLogger, + mock(DumpManager::class.java), + interactionJankMonitor + ) } @Test diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewControllerTest.java index 270c64ddfa7a0..3ec9629e187a0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewControllerTest.java @@ -68,6 +68,7 @@ import androidx.constraintlayout.widget.ConstraintLayout; import androidx.constraintlayout.widget.ConstraintSet; import androidx.test.filters.SmallTest; +import com.android.internal.jank.InteractionJankMonitor; import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.UiEventLogger; import com.android.internal.logging.testing.UiEventLoggerFake; @@ -350,6 +351,8 @@ public class NotificationPanelViewControllerTest extends SysuiTestCase { @Mock private DumpManager mDumpManager; @Mock + private InteractionJankMonitor mInteractionJankMonitor; + @Mock private NotificationsQSContainerController mNotificationsQSContainerController; private Optional mSysUIUnfoldComponent = Optional.empty(); private SysuiStatusBarStateController mStatusBarStateController; @@ -363,7 +366,8 @@ public class NotificationPanelViewControllerTest extends SysuiTestCase { @Before public void setup() { MockitoAnnotations.initMocks(this); - mStatusBarStateController = new StatusBarStateControllerImpl(mUiEventLogger, mDumpManager); + mStatusBarStateController = new StatusBarStateControllerImpl(mUiEventLogger, mDumpManager, + mInteractionJankMonitor); mKeyguardStatusView = new KeyguardStatusView(mContext); mKeyguardStatusView.setId(R.id.keyguard_status_view); @@ -426,7 +430,8 @@ public class NotificationPanelViewControllerTest extends SysuiTestCase { NotificationWakeUpCoordinator coordinator = new NotificationWakeUpCoordinator( mock(HeadsUpManagerPhone.class), - new StatusBarStateControllerImpl(new UiEventLoggerFake(), mDumpManager), + new StatusBarStateControllerImpl(new UiEventLoggerFake(), mDumpManager, + mInteractionJankMonitor), mKeyguardBypassController, mDozeParameters, mUnlockedScreenOffAnimationController); @@ -465,8 +470,12 @@ public class NotificationPanelViewControllerTest extends SysuiTestCase { .thenReturn(mUserSwitcherView); when(mLayoutInflater.inflate(eq(R.layout.keyguard_bottom_area), any(), anyBoolean())) .thenReturn(mKeyguardBottomArea); - when(mNotificationRemoteInputManager.isRemoteInputActive()).thenReturn(false); - + when(mNotificationRemoteInputManager.isRemoteInputActive()) + .thenReturn(false); + when(mInteractionJankMonitor.begin(any(), anyInt())) + .thenReturn(true); + when(mInteractionJankMonitor.end(anyInt())) + .thenReturn(true); reset(mView); mNotificationPanelViewController = new NotificationPanelViewController(mView, @@ -520,7 +529,8 @@ public class NotificationPanelViewControllerTest extends SysuiTestCase { new PanelExpansionStateManager(), mNotificationRemoteInputManager, mSysUIUnfoldComponent, - mControlsComponent); + mControlsComponent, + mInteractionJankMonitor); mNotificationPanelViewController.initDependencies( mStatusBar, () -> {},