Merge "fix flaky behavior in FakeLatencyTracker" into udc-dev am: 819b087abc (7ecd2ca5) · Commits · e / os / android_frameworks_base

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

+61 −21

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

	// Wrapping this in a holder class achieves lazy loading behavior		// Wrapping this in a holder class achieves lazy loading behavior
	private static final class SLatencyTrackerHolder {		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) {		public static LatencyTracker getInstance(Context context) {
	@@ -319,12 +326,48 @@ public class LatencyTracker {
	* Constructor for LatencyTracker		* Constructor for LatencyTracker
	*		*
	* <p>This constructor is only visible for test classes to inject their own consumer callbacks		* <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)		@RequiresPermission(Manifest.permission.READ_DEVICE_CONFIG)
	@VisibleForTesting		@VisibleForTesting
	public LatencyTracker() {		public LatencyTracker() {
	mEnabled = DEFAULT_ENABLED;		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();		final Context context = ActivityThread.currentApplication();
	if (context != null		if (context != null
	&& context.checkCallingOrSelfPermission(READ_DEVICE_CONFIG) == PERMISSION_GRANTED) {		&& context.checkCallingOrSelfPermission(READ_DEVICE_CONFIG) == PERMISSION_GRANTED) {
	@@ -332,12 +375,13 @@ public class LatencyTracker {
	BackgroundThread.getHandler().post(() -> this.updateProperties(		BackgroundThread.getHandler().post(() -> this.updateProperties(
	DeviceConfig.getProperties(NAMESPACE_LATENCY_TRACKER)));		DeviceConfig.getProperties(NAMESPACE_LATENCY_TRACKER)));
	DeviceConfig.addOnPropertiesChangedListener(NAMESPACE_LATENCY_TRACKER,		DeviceConfig.addOnPropertiesChangedListener(NAMESPACE_LATENCY_TRACKER,
	BackgroundThread.getExecutor(), this::updateProperties);		BackgroundThread.getExecutor(), mOnPropertiesChangedListener);
	} else {		} else {
	if (DEBUG) {		if (DEBUG) {
	if (context == null) {		if (context == null) {
	Log.d(TAG, "No application for " + ActivityThread.currentActivityThread());		Log.d(TAG, "No application for " + ActivityThread.currentActivityThread());
	} else {		} else {
			synchronized (mLock) {
	Log.d(TAG, "Initialized the LatencyTracker."		Log.d(TAG, "Initialized the LatencyTracker."
	+ " (No READ_DEVICE_CONFIG permission to change configs)"		+ " (No READ_DEVICE_CONFIG permission to change configs)"
	+ " enabled=" + mEnabled + ", package=" + context.getPackageName());		+ " 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	Original line	Diff line number	Diff line
	@@ -28,12 +28,12 @@ import static com.google.common.truth.Truth.assertWithMessage;
	import android.provider.DeviceConfig;		import android.provider.DeviceConfig;

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

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

	import com.google.common.truth.Expect;		import com.google.common.truth.Expect;

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

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

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

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

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

+76 −103

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

	import androidx.annotation.Nullable;

	import com.android.internal.annotations.GuardedBy;		import com.android.internal.annotations.GuardedBy;

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

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

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

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

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

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

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

	public void waitForGlobalEnabledState(boolean enabledState) throws Exception {		public void waitForGlobalEnabledState(boolean enabledState) throws Exception {
	Log.i(TAG, "waitForGlobalEnabledState: enabledState=" + enabledState);		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		// Update the callable to only close the properties updated condition when all the
	// desired properties have been updated. The DeviceConfig callbacks may happen multiple		// desired properties have been updated. The DeviceConfig callbacks may happen multiple
	// times so testing the resulting updates is required.		// times so testing the resulting updates is required.
	mShouldClosePropertiesUpdatedCallable = () -> {		waitForPropertiesCondition(() -> {
	//noinspection deprecation		//noinspection deprecation
	return isEnabled() == enabledState;		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();		mDeviceConfigPropertiesUpdated.block();
	}		}
			Log.i(TAG, "waitForPropertiesCondition: returning");
			}
	}		}

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

+0 −6

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

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

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

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

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

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

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

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