Merge "Always call unbind." into oc-dev
diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java
index 9218c25..d25b3cc 100644
--- a/services/core/java/com/android/server/NetworkScoreService.java
+++ b/services/core/java/com/android/server/NetworkScoreService.java
@@ -84,6 +84,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
+import java.util.function.Function;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;
@@ -106,6 +107,7 @@
private final Object mServiceConnectionLock = new Object();
private final Handler mHandler;
private final DispatchingContentObserver mContentObserver;
+ private final Function<NetworkScorerAppData, ScoringServiceConnection> mServiceConnProducer;
@GuardedBy("mPackageMonitorLock")
private NetworkScorerPackageMonitor mPackageMonitor;
@@ -238,11 +240,13 @@
}
public NetworkScoreService(Context context) {
- this(context, new NetworkScorerAppManager(context), Looper.myLooper());
+ this(context, new NetworkScorerAppManager(context),
+ ScoringServiceConnection::new, Looper.myLooper());
}
@VisibleForTesting
NetworkScoreService(Context context, NetworkScorerAppManager networkScoreAppManager,
+ Function<NetworkScorerAppData, ScoringServiceConnection> serviceConnProducer,
Looper looper) {
mContext = context;
mNetworkScorerAppManager = networkScoreAppManager;
@@ -257,6 +261,7 @@
mRecommendationRequestTimeoutMs = TimedRemoteCaller.DEFAULT_CALL_TIMEOUT_MILLIS;
mHandler = new ServiceHandler(looper);
mContentObserver = new DispatchingContentObserver(context, mHandler);
+ mServiceConnProducer = serviceConnProducer;
}
/** Called when the system is ready to run third-party code but before it actually does so. */
@@ -356,17 +361,17 @@
synchronized (mServiceConnectionLock) {
// If we're connected to a different component then drop it.
if (mServiceConnection != null
- && !mServiceConnection.mAppData.equals(appData)) {
+ && !mServiceConnection.getAppData().equals(appData)) {
unbindFromScoringServiceIfNeeded();
}
// If we're not connected at all then create a new connection.
if (mServiceConnection == null) {
- mServiceConnection = new ScoringServiceConnection(appData);
+ mServiceConnection = mServiceConnProducer.apply(appData);
}
// Make sure the connection is connected (idempotent)
- mServiceConnection.connect(mContext);
+ mServiceConnection.bind(mContext);
}
} else { // otherwise make sure it isn't bound.
unbindFromScoringServiceIfNeeded();
@@ -377,9 +382,9 @@
if (DBG) Log.d(TAG, "unbindFromScoringServiceIfNeeded");
synchronized (mServiceConnectionLock) {
if (mServiceConnection != null) {
- mServiceConnection.disconnect(mContext);
+ mServiceConnection.unbind(mContext);
if (DBG) Log.d(TAG, "Disconnected from: "
- + mServiceConnection.mAppData.getRecommendationServiceComponent());
+ + mServiceConnection.getAppData().getRecommendationServiceComponent());
}
mServiceConnection = null;
}
@@ -687,7 +692,7 @@
public boolean isCallerActiveScorer(int callingUid) {
synchronized (mServiceConnectionLock) {
return mServiceConnection != null
- && mServiceConnection.mAppData.packageUid == callingUid;
+ && mServiceConnection.getAppData().packageUid == callingUid;
}
}
@@ -720,7 +725,7 @@
if (isCallerSystemProcess(getCallingUid()) || callerCanRequestScores()) {
synchronized (mServiceConnectionLock) {
if (mServiceConnection != null) {
- return mServiceConnection.mAppData;
+ return mServiceConnection.getAppData();
}
}
} else {
@@ -1020,7 +1025,9 @@
mReqRecommendationCallerRef.set(new RequestRecommendationCaller(timeoutMs));
}
- private static class ScoringServiceConnection implements ServiceConnection {
+ // The class and methods need to be public for Mockito to work.
+ @VisibleForTesting
+ public static class ScoringServiceConnection implements ServiceConnection {
private final NetworkScorerAppData mAppData;
private volatile boolean mBound = false;
private volatile boolean mConnected = false;
@@ -1030,7 +1037,8 @@
mAppData = appData;
}
- void connect(Context context) {
+ @VisibleForTesting
+ public void bind(Context context) {
if (!mBound) {
Intent service = new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS);
service.setComponent(mAppData.getRecommendationServiceComponent());
@@ -1039,13 +1047,15 @@
UserHandle.SYSTEM);
if (!mBound) {
Log.w(TAG, "Bind call failed for " + service);
+ context.unbindService(this);
} else {
if (DBG) Log.d(TAG, "ScoringServiceConnection bound.");
}
}
}
- void disconnect(Context context) {
+ @VisibleForTesting
+ public void unbind(Context context) {
try {
if (mBound) {
mBound = false;
@@ -1056,17 +1066,30 @@
Log.e(TAG, "Unbind failed.", e);
}
+ mConnected = false;
mRecommendationProvider = null;
}
- INetworkRecommendationProvider getRecommendationProvider() {
+ @VisibleForTesting
+ public NetworkScorerAppData getAppData() {
+ return mAppData;
+ }
+
+ @VisibleForTesting
+ public INetworkRecommendationProvider getRecommendationProvider() {
return mRecommendationProvider;
}
- String getPackageName() {
+ @VisibleForTesting
+ public String getPackageName() {
return mAppData.getRecommendationServiceComponent().getPackageName();
}
+ @VisibleForTesting
+ public boolean isAlive() {
+ return mBound && mConnected;
+ }
+
@Override
public void onServiceConnected(ComponentName name, IBinder service) {
if (DBG) Log.d(TAG, "ScoringServiceConnection: " + name.flattenToString());
diff --git a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java
index 353199a..985e5ea 100644
--- a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java
@@ -23,7 +23,6 @@
import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNotNull;
-import static junit.framework.Assert.assertNull;
import static junit.framework.Assert.assertSame;
import static junit.framework.Assert.assertTrue;
import static junit.framework.Assert.fail;
@@ -37,7 +36,7 @@
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -49,7 +48,6 @@
import android.content.ContentResolver;
import android.content.Context;
import android.content.Intent;
-import android.content.ServiceConnection;
import android.content.pm.PackageManager;
import android.content.res.Resources;
import android.net.INetworkRecommendationProvider;
@@ -94,8 +92,6 @@
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
import java.io.FileDescriptor;
import java.io.PrintWriter;
@@ -138,11 +134,13 @@
@Mock private Context mContext;
@Mock private Resources mResources;
@Mock private INetworkScoreCache.Stub mNetworkScoreCache, mNetworkScoreCache2;
- @Mock private IBinder mIBinder, mIBinder2;
+ @Mock private IBinder mScoreCacheIBinder, mScoreCacheIBinder2;
+ @Mock private IBinder mServiceConnectionIBinder;
@Mock private INetworkRecommendationProvider mRecommendationProvider;
@Mock private UnaryOperator<List<ScoredNetwork>> mCurrentNetworkFilter;
@Mock private UnaryOperator<List<ScoredNetwork>> mScanResultsFilter;
@Mock private WifiInfo mWifiInfo;
+ @Mock private NetworkScoreService.ScoringServiceConnection mServiceConnection;
@Captor private ArgumentCaptor<List<ScoredNetwork>> mScoredNetworkCaptor;
private ContentResolver mContentResolver;
@@ -160,17 +158,22 @@
@Before
public void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
- when(mNetworkScoreCache.asBinder()).thenReturn(mIBinder);
- when(mNetworkScoreCache2.asBinder()).thenReturn(mIBinder2);
+ when(mNetworkScoreCache.asBinder()).thenReturn(mScoreCacheIBinder);
+ when(mNetworkScoreCache2.asBinder()).thenReturn(mScoreCacheIBinder2);
+ when(mServiceConnectionIBinder.queryLocalInterface(anyString()))
+ .thenReturn(mRecommendationProvider);
mContentResolver = InstrumentationRegistry.getContext().getContentResolver();
when(mContext.getContentResolver()).thenReturn(mContentResolver);
when(mContext.getResources()).thenReturn(mResources);
when(mWifiInfo.getSSID()).thenReturn(SCORED_NETWORK.networkKey.wifiKey.ssid);
when(mWifiInfo.getBSSID()).thenReturn(SCORED_NETWORK.networkKey.wifiKey.bssid);
+ when(mServiceConnection.getAppData()).thenReturn(NEW_SCORER);
+ when(mServiceConnection.getRecommendationProvider()).thenReturn(mRecommendationProvider);
+ when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(NEW_SCORER);
mHandlerThread = new HandlerThread("NetworkScoreServiceTest");
mHandlerThread.start();
mNetworkScoreService = new NetworkScoreService(mContext, mNetworkScorerAppManager,
- mHandlerThread.getLooper());
+ networkScorerAppData -> mServiceConnection, mHandlerThread.getLooper());
WifiConfiguration configuration = new WifiConfiguration();
configuration.SSID = "NetworkScoreServiceTest_SSID";
configuration.BSSID = "NetworkScoreServiceTest_BSSID";
@@ -209,12 +212,10 @@
mNetworkScoreService.onUserUnlocked(0);
- verify(mContext).bindServiceAsUser(MockUtils.checkIntent(
- new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS)
- .setComponent(RECOMMENDATION_SERVICE_COMP)),
- any(ServiceConnection.class),
- eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE),
- eq(UserHandle.SYSTEM));
+ verify(mNetworkScorerAppManager).updateState();
+ verify(mNetworkScorerAppManager).migrateNetworkScorerAppSettingIfNeeded();
+ verify(mServiceConnection).bind(mContext);
+
}
@Test
@@ -238,16 +239,16 @@
@Test
public void testRequestScores_providerThrowsRemoteException() throws Exception {
- injectProvider();
doThrow(new RemoteException()).when(mRecommendationProvider)
.requestScores(any(NetworkKey[].class));
+ mNetworkScoreService.onUserUnlocked(0);
assertFalse(mNetworkScoreService.requestScores(new NetworkKey[0]));
}
@Test
public void testRequestScores_providerAvailable() throws Exception {
- injectProvider();
+ mNetworkScoreService.onUserUnlocked(0);
final NetworkKey[] networks = new NetworkKey[0];
assertTrue(mNetworkScoreService.requestScores(networks));
@@ -291,11 +292,11 @@
@Test
public void testRequestRecommendation_providerThrowsRemoteException() throws Exception {
- injectProvider();
when(mContext.getMainLooper()).thenReturn(Looper.getMainLooper());
doThrow(new RemoteException()).when(mRecommendationProvider)
.requestRecommendation(eq(mRecommendationRequest), isA(IRemoteCallback.class),
anyInt());
+ mNetworkScoreService.onUserUnlocked(0);
final RecommendationResult result =
mNetworkScoreService.requestRecommendation(mRecommendationRequest);
@@ -306,7 +307,6 @@
@Test
public void testRequestRecommendation_resultReturned() throws Exception {
- injectProvider();
when(mContext.getMainLooper()).thenReturn(Looper.getMainLooper());
final WifiConfiguration wifiConfiguration = new WifiConfiguration();
wifiConfiguration.SSID = "testRequestRecommendation_resultReturned_SSID";
@@ -322,6 +322,7 @@
}).when(mRecommendationProvider)
.requestRecommendation(eq(mRecommendationRequest), isA(IRemoteCallback.class),
anyInt());
+ mNetworkScoreService.onUserUnlocked(0);
final RecommendationResult result =
mNetworkScoreService.requestRecommendation(mRecommendationRequest);
@@ -357,7 +358,7 @@
@Test
public void testRequestRecommendationAsync_requestTimesOut() throws Exception {
- injectProvider();
+ mNetworkScoreService.onUserUnlocked(0);
Settings.Global.putLong(mContentResolver,
Settings.Global.NETWORK_RECOMMENDATION_REQUEST_TIMEOUT_MS, 1L);
mNetworkScoreService.refreshRecommendationRequestTimeoutMs();
@@ -378,7 +379,7 @@
@Test
public void testRequestRecommendationAsync_requestSucceeds() throws Exception {
- injectProvider();
+ mNetworkScoreService.onUserUnlocked(0);
final Bundle bundle = new Bundle();
doAnswer(invocation -> {
invocation.<IRemoteCallback>getArgument(1).sendResult(bundle);
@@ -397,7 +398,7 @@
@Test
public void testRequestRecommendationAsync_requestThrowsRemoteException() throws Exception {
- injectProvider();
+ mNetworkScoreService.onUserUnlocked(0);
doThrow(new RemoteException()).when(mRecommendationProvider)
.requestRecommendation(eq(mRecommendationRequest), isA(IRemoteCallback.class),
anyInt());
@@ -657,9 +658,13 @@
@Test
public void testIsCallerActiveScorer_noBoundService() throws Exception {
+ // No active scorer.
+ when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(null);
mNetworkScoreService.onUserUnlocked(0);
- assertFalse(mNetworkScoreService.isCallerActiveScorer(Binder.getCallingUid()));
+ mNetworkScoreService.isCallerActiveScorer(Binder.getCallingUid());
+
+ verify(mServiceConnection, never()).getAppData();
}
@Test
@@ -678,9 +683,13 @@
@Test
public void testGetActiveScorerPackage_notActive() throws Exception {
+ // No active scorer.
+ when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(null);
mNetworkScoreService.onUserUnlocked(0);
- assertNull(mNetworkScoreService.getActiveScorerPackage());
+ mNetworkScoreService.getActiveScorerPackage();
+
+ verify(mServiceConnection, never()).getPackageName();
}
@Test
@@ -688,10 +697,9 @@
when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(NEW_SCORER);
mNetworkScoreService.onUserUnlocked(0);
- assertEquals(NEW_SCORER.getRecommendationServicePackageName(),
- mNetworkScoreService.getActiveScorerPackage());
- assertEquals(NEW_SCORER.getEnableUseOpenWifiActivity(),
- mNetworkScoreService.getActiveScorer().getEnableUseOpenWifiActivity());
+ mNetworkScoreService.getActiveScorerPackage();
+
+ verify(mServiceConnection).getPackageName();
}
@Test
@@ -931,7 +939,12 @@
public void testGetActiveScorer_notConnected_canRequestScores() throws Exception {
when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES))
.thenReturn(PackageManager.PERMISSION_GRANTED);
- assertNull(mNetworkScoreService.getActiveScorer());
+ when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(null);
+ mNetworkScoreService.onUserUnlocked(0);
+
+ mNetworkScoreService.getActiveScorer();
+
+ verify(mServiceConnection, never()).getAppData();
}
@Test
@@ -951,11 +964,11 @@
throws Exception {
when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES))
.thenReturn(PackageManager.PERMISSION_GRANTED);
- NetworkScorerAppData expectedAppData = new NetworkScorerAppData(Binder.getCallingUid(),
- RECOMMENDATION_SERVICE_COMP, RECOMMENDATION_SERVICE_LABEL,
- USE_WIFI_ENABLE_ACTIVITY_COMP, NETWORK_AVAILABLE_NOTIFICATION_CHANNEL_ID);
- bindToScorer(expectedAppData);
- assertEquals(expectedAppData, mNetworkScoreService.getActiveScorer());
+ mNetworkScoreService.onUserUnlocked(0);
+
+ mNetworkScoreService.getActiveScorer();
+
+ verify(mServiceConnection).getAppData();
}
@Test
@@ -972,23 +985,98 @@
}
}
- // "injects" the mock INetworkRecommendationProvider into the NetworkScoreService.
- private void injectProvider() {
- when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(NEW_SCORER);
- when(mContext.bindServiceAsUser(isA(Intent.class), isA(ServiceConnection.class), anyInt(),
- isA(UserHandle.class))).thenAnswer(new Answer<Boolean>() {
- @Override
- public Boolean answer(InvocationOnMock invocation) throws Throwable {
- IBinder mockBinder = mock(IBinder.class);
- when(mockBinder.queryLocalInterface(anyString()))
- .thenReturn(mRecommendationProvider);
- invocation.<ServiceConnection>getArgument(1)
- .onServiceConnected(NEW_SCORER.getRecommendationServiceComponent(),
- mockBinder);
- return true;
- }
- });
- mNetworkScoreService.onUserUnlocked(0);
+ @Test
+ public void testServiceConnection_bind_notBound() throws Exception {
+ NetworkScoreService.ScoringServiceConnection connection = new NetworkScoreService
+ .ScoringServiceConnection(NEW_SCORER);
+ connection.bind(mContext);
+
+ verify(mContext).bindServiceAsUser(MockUtils.checkIntent(
+ new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS)
+ .setComponent(RECOMMENDATION_SERVICE_COMP)),
+ eq(connection),
+ eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE),
+ eq(UserHandle.SYSTEM));
+ }
+
+ @Test
+ public void testServiceConnection_bind_alreadyBound() throws Exception {
+ NetworkScoreService.ScoringServiceConnection connection = new NetworkScoreService
+ .ScoringServiceConnection(NEW_SCORER);
+
+ when(mContext.bindServiceAsUser(MockUtils.checkIntent(
+ new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS)
+ .setComponent(RECOMMENDATION_SERVICE_COMP)),
+ eq(connection),
+ eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE),
+ eq(UserHandle.SYSTEM))).thenReturn(true /*bound*/);
+
+ // Calling bind more than once should only result in 1 bindService call.
+ connection.bind(mContext);
+ connection.bind(mContext);
+
+ verify(mContext, times(1)).bindServiceAsUser(MockUtils.checkIntent(
+ new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS)
+ .setComponent(RECOMMENDATION_SERVICE_COMP)),
+ eq(connection),
+ eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE),
+ eq(UserHandle.SYSTEM));
+ }
+
+ @Test
+ public void testServiceConnection_bindFails() throws Exception {
+ NetworkScoreService.ScoringServiceConnection connection = new NetworkScoreService
+ .ScoringServiceConnection(NEW_SCORER);
+
+ when(mContext.bindServiceAsUser(MockUtils.checkIntent(
+ new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS)
+ .setComponent(RECOMMENDATION_SERVICE_COMP)),
+ eq(connection),
+ eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE),
+ eq(UserHandle.SYSTEM))).thenReturn(false /*bound*/);
+
+ connection.bind(mContext);
+
+ verify(mContext).unbindService(connection);
+ }
+
+ @Test
+ public void testServiceConnection_unbind_notBound() throws Exception {
+ NetworkScoreService.ScoringServiceConnection connection = new NetworkScoreService
+ .ScoringServiceConnection(NEW_SCORER);
+
+ connection.unbind(mContext);
+
+ verify(mContext, never()).unbindService(connection);
+ }
+
+ @Test
+ public void testServiceConnection_unbind_alreadyBound() throws Exception {
+ NetworkScoreService.ScoringServiceConnection connection = new NetworkScoreService
+ .ScoringServiceConnection(NEW_SCORER);
+
+ when(mContext.bindServiceAsUser(MockUtils.checkIntent(
+ new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS)
+ .setComponent(RECOMMENDATION_SERVICE_COMP)),
+ eq(connection),
+ eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE),
+ eq(UserHandle.SYSTEM))).thenReturn(true /*bound*/);
+
+ connection.bind(mContext);
+ connection.unbind(mContext);
+
+ verify(mContext).unbindService(connection);
+ }
+
+ @Test
+ public void testServiceConnection_dump_doesNotCrash() throws Exception {
+ NetworkScoreService.ScoringServiceConnection connection = new NetworkScoreService
+ .ScoringServiceConnection(NEW_SCORER);
+ final StringWriter stringWriter = new StringWriter();
+
+ connection.dump(new FileDescriptor(), new PrintWriter(stringWriter), new String[0]);
+
+ assertFalse(stringWriter.toString().isEmpty());
}
private void bindToScorer(boolean callerIsScorer) {
@@ -996,13 +1084,7 @@
NetworkScorerAppData appData = new NetworkScorerAppData(callingUid,
RECOMMENDATION_SERVICE_COMP, RECOMMENDATION_SERVICE_LABEL,
USE_WIFI_ENABLE_ACTIVITY_COMP, NETWORK_AVAILABLE_NOTIFICATION_CHANNEL_ID);
- bindToScorer(appData);
- }
-
- private void bindToScorer(NetworkScorerAppData appData) {
- when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(appData);
- when(mContext.bindServiceAsUser(isA(Intent.class), isA(ServiceConnection.class), anyInt(),
- isA(UserHandle.class))).thenReturn(true);
+ when(mServiceConnection.getAppData()).thenReturn(appData);
mNetworkScoreService.onUserUnlocked(0);
}