Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 41998f86 authored by Ahan Wu's avatar Ahan Wu
Browse files

Avoid too long trace section name to crash FrameTracker 

android.os.Trace#beginSection limits the length of the section to 127,
otherwise, a IllegalArgumentException throws. We limit the length before
passing the argument to avoid this.

Bug: 239860117
Test: atest InteractionJankMonitorTest FrameTrackerTest
Change-Id: I3d9cbc65a8036c18680edf11aace0b324a1befb6
parent 4cb516d8

core/java/com/android/internal/jank/FrameTracker.java

+14 −1
Original line number Diff line number Diff line
@@ -66,6 +66,8 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
    private static final long INVALID_ID = -1;

    public static final int NANOS_IN_MILLISECOND = 1_000_000;



    private static final int MAX_LENGTH_EVENT_DESC = 20;



    static final int REASON_END_UNKNOWN = -1;

    static final int REASON_END_NORMAL = 0;

    static final int REASON_END_SURFACE_DESTROYED = 1;
@@ -394,7 +396,18 @@ public class FrameTracker extends SurfaceControl.OnJankDataListener 
        return true;

    }



    private void markEvent(String desc) {

    /**

     * Mark the FrameTracker events in the trace.

     *

     * @param desc The description of the trace event,

     *            shouldn't exceed {@link #MAX_LENGTH_EVENT_DESC}.

     */

    private void markEvent(@NonNull String desc) {

        if (desc.length() > MAX_LENGTH_EVENT_DESC) {

            throw new IllegalArgumentException(TextUtils.formatSimple(

                    "The length of the trace event description <%s> exceeds %d",

                    desc, MAX_LENGTH_EVENT_DESC));

        }

        Trace.beginSection(TextUtils.formatSimple("%s#%s", mSession.getName(), desc));

        Trace.endSection();

    }

core/java/com/android/internal/jank/InteractionJankMonitor.java

+28 −3
Original line number Diff line number Diff line
@@ -149,6 +149,10 @@ public class InteractionJankMonitor { 
    private static final int DEFAULT_TRACE_THRESHOLD_MISSED_FRAMES = 3;

    private static final int DEFAULT_TRACE_THRESHOLD_FRAME_TIME_MILLIS = 64;



    @VisibleForTesting

    public static final int MAX_LENGTH_OF_CUJ_NAME = 80;

    private static final int MAX_LENGTH_SESSION_NAME = 100;



    public static final String ACTION_SESSION_END = ACTION_PREFIX + ".ACTION_SESSION_END";

    public static final String ACTION_SESSION_CANCEL = ACTION_PREFIX + ".ACTION_SESSION_CANCEL";
@@ -732,6 +736,9 @@ public class InteractionJankMonitor { 
     * @return the name of the cuj type

     */

    public static String getNameOfCuj(int cujType) {

        // Please note:

        // 1. The length of the returned string shouldn't exceed MAX_LENGTH_OF_CUJ_NAME.

        // 2. The returned string should be the same with the name defined in atoms.proto.

        switch (cujType) {

            case CUJ_NOTIFICATION_SHADE_EXPAND_COLLAPSE:

                return "SHADE_EXPAND_COLLAPSE";
@@ -1106,9 +1113,27 @@ public class InteractionJankMonitor { 
        public Session(@CujType int cujType, @NonNull String postfix) {

            mCujType = cujType;

            mTimeStamp = System.nanoTime();

            mName = TextUtils.isEmpty(postfix)

                    ? String.format("J<%s>", getNameOfCuj(mCujType))

                    : String.format("J<%s::%s>", getNameOfCuj(mCujType), postfix);

            mName = generateSessionName(getNameOfCuj(cujType), postfix);

        }



        private String generateSessionName(@NonNull String cujName, @NonNull String cujPostfix) {

            final boolean hasPostfix = !TextUtils.isEmpty(cujPostfix);

            // We assert that the cujName shouldn't exceed MAX_LENGTH_OF_CUJ_NAME.

            if (cujName.length() > MAX_LENGTH_OF_CUJ_NAME) {

                throw new IllegalArgumentException(TextUtils.formatSimple(

                        "The length of cuj name <%s> exceeds %d", cujName, MAX_LENGTH_OF_CUJ_NAME));

            }

            if (hasPostfix) {

                final int remaining = MAX_LENGTH_SESSION_NAME - cujName.length();

                if (cujPostfix.length() > remaining) {

                    cujPostfix = cujPostfix.substring(0, remaining - 3).concat("...");

                }

            }

            // The max length of the whole string should be:

            // 105 with postfix, 83 without postfix

            return hasPostfix

                    ? TextUtils.formatSimple("J<%s::%s>", cujName, cujPostfix)

                    : TextUtils.formatSimple("J<%s>", cujName);

        }



        @CujType

core/tests/coretests/src/com/android/internal/jank/InteractionJankMonitorTest.java

+158 −0
Original line number Diff line number Diff line
@@ -16,10 +16,26 @@ 


package com.android.internal.jank;



import static android.text.TextUtils.formatSimple;



import static com.android.internal.jank.FrameTracker.REASON_CANCEL_TIMEOUT;

import static com.android.internal.jank.FrameTracker.REASON_END_NORMAL;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_LOCKSCREEN_LAUNCH_CAMERA;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_ADD;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_APP_START;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_HEADS_UP_APPEAR;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_HEADS_UP_DISAPPEAR;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_REMOVE;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_SHADE_EXPAND_COLLAPSE;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_SHADE_QS_SCROLL_SWIPE;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_SHADE_ROW_EXPAND;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_SHADE_ROW_SWIPE;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_NOTIFICATION_SHADE_SCROLL_FLING;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_SPLIT_SCREEN_RESIZE;

import static com.android.internal.jank.InteractionJankMonitor.CUJ_TO_STATSD_INTERACTION_TYPE;

import static com.android.internal.jank.InteractionJankMonitor.MAX_LENGTH_OF_CUJ_NAME;

import static com.android.internal.jank.InteractionJankMonitor.getNameOfCuj;



import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;
@@ -38,6 +54,7 @@ import android.os.Handler; 
import android.os.HandlerThread;

import android.os.SystemClock;

import android.provider.DeviceConfig;

import android.util.SparseArray;

import android.view.View;

import android.view.ViewAttachTestActivity;
@@ -52,8 +69,12 @@ import com.android.internal.jank.FrameTracker.ThreadedRendererWrapper; 
import com.android.internal.jank.FrameTracker.ViewRootWrapper;

import com.android.internal.jank.InteractionJankMonitor.Configuration;

import com.android.internal.jank.InteractionJankMonitor.Session;

import com.android.internal.util.FrameworkStatsLog;



import com.google.common.truth.Expect;



import org.junit.Before;

import org.junit.BeforeClass;

import org.junit.Rule;

import org.junit.Test;

import org.mockito.ArgumentCaptor;
@@ -64,11 +85,17 @@ import java.util.Arrays; 
import java.util.HashMap;

import java.util.HashSet;

import java.util.List;

import java.util.Map;

import java.util.stream.Collectors;



@SmallTest

public class InteractionJankMonitorTest {

    private static final String CUJ_POSTFIX = "";

    private static final SparseArray<String> ENUM_NAME_EXCEPTION_MAP = new SparseArray<>();

    private static final SparseArray<String> CUJ_NAME_EXCEPTION_MAP = new SparseArray<>();

    private static final String ENUM_NAME_PREFIX =

            "UIINTERACTION_FRAME_INFO_REPORTED__INTERACTION_TYPE__";



    private ViewAttachTestActivity mActivity;

    private View mView;

    private HandlerThread mWorker;
@@ -77,6 +104,53 @@ public class InteractionJankMonitorTest { 
    public ActivityTestRule<ViewAttachTestActivity> mRule =

            new ActivityTestRule<>(ViewAttachTestActivity.class);



    @Rule

    public final Expect mExpect = Expect.create();



    @BeforeClass

    public static void initialize() {

        ENUM_NAME_EXCEPTION_MAP.put(CUJ_NOTIFICATION_ADD, getEnumName("SHADE_NOTIFICATION_ADD"));

        ENUM_NAME_EXCEPTION_MAP.put(CUJ_NOTIFICATION_APP_START, getEnumName("SHADE_APP_LAUNCH"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_HEADS_UP_APPEAR, getEnumName("SHADE_HEADS_UP_APPEAR"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_HEADS_UP_DISAPPEAR, getEnumName("SHADE_HEADS_UP_DISAPPEAR"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_REMOVE, getEnumName("SHADE_NOTIFICATION_REMOVE"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_EXPAND_COLLAPSE, getEnumName("NOTIFICATION_SHADE_SWIPE"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE, getEnumName("SHADE_QS_EXPAND_COLLAPSE"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_QS_SCROLL_SWIPE, getEnumName("SHADE_QS_SCROLL_SWIPE"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_ROW_EXPAND, getEnumName("SHADE_ROW_EXPAND"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_ROW_SWIPE, getEnumName("SHADE_ROW_SWIPE"));

        ENUM_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_SCROLL_FLING, getEnumName("SHADE_SCROLL_FLING"));



        CUJ_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_EXPAND_COLLAPSE, "CUJ_NOTIFICATION_SHADE_EXPAND_COLLAPSE");

        CUJ_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE,

                "CUJ_NOTIFICATION_SHADE_QS_EXPAND_COLLAPSE");

        CUJ_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_QS_SCROLL_SWIPE, "CUJ_NOTIFICATION_SHADE_QS_SCROLL_SWIPE");

        CUJ_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_ROW_EXPAND, "CUJ_NOTIFICATION_SHADE_ROW_EXPAND");

        CUJ_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_ROW_SWIPE, "CUJ_NOTIFICATION_SHADE_ROW_SWIPE");

        CUJ_NAME_EXCEPTION_MAP.put(

                CUJ_NOTIFICATION_SHADE_SCROLL_FLING, "CUJ_NOTIFICATION_SHADE_SCROLL_FLING");

        CUJ_NAME_EXCEPTION_MAP.put(CUJ_LOCKSCREEN_LAUNCH_CAMERA, "CUJ_LOCKSCREEN_LAUNCH_CAMERA");

        CUJ_NAME_EXCEPTION_MAP.put(CUJ_SPLIT_SCREEN_RESIZE, "CUJ_SPLIT_SCREEN_RESIZE");

    }



    private static String getEnumName(String name) {

        return formatSimple("%s%s", ENUM_NAME_PREFIX, name);

    }



    @Before

    public void setup() {

        // Prepare an activity for getting ThreadedRenderer later.
@@ -174,6 +248,82 @@ public class InteractionJankMonitorTest { 
        }

    }



    @Test

    public void testCujsMapToEnumsCorrectly() {

        List<Field> cujs = Arrays.stream(InteractionJankMonitor.class.getDeclaredFields())

                .filter(f -> f.getName().startsWith("CUJ_")

                        && Modifier.isStatic(f.getModifiers())

                        && f.getType() == int.class)

                .collect(Collectors.toList());



        Map<Integer, String> enumsMap = Arrays.stream(FrameworkStatsLog.class.getDeclaredFields())

                .filter(f -> f.getName().startsWith(ENUM_NAME_PREFIX)

                        && Modifier.isStatic(f.getModifiers())

                        && f.getType() == int.class)

                .collect(Collectors.toMap(this::getIntFieldChecked, Field::getName));



        cujs.forEach(f -> {

            final int cuj = getIntFieldChecked(f);

            final String cujName = f.getName();

            final String expectedEnumName = ENUM_NAME_EXCEPTION_MAP.contains(cuj)

                    ? ENUM_NAME_EXCEPTION_MAP.get(cuj)

                    : formatSimple("%s%s", ENUM_NAME_PREFIX, cujName.substring(4));

            final int enumKey = CUJ_TO_STATSD_INTERACTION_TYPE[cuj];

            final String enumName = enumsMap.get(enumKey);

            final String expectedNameOfCuj = CUJ_NAME_EXCEPTION_MAP.contains(cuj)

                    ? CUJ_NAME_EXCEPTION_MAP.get(cuj)

                    : formatSimple("CUJ_%s", getNameOfCuj(cuj));

            mExpect

                    .withMessage(formatSimple(

                            "%s (%d) not matches %s (%d)", cujName, cuj, enumName, enumKey))

                    .that(expectedEnumName.equals(enumName))

                    .isTrue();

            mExpect

                    .withMessage(formatSimple("getNameOfCuj(%d) not matches %s", cuj, cujName))

                    .that(cujName.equals(expectedNameOfCuj))

                    .isTrue();

        });

    }



    @Test

    public void testCujNameLimit() {

        Arrays.stream(InteractionJankMonitor.class.getDeclaredFields())

                .filter(f -> f.getName().startsWith("CUJ_")

                        && Modifier.isStatic(f.getModifiers())

                        && f.getType() == int.class)

                .forEach(f -> {

                    final int cuj = getIntFieldChecked(f);

                    mExpect

                            .withMessage(formatSimple(

                                    "Too long CUJ(%d) name: %s", cuj, getNameOfCuj(cuj)))

                            .that(getNameOfCuj(cuj).length())

                            .isAtMost(MAX_LENGTH_OF_CUJ_NAME);

                });

    }



    @Test

    public void testSessionNameLengthLimit() {

        final int cujType = CUJ_NOTIFICATION_SHADE_EXPAND_COLLAPSE;

        final String cujName = getNameOfCuj(cujType);

        final String cujTag = "ThisIsTheCujTag";

        final String tooLongTag = cujTag.repeat(10);



        // Normal case, no postfix.

        Session noPostfix = new Session(cujType, "");

        assertThat(noPostfix.getName()).isEqualTo(formatSimple("J<%s>", cujName));



        // Normal case, with postfix.

        Session withPostfix = new Session(cujType, cujTag);

        assertThat(withPostfix.getName()).isEqualTo(formatSimple("J<%s::%s>", cujName, cujTag));



        // Since the length of the cuj name is tested in another test, no need to test it here.

        // Too long postfix case, should trim the postfix and keep the cuj name completed.

        final String expectedTrimmedName = formatSimple("J<%s::%s>", cujName,

                "ThisIsTheCujTagThisIsTheCujTagThisIsTheCujTagThisIsTheCujTagThisIsTheCujTagT...");

        Session longPostfix = new Session(cujType, tooLongTag);

        assertThat(longPostfix.getName()).isEqualTo(expectedTrimmedName);

    }



    private InteractionJankMonitor createMockedInteractionJankMonitor() {

        InteractionJankMonitor monitor = spy(new InteractionJankMonitor(mWorker));

        doReturn(true).when(monitor).shouldMonitor(anyInt());
@@ -217,4 +367,12 @@ public class InteractionJankMonitorTest { 


        return tracker;

    }



    private int getIntFieldChecked(Field field) {

        try {

            return field.getInt(null);

        } catch (IllegalAccessException ex) {

            throw new RuntimeException(ex);

        }

    }

}