Merge "Add jank CUJ instrumentation to DialogLaunchAnimator" into tm-qpr-dev

2022-06-07 12:26:44 +00:00
parent 4c1edb45c5 9ae5815927
commit 4b1750247e
6 changed files with 102 additions and 32 deletions
--- a/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt
+++ b/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt
@@ -34,6 +34,9 @@ import android.view.WindowInsets
 import android.view.WindowManager
 import android.view.WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_ALWAYS
 import android.widget.FrameLayout
+import com.android.internal.jank.InteractionJankMonitor
+import com.android.internal.jank.InteractionJankMonitor.Configuration
+import com.android.internal.jank.InteractionJankMonitor.CujType
 import kotlin.math.roundToInt

 private const val TAG = "DialogLaunchAnimator"
@@ -50,6 +53,7 @@ private const val TAG = "DialogLaunchAnimator"
 */
 class DialogLaunchAnimator @JvmOverloads constructor(
    private val dreamManager: IDreamManager,
+    private val interactionJankMonitor: InteractionJankMonitor,
    private val launchAnimator: LaunchAnimator = LaunchAnimator(TIMINGS, INTERPOLATORS),
    private val isForTesting: Boolean = false
 ) {
@@ -89,12 +93,14 @@ class DialogLaunchAnimator @JvmOverloads constructor(
    fun showFromView(
        dialog: Dialog,
        view: View,
-        animateBackgroundBoundsChange: Boolean = false
+        cuj: DialogCuj? = null,
+        animateBackgroundBoundsChange: Boolean = false,
    ) {
        if (Looper.myLooper() != Looper.getMainLooper()) {
            throw IllegalStateException(
                "showFromView must be called from the main thread and dialog must be created in " +
-                    "the main thread")
+                    "the main thread"
+            )
        }

        // If the view we are launching from belongs to another dialog, then this means the caller
@@ -113,14 +119,16 @@ class DialogLaunchAnimator @JvmOverloads constructor(
        animateFrom.setTag(TAG_LAUNCH_ANIMATION_RUNNING, true)

        val animatedDialog = AnimatedDialog(
-                launchAnimator,
-                dreamManager,
-                animateFrom,
-                onDialogDismissed = { openedDialogs.remove(it) },
-                dialog = dialog,
-                animateBackgroundBoundsChange,
-                animatedParent,
-                isForTesting
+            launchAnimator,
+            dreamManager,
+            interactionJankMonitor,
+            animateFrom,
+            onDialogDismissed = { openedDialogs.remove(it) },
+            dialog = dialog,
+            animateBackgroundBoundsChange,
+            animatedParent,
+            isForTesting,
+            cuj
        )

        openedDialogs.add(animatedDialog)
@@ -143,8 +151,9 @@ class DialogLaunchAnimator @JvmOverloads constructor(
            ?.dialogContentWithBackground
            ?: throw IllegalStateException(
                "The animateFrom dialog was not animated using " +
-                    "DialogLaunchAnimator.showFrom(View|Dialog)")
-        showFromView(dialog, view, animateBackgroundBoundsChange)
+                    "DialogLaunchAnimator.showFrom(View|Dialog)"
+            )
+        showFromView(dialog, view, animateBackgroundBoundsChange = animateBackgroundBoundsChange)
    }

    /**
@@ -270,9 +279,17 @@ class DialogLaunchAnimator @JvmOverloads constructor(
    }
 }

+/**
+ * The CUJ interaction associated with opening the dialog.
+ *
+ * The optional tag indicates the specific dialog being opened.
+ */
+data class DialogCuj(@CujType val cujType: Int, val tag: String? = null)
+
 private class AnimatedDialog(
    private val launchAnimator: LaunchAnimator,
    private val dreamManager: IDreamManager,
+    private val interactionJankMonitor: InteractionJankMonitor,

    /** The view that triggered the dialog after being tapped. */
    var touchSurface: View,
@@ -295,14 +312,17 @@ private class AnimatedDialog(
    /**
     * Whether synchronization should be disabled, which can be useful if we are running in a test.
     */
-    private val forceDisableSynchronization: Boolean
+    private val forceDisableSynchronization: Boolean,
+
+    /** Interaction to which the dialog animation is associated. */
+    private val cuj: DialogCuj? = null
 ) {
    /**
     * The DecorView of this dialog window.
     *
     * Note that we access this DecorView lazily to avoid accessing it before the dialog is created,
     * which can sometimes cause crashes (e.g. with the Cast dialog).
-      */
+     */
    private val decorView by lazy { dialog.window!!.decorView as ViewGroup }

    /**
@@ -346,6 +366,14 @@ private class AnimatedDialog(
    private var decorViewLayoutListener: View.OnLayoutChangeListener? = null

    fun start() {
+        if (cuj != null) {
+            val config = Configuration.Builder.withView(cuj.cujType, touchSurface)
+            if (cuj.tag != null) {
+                config.setTag(cuj.tag)
+            }
+            interactionJankMonitor.begin(config)
+        }
+
        // Create the dialog so that its onCreate() method is called, which usually sets the dialog
        // content.
        dialog.create()
@@ -430,9 +458,10 @@ private class AnimatedDialog(
            // Make the window fullscreen and add a layout listener to ensure it stays fullscreen.
            window.setLayout(MATCH_PARENT, MATCH_PARENT)
            decorViewLayoutListener = View.OnLayoutChangeListener {
-                v, left, top, right, bottom, oldLeft, oldTop, oldRight, oldBottom ->
+                    v, left, top, right, bottom, oldLeft, oldTop, oldRight, oldBottom ->
                if (window.attributes.width != MATCH_PARENT ||
-                    window.attributes.height != MATCH_PARENT) {
+                    window.attributes.height != MATCH_PARENT
+                ) {
                    // The dialog size changed, copy its size to dialogContentWithBackground and
                    // make the dialog window full screen again.
                    val layoutParams = dialogContentWithBackground.layoutParams
@@ -606,6 +635,7 @@ private class AnimatedDialog(
                    dialogContentWithBackground!!
                        .addOnLayoutChangeListener(backgroundLayoutListener)
                }
+                cuj?.run { interactionJankMonitor.end(cujType) }
            }
        )
    }
--- a/packages/SystemUI/src/com/android/systemui/qs/tiles/DndTile.java
+++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/DndTile.java
@@ -175,7 +175,9 @@ public class DndTile extends QSTileImpl<BooleanState> {
                    mUiHandler.post(() -> {
                        Dialog dialog = makeZenModeDialog();
                        if (view != null) {
-                            mDialogLaunchAnimator.showFromView(dialog, view, false);
+                            mDialogLaunchAnimator.showFromView(dialog, view,
+                                    /* cuj= */ null,
+                                    /* animateBackgroundBoundsChange= */ false);
                        } else {
                            dialog.show();
                        }
--- a/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogFactory.kt
+++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/dialog/InternetDialogFactory.kt
@@ -19,7 +19,9 @@ import android.content.Context
 import android.os.Handler
 import android.util.Log
 import android.view.View
+import com.android.internal.jank.InteractionJankMonitor
 import com.android.internal.logging.UiEventLogger
+import com.android.systemui.animation.DialogCuj
 import com.android.systemui.animation.DialogLaunchAnimator
 import com.android.systemui.dagger.SysUISingleton
 import com.android.systemui.dagger.qualifiers.Background
@@ -45,6 +47,7 @@ class InternetDialogFactory @Inject constructor(
    private val keyguardStateController: KeyguardStateController
 ) {
    companion object {
+        private const val INTERACTION_JANK_TAG = "internet"
        var internetDialog: InternetDialog? = null
    }

@@ -61,12 +64,20 @@ class InternetDialogFactory @Inject constructor(
            }
            return
        } else {
-            internetDialog = InternetDialog(context, this, internetDialogController,
-                    canConfigMobileData, canConfigWifi, aboveStatusBar, uiEventLogger, handler,
-                    executor, keyguardStateController)
+            internetDialog = InternetDialog(
+                context, this, internetDialogController,
+                canConfigMobileData, canConfigWifi, aboveStatusBar, uiEventLogger, handler,
+                executor, keyguardStateController
+            )
            if (view != null) {
-                dialogLaunchAnimator.showFromView(internetDialog!!, view,
-                    animateBackgroundBoundsChange = true)
+                dialogLaunchAnimator.showFromView(
+                    internetDialog!!, view,
+                    animateBackgroundBoundsChange = true,
+                    cuj = DialogCuj(
+                        InteractionJankMonitor.CUJ_SHADE_DIALOG_OPEN,
+                        INTERACTION_JANK_TAG
+                    )
+                )
            } else {
                internetDialog?.show()
            }
--- a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/CentralSurfacesDependenciesModule.java
@@ -21,6 +21,7 @@ import android.content.Context;
 import android.os.Handler;
 import android.service.dreams.IDreamManager;

+import com.android.internal.jank.InteractionJankMonitor;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.keyguard.KeyguardUpdateMonitor;
 import com.android.systemui.animation.ActivityLaunchAnimator;
@@ -317,7 +318,8 @@ public interface CentralSurfacesDependenciesModule {
     */
    @Provides
    @SysUISingleton
-    static DialogLaunchAnimator provideDialogLaunchAnimator(IDreamManager dreamManager) {
-        return new DialogLaunchAnimator(dreamManager);
+    static DialogLaunchAnimator provideDialogLaunchAnimator(IDreamManager dreamManager,
+            InteractionJankMonitor interactionJankMonitor) {
+        return new DialogLaunchAnimator(dreamManager, interactionJankMonitor);
    }
 }
--- a/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt
@@ -15,6 +15,7 @@ import android.view.ViewGroup.LayoutParams.MATCH_PARENT
 import android.view.WindowManager
 import android.widget.LinearLayout
 import androidx.test.filters.SmallTest
+import com.android.internal.jank.InteractionJankMonitor
 import com.android.internal.policy.DecorView
 import com.android.systemui.SysuiTestCase
 import junit.framework.Assert.assertEquals
@@ -29,6 +30,8 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
+import org.mockito.Mockito.any
+import org.mockito.Mockito.verify
 import org.mockito.junit.MockitoJUnit

@SmallTest
@@ -40,12 +43,14 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
    private val attachedViews = mutableSetOf<View>()

    @Mock lateinit var dreamManager: IDreamManager
+    @Mock lateinit var interactionJankMonitor: InteractionJankMonitor
    @get:Rule val rule = MockitoJUnit.rule()

    @Before
    fun setUp() {
        dialogLaunchAnimator = DialogLaunchAnimator(
-            dreamManager, launchAnimator, isForTesting = true)
+            dreamManager, interactionJankMonitor, launchAnimator, isForTesting = true
+        )
    }

    @After
@@ -90,7 +95,8 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {

        // The dialog content is inside this fake window view.
        assertNotNull(
-            dialogContentWithBackground.findViewByPredicate { it === dialog.contentView })
+            dialogContentWithBackground.findViewByPredicate { it === dialog.contentView }
+        )

        // Clicking the transparent background should dismiss the dialog.
        runOnMainThreadAndWaitForIdleSync {
@@ -161,6 +167,24 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
        assertNotEquals(0, dialog.window.attributes.windowAnimations)
    }

+    @Test
+    fun testCujSpecificationLogsInteraction() {
+        val touchSurface = createTouchSurface()
+        return runOnMainThreadAndWaitForIdleSync {
+            val dialog = TestDialog(context)
+            dialogLaunchAnimator.showFromView(
+                dialog, touchSurface,
+                cuj = DialogCuj(InteractionJankMonitor.CUJ_SHADE_DIALOG_OPEN)
+            )
+        }
+
+        verify(interactionJankMonitor).begin(
+            any()
+        )
+        verify(interactionJankMonitor)
+            .end(InteractionJankMonitor.CUJ_SHADE_DIALOG_OPEN)
+    }
+
    private fun createAndShowDialog(): TestDialog {
        val touchSurface = createTouchSurface()
        return runOnMainThreadAndWaitForIdleSync {
@@ -224,4 +248,4 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
            window.setBackgroundDrawable(windowBackground)
        }
    }
-}
+}
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/DndTileTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tiles/DndTileTest.kt
@@ -38,9 +38,11 @@ import com.android.systemui.qs.logging.QSLogger
 import com.android.systemui.statusbar.policy.ZenModeController
 import com.android.systemui.util.mockito.any
 import com.android.systemui.util.mockito.eq
+import com.android.systemui.util.mockito.nullable
 import com.android.systemui.util.settings.FakeSettings
 import com.android.systemui.util.settings.SecureSettings
 import com.google.common.truth.Truth.assertThat
+import java.io.File
 import org.junit.After
 import org.junit.Before
 import org.junit.Test
@@ -49,9 +51,8 @@ import org.mockito.Mock
 import org.mockito.Mockito.anyBoolean
 import org.mockito.Mockito.never
 import org.mockito.Mockito.verify
-import org.mockito.MockitoAnnotations
-import java.io.File
 import org.mockito.Mockito.`when` as whenever
+import org.mockito.MockitoAnnotations

@SmallTest
@RunWith(AndroidTestingRunner::class)
@@ -173,7 +174,7 @@ class DndTileTest : SysuiTestCase() {
        tile.handleClick(view)
        testableLooper.processAllMessages()

-        verify(dialogLaunchAnimator).showFromView(any(), eq(view), anyBoolean())
+        verify(dialogLaunchAnimator).showFromView(any(), eq(view), nullable(), anyBoolean())
    }

    @Test
@@ -187,6 +188,6 @@ class DndTileTest : SysuiTestCase() {
        tile.handleClick(view)
        testableLooper.processAllMessages()

-        verify(dialogLaunchAnimator, never()).showFromView(any(), any(), anyBoolean())
+        verify(dialogLaunchAnimator, never()).showFromView(any(), any(), nullable(), anyBoolean())
    }
-}
+}