Merge "Avoid too long trace section name to crash FrameTracker" into tm-qpr-dev

This commit is contained in:
TreeHugger Robot
2022-08-05 19:54:14 +00:00
committed by Android (Google) Code Review
3 changed files with 200 additions and 4 deletions

View File

@@ -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();
}

View File

@@ -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

View File

@@ -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);
}
}
}