Add removeListener, update tests, hide binder

Add a removeListener method to allow deregistration of interest in
config changes, update tests. Also, hide binder interfaces on the
android.app methods.

Test: Treehugger / atest of touched test files
Bug: 149014708
Bug: 159888952
Change-Id: Ic1a14c34334eb276724d97c4e763360d51cb035b
This commit is contained in:
Neil Fuller
2020-07-01 15:05:34 +01:00
parent fc0f88505c
commit d760ca71c9
5 changed files with 228 additions and 33 deletions

View File

@@ -41,6 +41,7 @@ interface ITimeZoneDetectorService {
TimeZoneConfiguration getConfiguration();
boolean updateConfiguration(in TimeZoneConfiguration configuration);
void addConfigurationListener(ITimeZoneConfigurationListener listener);
void removeConfigurationListener(ITimeZoneConfigurationListener listener);
boolean suggestManualTimeZone(in ManualTimeZoneSuggestion timeZoneSuggestion);
void suggestTelephonyTimeZone(in TelephonyTimeZoneSuggestion timeZoneSuggestion);

View File

@@ -73,12 +73,27 @@ public interface TimeZoneDetector {
@RequiresPermission(android.Manifest.permission.WRITE_SECURE_SETTINGS)
boolean updateConfiguration(@NonNull TimeZoneConfiguration configuration);
/**
* An interface that can be used to listen for changes to the time zone detector configuration.
*/
interface TimeZoneConfigurationListener {
/** Called when the configuration changes. There are no guarantees about the thread used. */
void onChange(@NonNull TimeZoneConfiguration configuration);
}
/**
* Registers a listener that will be informed when the configuration changes. The complete
* configuration is passed to the listener, not just the properties that have changed.
*/
@RequiresPermission(android.Manifest.permission.WRITE_SECURE_SETTINGS)
void addConfigurationListener(@NonNull ITimeZoneConfigurationListener listener);
void addConfigurationListener(@NonNull TimeZoneConfigurationListener listener);
/**
* Removes a listener previously passed to
* {@link #addConfigurationListener(ITimeZoneConfigurationListener)}
*/
@RequiresPermission(android.Manifest.permission.WRITE_SECURE_SETTINGS)
void removeConfigurationListener(@NonNull TimeZoneConfigurationListener listener);
/**
* A shared utility method to create a {@link ManualTimeZoneSuggestion}.

View File

@@ -21,6 +21,7 @@ import android.content.Context;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.os.ServiceManager.ServiceNotFoundException;
import android.util.ArraySet;
import android.util.Log;
/**
@@ -34,13 +35,16 @@ public final class TimeZoneDetectorImpl implements TimeZoneDetector {
private final ITimeZoneDetectorService mITimeZoneDetectorService;
private ITimeZoneConfigurationListener mConfigurationReceiver;
private ArraySet<TimeZoneConfigurationListener> mConfigurationListeners;
public TimeZoneDetectorImpl() throws ServiceNotFoundException {
mITimeZoneDetectorService = ITimeZoneDetectorService.Stub.asInterface(
ServiceManager.getServiceOrThrow(Context.TIME_ZONE_DETECTOR_SERVICE));
}
@Override
@NonNull
@NonNull
public TimeZoneCapabilities getCapabilities() {
if (DEBUG) {
Log.d(TAG, "getCapabilities called");
@@ -78,14 +82,70 @@ public final class TimeZoneDetectorImpl implements TimeZoneDetector {
}
@Override
public void addConfigurationListener(@NonNull ITimeZoneConfigurationListener listener) {
public void addConfigurationListener(@NonNull TimeZoneConfigurationListener listener) {
if (DEBUG) {
Log.d(TAG, "addConfigurationListener called: " + listener);
}
try {
mITimeZoneDetectorService.addConfigurationListener(listener);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
synchronized (this) {
if (mConfigurationListeners.contains(listener)) {
return;
}
if (mConfigurationReceiver == null) {
ITimeZoneConfigurationListener iListener =
new ITimeZoneConfigurationListener.Stub() {
@Override
public void onChange(@NonNull TimeZoneConfiguration configuration) {
notifyConfigurationListeners(configuration);
}
};
mConfigurationReceiver = iListener;
}
if (mConfigurationListeners == null) {
mConfigurationListeners = new ArraySet<>();
}
boolean wasEmpty = mConfigurationListeners.isEmpty();
mConfigurationListeners.add(listener);
if (wasEmpty) {
try {
mITimeZoneDetectorService.addConfigurationListener(mConfigurationReceiver);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
}
}
}
private void notifyConfigurationListeners(@NonNull TimeZoneConfiguration configuration) {
ArraySet<TimeZoneConfigurationListener> configurationListeners;
synchronized (this) {
configurationListeners = new ArraySet<>(mConfigurationListeners);
}
int size = configurationListeners.size();
for (int i = 0; i < size; i++) {
configurationListeners.valueAt(i).onChange(configuration);
}
}
@Override
public void removeConfigurationListener(@NonNull TimeZoneConfigurationListener listener) {
if (DEBUG) {
Log.d(TAG, "removeConfigurationListener called: " + listener);
}
synchronized (this) {
if (mConfigurationListeners == null) {
return;
}
boolean wasEmpty = mConfigurationListeners.isEmpty();
mConfigurationListeners.remove(listener);
if (mConfigurationListeners.isEmpty() && !wasEmpty) {
try {
mITimeZoneDetectorService.removeConfigurationListener(mConfigurationReceiver);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
}
}
}

View File

@@ -49,6 +49,7 @@ import com.android.server.timezonedetector.TimeZoneDetectorStrategy.StrategyList
import java.io.FileDescriptor;
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Objects;
/**
@@ -188,20 +189,14 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
int userId = UserHandle.getCallingUserId();
ConfigListenerInfo listenerInfo = new ConfigListenerInfo(userId, listener);
final IBinder.DeathRecipient deathRecipient = new IBinder.DeathRecipient() {
@Override
public void binderDied() {
synchronized (mConfigurationListeners) {
Slog.i(TAG, "Configuration listener died: " + listenerInfo);
mConfigurationListeners.remove(listenerInfo);
}
}
};
synchronized (mConfigurationListeners) {
if (mConfigurationListeners.contains(listenerInfo)) {
return;
}
try {
// Remove the record of the listener if the client process dies.
listener.asBinder().linkToDeath(deathRecipient, 0 /* flags */);
// Ensure the reference to the listener is removed if the client process dies.
listenerInfo.linkToDeath();
// Only add the listener if we can linkToDeath().
mConfigurationListeners.add(listenerInfo);
@@ -211,6 +206,31 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
}
}
@Override
public void removeConfigurationListener(@NonNull ITimeZoneConfigurationListener listener) {
enforceManageTimeZoneDetectorConfigurationPermission();
Objects.requireNonNull(listener);
int userId = UserHandle.getCallingUserId();
synchronized (mConfigurationListeners) {
ConfigListenerInfo toRemove = new ConfigListenerInfo(userId, listener);
Iterator<ConfigListenerInfo> listenerIterator = mConfigurationListeners.iterator();
while (listenerIterator.hasNext()) {
ConfigListenerInfo currentListenerInfo = listenerIterator.next();
if (currentListenerInfo.equals(toRemove)) {
listenerIterator.remove();
// Stop listening for the client process to die.
try {
currentListenerInfo.unlinkToDeath();
} catch (RemoteException e) {
Slog.e(TAG, "Unable to unlinkToDeath() for listener=" + listener, e);
}
}
}
}
}
void handleConfigurationChanged() {
// Note: we could trigger an async time zone detection operation here via a call to
// handleAutoTimeZoneDetectionChanged(), but that is triggered in response to the underlying
@@ -319,7 +339,7 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
this, in, out, err, args, callback, resultReceiver);
}
private static class ConfigListenerInfo {
private class ConfigListenerInfo implements IBinder.DeathRecipient {
private final @UserIdInt int mUserId;
private final ITimeZoneConfigurationListener mListener;
@@ -337,6 +357,40 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub
return mListener;
}
void linkToDeath() throws RemoteException {
mListener.asBinder().linkToDeath(this, 0 /* flags */);
}
void unlinkToDeath() throws RemoteException {
mListener.asBinder().unlinkToDeath(this, 0 /* flags */);
}
@Override
public void binderDied() {
synchronized (mConfigurationListeners) {
Slog.i(TAG, "Configuration listener client died: " + this);
mConfigurationListeners.remove(this);
}
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
ConfigListenerInfo that = (ConfigListenerInfo) o;
return mUserId == that.mUserId
&& mListener.equals(that.mListener);
}
@Override
public int hashCode() {
return Objects.hash(mUserId, mListener);
}
@Override
public String toString() {
return "ConfigListenerInfo{"

View File

@@ -21,12 +21,16 @@ import static android.app.timezonedetector.TimeZoneCapabilities.CAPABILITY_POSSE
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
import android.app.timezonedetector.ITimeZoneConfigurationListener;
@@ -158,10 +162,24 @@ public class TimeZoneDetectorServiceTest {
}
}
@Test(expected = SecurityException.class)
public void testRemoveConfigurationListener_withoutPermission() {
doThrow(new SecurityException("Mock"))
.when(mMockContext).enforceCallingPermission(anyString(), any());
ITimeZoneConfigurationListener mockListener = mock(ITimeZoneConfigurationListener.class);
try {
mTimeZoneDetectorService.removeConfigurationListener(mockListener);
fail();
} finally {
verify(mMockContext).enforceCallingPermission(
eq(android.Manifest.permission.WRITE_SECURE_SETTINGS),
anyString());
}
}
@Test
public void testConfigurationChangeListenerRegistrationAndCallbacks() throws Exception {
doNothing().when(mMockContext).enforceCallingPermission(anyString(), any());
TimeZoneConfiguration autoDetectDisabledConfiguration =
createTimeZoneConfiguration(false /* autoDetectionEnabled */);
@@ -169,22 +187,69 @@ public class TimeZoneDetectorServiceTest {
IBinder mockListenerBinder = mock(IBinder.class);
ITimeZoneConfigurationListener mockListener = mock(ITimeZoneConfigurationListener.class);
when(mockListener.asBinder()).thenReturn(mockListenerBinder);
mTimeZoneDetectorService.addConfigurationListener(mockListener);
{
doNothing().when(mMockContext).enforceCallingPermission(anyString(), any());
when(mockListener.asBinder()).thenReturn(mockListenerBinder);
verify(mMockContext).enforceCallingPermission(
eq(android.Manifest.permission.WRITE_SECURE_SETTINGS),
anyString());
verify(mockListenerBinder).linkToDeath(any(), eq(0));
mTimeZoneDetectorService.addConfigurationListener(mockListener);
// Simulate the configuration being changed and verify the mockListener was notified.
TimeZoneConfiguration autoDetectEnabledConfiguration =
createTimeZoneConfiguration(true /* autoDetectionEnabled */);
mFakeTimeZoneDetectorStrategy.updateConfiguration(
ARBITRARY_USER_ID, autoDetectEnabledConfiguration);
verify(mMockContext).enforceCallingPermission(
eq(android.Manifest.permission.WRITE_SECURE_SETTINGS),
anyString());
verify(mockListener).asBinder();
verify(mockListenerBinder).linkToDeath(any(), anyInt());
verifyNoMoreInteractions(mockListenerBinder, mockListener, mMockContext);
reset(mockListenerBinder, mockListener, mMockContext);
}
verify(mockListener).onChange(autoDetectEnabledConfiguration);
{
doNothing().when(mMockContext).enforceCallingPermission(anyString(), any());
// Simulate the configuration being changed and verify the mockListener was notified.
TimeZoneConfiguration autoDetectEnabledConfiguration =
createTimeZoneConfiguration(true /* autoDetectionEnabled */);
mTimeZoneDetectorService.updateConfiguration(autoDetectEnabledConfiguration);
verify(mMockContext).enforceCallingPermission(
eq(android.Manifest.permission.WRITE_SECURE_SETTINGS),
anyString());
verify(mockListener).onChange(autoDetectEnabledConfiguration);
verifyNoMoreInteractions(mockListenerBinder, mockListener, mMockContext);
reset(mockListenerBinder, mockListener, mMockContext);
}
{
doNothing().when(mMockContext).enforceCallingPermission(anyString(), any());
when(mockListener.asBinder()).thenReturn(mockListenerBinder);
when(mockListenerBinder.unlinkToDeath(any(), anyInt())).thenReturn(true);
// Now remove the listener, change the config again, and verify the listener is not
// called.
mTimeZoneDetectorService.removeConfigurationListener(mockListener);
verify(mMockContext).enforceCallingPermission(
eq(android.Manifest.permission.WRITE_SECURE_SETTINGS),
anyString());
verify(mockListener).asBinder();
verify(mockListenerBinder).unlinkToDeath(any(), eq(0));
verifyNoMoreInteractions(mockListenerBinder, mockListener, mMockContext);
reset(mockListenerBinder, mockListener, mMockContext);
}
{
doNothing().when(mMockContext).enforceCallingPermission(anyString(), any());
mTimeZoneDetectorService.updateConfiguration(autoDetectDisabledConfiguration);
verify(mMockContext).enforceCallingPermission(
eq(android.Manifest.permission.WRITE_SECURE_SETTINGS),
anyString());
verify(mockListener, never()).onChange(any());
verifyNoMoreInteractions(mockListenerBinder, mockListener, mMockContext);
reset(mockListenerBinder, mockListener, mMockContext);
}
}
@Test(expected = SecurityException.class)