Merge "fix flaky behavior in FakeLatencyTracker" into udc-dev (819b087a) · Commits · e / os / android_frameworks_base

core/java/com/android/internal/util/LatencyTracker.java

+61 −21

Original line number	Diff line number	Diff line
		@@ -305,10 +305,17 @@ public class LatencyTracker {
		private final SparseArray<ActionProperties> mActionPropertiesMap = new SparseArray<>();
		@GuardedBy("mLock")
		private boolean mEnabled;
		private final DeviceConfig.OnPropertiesChangedListener mOnPropertiesChangedListener =
		this::updateProperties;

		// Wrapping this in a holder class achieves lazy loading behavior
		private static final class SLatencyTrackerHolder {
		private static final LatencyTracker sLatencyTracker = new LatencyTracker();
		private static final LatencyTracker sLatencyTracker;

		static {
		sLatencyTracker = new LatencyTracker();
		sLatencyTracker.startListeningForLatencyTrackerConfigChanges();
		}
		}

		public static LatencyTracker getInstance(Context context) {
		@@ -319,12 +326,48 @@ public class LatencyTracker {
		* Constructor for LatencyTracker
		*
		* <p>This constructor is only visible for test classes to inject their own consumer callbacks
		*
		* @param startListeningForPropertyChanges If set, constructor will register for device config
		* property updates prior to returning. If not set,
		* {@link #startListeningForLatencyTrackerConfigChanges} must be called
		* to start listening.
		*/
		@RequiresPermission(Manifest.permission.READ_DEVICE_CONFIG)
		@VisibleForTesting
		public LatencyTracker() {
		mEnabled = DEFAULT_ENABLED;
		}

		private void updateProperties(DeviceConfig.Properties properties) {
		synchronized (mLock) {
		int samplingInterval = properties.getInt(SETTINGS_SAMPLING_INTERVAL_KEY,
		DEFAULT_SAMPLING_INTERVAL);
		mEnabled = properties.getBoolean(SETTINGS_ENABLED_KEY, DEFAULT_ENABLED);
		for (int action : ACTIONS_ALL) {
		String actionName = getNameOfAction(STATSD_ACTION[action]).toLowerCase(Locale.ROOT);
		int legacyActionTraceThreshold = properties.getInt(
		actionName + LEGACY_TRACE_THRESHOLD_SUFFIX, -1);
		mActionPropertiesMap.put(action, new ActionProperties(action,
		properties.getBoolean(actionName + ENABLE_SUFFIX, mEnabled),
		properties.getInt(actionName + SAMPLE_INTERVAL_SUFFIX, samplingInterval),
		properties.getInt(actionName + TRACE_THRESHOLD_SUFFIX,
		legacyActionTraceThreshold)));
		}
		onDeviceConfigPropertiesUpdated(mActionPropertiesMap);
		}
		}

		/**
		* Test method to start listening to {@link DeviceConfig} properties changes.
		*
		* <p>During testing, a {@link LatencyTracker} it is desired to stop and start listening for
		* config updates.
		*
		* <p>This is not used for production usages of this class outside of testing as we are
		* using a single static object.
		*/
		@VisibleForTesting
		public void startListeningForLatencyTrackerConfigChanges() {
		final Context context = ActivityThread.currentApplication();
		if (context != null
		&& context.checkCallingOrSelfPermission(READ_DEVICE_CONFIG) == PERMISSION_GRANTED) {
		@@ -332,12 +375,13 @@ public class LatencyTracker {
		BackgroundThread.getHandler().post(() -> this.updateProperties(
		DeviceConfig.getProperties(NAMESPACE_LATENCY_TRACKER)));
		DeviceConfig.addOnPropertiesChangedListener(NAMESPACE_LATENCY_TRACKER,
		BackgroundThread.getExecutor(), this::updateProperties);
		BackgroundThread.getExecutor(), mOnPropertiesChangedListener);
		} else {
		if (DEBUG) {
		if (context == null) {
		Log.d(TAG, "No application for " + ActivityThread.currentActivityThread());
		} else {
		synchronized (mLock) {
		Log.d(TAG, "Initialized the LatencyTracker."
		+ " (No READ_DEVICE_CONFIG permission to change configs)"
		+ " enabled=" + mEnabled + ", package=" + context.getPackageName());
		@@ -345,24 +389,20 @@ public class LatencyTracker {
		}
		}
		}

		private void updateProperties(DeviceConfig.Properties properties) {
		synchronized (mLock) {
		int samplingInterval = properties.getInt(SETTINGS_SAMPLING_INTERVAL_KEY,
		DEFAULT_SAMPLING_INTERVAL);
		mEnabled = properties.getBoolean(SETTINGS_ENABLED_KEY, DEFAULT_ENABLED);
		for (int action : ACTIONS_ALL) {
		String actionName = getNameOfAction(STATSD_ACTION[action]).toLowerCase(Locale.ROOT);
		int legacyActionTraceThreshold = properties.getInt(
		actionName + LEGACY_TRACE_THRESHOLD_SUFFIX, -1);
		mActionPropertiesMap.put(action, new ActionProperties(action,
		properties.getBoolean(actionName + ENABLE_SUFFIX, mEnabled),
		properties.getInt(actionName + SAMPLE_INTERVAL_SUFFIX, samplingInterval),
		properties.getInt(actionName + TRACE_THRESHOLD_SUFFIX,
		legacyActionTraceThreshold)));
		}
		onDeviceConfigPropertiesUpdated(mActionPropertiesMap);
		}

		/**
		* Test method to stop listening to {@link DeviceConfig} properties changes.
		*
		* <p>During testing, a {@link LatencyTracker} it is desired to stop and start listening for
		* config updates.
		*
		* <p>This is not used for production usages of this class outside of testing as we are
		* using a single static object.
		*/
		@VisibleForTesting
		public void stopListeningForLatencyTrackerConfigChanges() {
		DeviceConfig.removeOnPropertiesChangedListener(mOnPropertiesChangedListener);
		}

		/**

core/tests/coretests/src/com/android/internal/util/LatencyTrackerTest.java

+6 −2

Original line number	Diff line number	Diff line
		@@ -28,12 +28,12 @@ import static com.google.common.truth.Truth.assertWithMessage;
		import android.provider.DeviceConfig;

		import androidx.test.ext.junit.runners.AndroidJUnit4;
		import androidx.test.filters.SmallTest;

		import com.android.internal.util.LatencyTracker.ActionProperties;

		import com.google.common.truth.Expect;

		import org.junit.After;
		import org.junit.Before;
		import org.junit.Rule;
		import org.junit.Test;
		@@ -48,7 +48,6 @@ import java.util.List;
		import java.util.Map;
		import java.util.stream.Collectors;

		@SmallTest
		@RunWith(AndroidJUnit4.class)
		public class LatencyTrackerTest {
		private static final String ENUM_NAME_PREFIX = "UIACTION_LATENCY_REPORTED__ACTION__";
		@@ -65,6 +64,11 @@ public class LatencyTrackerTest {
		mLatencyTracker = FakeLatencyTracker.create();
		}

		@After
		public void tearDown() {
		mLatencyTracker.stopListeningForLatencyTrackerConfigChanges();
		}

		@Test
		public void testCujsMapToEnumsCorrectly() {
		List<Field> actions = getAllActionFields();

core/tests/coretests/testdoubles/src/com/android/internal/util/FakeLatencyTracker.java

+76 −103

Original line number	Diff line number	Diff line
		@@ -25,8 +25,6 @@ import android.provider.DeviceConfig;
		import android.util.Log;
		import android.util.SparseArray;

		import androidx.annotation.Nullable;

		import com.android.internal.annotations.GuardedBy;

		import com.google.common.collect.ImmutableMap;
		@@ -51,15 +49,17 @@ public final class FakeLatencyTracker extends LatencyTracker {
		private final List<String> mPerfettoTraceNamesTriggered;
		private final AtomicReference<SparseArray<ActionProperties>> mLastPropertiesUpdate =
		new AtomicReference<>();
		@Nullable
		@GuardedBy("mLock")
		private Callable<Boolean> mShouldClosePropertiesUpdatedCallable = null;
		private final AtomicReference<Callable<Boolean>> mShouldClosePropertiesUpdatedCallable =
		new AtomicReference<>();
		private final ConditionVariable mDeviceConfigPropertiesUpdated = new ConditionVariable();

		public static FakeLatencyTracker create() throws Exception {
		Log.i(TAG, "create");
		disableForAllActions();
		Log.i(TAG, "done disabling all actions");
		FakeLatencyTracker fakeLatencyTracker = new FakeLatencyTracker();
		Log.i(TAG, "done creating tracker object");
		fakeLatencyTracker.startListeningForLatencyTrackerConfigChanges();
		// always return the fake in the disabled state and let the client control the desired state
		fakeLatencyTracker.waitForGlobalEnabledState(false);
		fakeLatencyTracker.waitForAllPropertiesEnableState(false);
		@@ -131,17 +131,16 @@ public final class FakeLatencyTracker extends LatencyTracker {
		@Override
		public void onDeviceConfigPropertiesUpdated(SparseArray<ActionProperties> actionProperties) {
		Log.d(TAG, "onDeviceConfigPropertiesUpdated: " + actionProperties);

		mLastPropertiesUpdate.set(actionProperties);
		synchronized (mLock) {
		if (mShouldClosePropertiesUpdatedCallable != null) {
		Callable<Boolean> shouldClosePropertiesUpdated =
		mShouldClosePropertiesUpdatedCallable.get();
		if (shouldClosePropertiesUpdated != null) {
		try {
		boolean shouldClosePropertiesUpdated =
		mShouldClosePropertiesUpdatedCallable.call();
		Log.i(TAG, "shouldClosePropertiesUpdatedCallable callable result="
		+ shouldClosePropertiesUpdated);
		if (shouldClosePropertiesUpdated) {
		Log.i(TAG, "shouldClosePropertiesUpdatedCallable=true, opening condition");
		mShouldClosePropertiesUpdatedCallable = null;
		boolean result = shouldClosePropertiesUpdated.call();
		Log.i(TAG, "shouldClosePropertiesUpdatedCallable callable result=" + result);
		if (result) {
		mShouldClosePropertiesUpdatedCallable.set(null);
		mDeviceConfigPropertiesUpdated.open();
		}
		} catch (Exception e) {
		@@ -153,7 +152,6 @@ public final class FakeLatencyTracker extends LatencyTracker {
		mDeviceConfigPropertiesUpdated.open();
		}
		}
		}

		@Override
		public void onTriggerPerfetto(String triggerName) {
		@@ -175,13 +173,10 @@ public final class FakeLatencyTracker extends LatencyTracker {

		public void waitForAllPropertiesEnableState(boolean enabledState) throws Exception {
		Log.i(TAG, "waitForAllPropertiesEnableState: enabledState=" + enabledState);
		synchronized (mLock) {
		Log.i(TAG, "closing condition");
		mDeviceConfigPropertiesUpdated.close();
		// Update the callable to only close the properties updated condition when all the
		// desired properties have been updated. The DeviceConfig callbacks may happen multiple
		// times so testing the resulting updates is required.
		mShouldClosePropertiesUpdatedCallable = () -> {
		waitForPropertiesCondition(() -> {
		Log.i(TAG, "verifying if last properties update has all properties enable="
		+ enabledState);
		SparseArray<ActionProperties> newProperties = mLastPropertiesUpdate.get();
		@@ -193,25 +188,16 @@ public final class FakeLatencyTracker extends LatencyTracker {
		}
		}
		return true;
		};
		if (mShouldClosePropertiesUpdatedCallable.call()) {
		return;
		}
		}
		Log.i(TAG, "waiting for condition");
		mDeviceConfigPropertiesUpdated.block();
		});
		}

		public void waitForMatchingActionProperties(ActionProperties actionProperties)
		throws Exception {
		Log.i(TAG, "waitForMatchingActionProperties: actionProperties=" + actionProperties);
		synchronized (mLock) {
		Log.i(TAG, "closing condition");
		mDeviceConfigPropertiesUpdated.close();
		// Update the callable to only close the properties updated condition when all the
		// desired properties have been updated. The DeviceConfig callbacks may happen multiple
		// times so testing the resulting updates is required.
		mShouldClosePropertiesUpdatedCallable = () -> {
		waitForPropertiesCondition(() -> {
		Log.i(TAG, "verifying if last properties update contains matching property ="
		+ actionProperties);
		SparseArray<ActionProperties> newProperties = mLastPropertiesUpdate.get();
		@@ -222,25 +208,16 @@ public final class FakeLatencyTracker extends LatencyTracker {
		}
		}
		return false;
		};
		if (mShouldClosePropertiesUpdatedCallable.call()) {
		return;
		}
		}
		Log.i(TAG, "waiting for condition");
		mDeviceConfigPropertiesUpdated.block();
		});
		}

		public void waitForActionEnabledState(int action, boolean enabledState) throws Exception {
		Log.i(TAG, "waitForActionEnabledState:"
		+ " action=" + action + ", enabledState=" + enabledState);
		synchronized (mLock) {
		Log.i(TAG, "closing condition");
		mDeviceConfigPropertiesUpdated.close();
		// Update the callable to only close the properties updated condition when all the
		// desired properties have been updated. The DeviceConfig callbacks may happen multiple
		// times so testing the resulting updates is required.
		mShouldClosePropertiesUpdatedCallable = () -> {
		waitForPropertiesCondition(() -> {
		Log.i(TAG, "verifying if last properties update contains action=" + action
		+ ", enabledState=" + enabledState);
		SparseArray<ActionProperties> newProperties = mLastPropertiesUpdate.get();
		@@ -250,32 +227,28 @@ public final class FakeLatencyTracker extends LatencyTracker {
		}
		}
		return false;
		};
		if (mShouldClosePropertiesUpdatedCallable.call()) {
		return;
		}
		}
		Log.i(TAG, "waiting for condition");
		mDeviceConfigPropertiesUpdated.block();
		});
		}

		public void waitForGlobalEnabledState(boolean enabledState) throws Exception {
		Log.i(TAG, "waitForGlobalEnabledState: enabledState=" + enabledState);
		synchronized (mLock) {
		Log.i(TAG, "closing condition");
		mDeviceConfigPropertiesUpdated.close();
		// Update the callable to only close the properties updated condition when all the
		// desired properties have been updated. The DeviceConfig callbacks may happen multiple
		// times so testing the resulting updates is required.
		mShouldClosePropertiesUpdatedCallable = () -> {
		waitForPropertiesCondition(() -> {
		//noinspection deprecation
		return isEnabled() == enabledState;
		};
		if (mShouldClosePropertiesUpdatedCallable.call()) {
		return;
		});
		}
		}
		Log.i(TAG, "waiting for condition");

		public void waitForPropertiesCondition(Callable<Boolean> shouldClosePropertiesUpdatedCallable)
		throws Exception {
		mShouldClosePropertiesUpdatedCallable.set(shouldClosePropertiesUpdatedCallable);
		mDeviceConfigPropertiesUpdated.close();
		if (!shouldClosePropertiesUpdatedCallable.call()) {
		Log.i(TAG, "waiting for mDeviceConfigPropertiesUpdated condition");
		mDeviceConfigPropertiesUpdated.block();
		}
		Log.i(TAG, "waitForPropertiesCondition: returning");
		}
		}

services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLoggingLatencyTest.java

+0 −6

Original line number	Diff line number	Diff line
		@@ -38,7 +38,6 @@ import android.os.BatteryStatsInternal;
		import android.os.Process;
		import android.os.RemoteException;

		import androidx.test.filters.FlakyTest;
		import androidx.test.platform.app.InstrumentationRegistry;

		import com.android.internal.util.FakeLatencyTracker;
		@@ -92,12 +91,10 @@ public class SoundTriggerMiddlewareLoggingLatencyTest {
		}

		@Test
		@FlakyTest(bugId = 275113847)
		public void testSetUpAndTearDown() {
		}

		@Test
		@FlakyTest(bugId = 275113847)
		public void testOnPhraseRecognitionStartsLatencyTrackerWithSuccessfulPhraseIdTrigger()
		throws RemoteException {
		ArgumentCaptor<ISoundTriggerCallback> soundTriggerCallbackCaptor = ArgumentCaptor.forClass(
		@@ -114,7 +111,6 @@ public class SoundTriggerMiddlewareLoggingLatencyTest {
		}

		@Test
		@FlakyTest(bugId = 275113847)
		public void testOnPhraseRecognitionRestartsActiveSession() throws RemoteException {
		ArgumentCaptor<ISoundTriggerCallback> soundTriggerCallbackCaptor = ArgumentCaptor.forClass(
		ISoundTriggerCallback.class);
		@@ -135,7 +131,6 @@ public class SoundTriggerMiddlewareLoggingLatencyTest {
		}

		@Test
		@FlakyTest(bugId = 275113847)
		public void testOnPhraseRecognitionNeverStartsLatencyTrackerWithNonSuccessEvent()
		throws RemoteException {
		ArgumentCaptor<ISoundTriggerCallback> soundTriggerCallbackCaptor = ArgumentCaptor.forClass(
		@@ -153,7 +148,6 @@ public class SoundTriggerMiddlewareLoggingLatencyTest {
		}

		@Test
		@FlakyTest(bugId = 275113847)
		public void testOnPhraseRecognitionNeverStartsLatencyTrackerWithNoKeyphraseId()
		throws RemoteException {
		ArgumentCaptor<ISoundTriggerCallback> soundTriggerCallbackCaptor = ArgumentCaptor.forClass(