From 145c34316649ba0a5cdb8f7954a63419785f379d Mon Sep 17 00:00:00 2001 From: Jeremy Joslin Date: Fri, 9 Dec 2016 13:11:51 -0800 Subject: [PATCH] Implement the request and recommend calls. Implemented requestRecommendation() and requestScores() to call through to the bound network recommendation provider if available. BUG: 33593157 BUG: 33668692 Test: runtest frameworks-services -c com.android.server.NetworkScoreServiceTest Change-Id: I8e2ed73dc6876deb1a8bd47bcaeaca8db68f3a44 --- core/java/android/net/NetworkScoreManager.java | 18 +-- .../com/android/server/NetworkScoreService.java | 160 +++++++++++++++++--- .../android/server/NetworkScoreServiceTest.java | 167 ++++++++++++++++++++- 3 files changed, 306 insertions(+), 39 deletions(-) diff --git a/core/java/android/net/NetworkScoreManager.java b/core/java/android/net/NetworkScoreManager.java index cd5628765098..4e606ef4eede 100644 --- a/core/java/android/net/NetworkScoreManager.java +++ b/core/java/android/net/NetworkScoreManager.java @@ -272,19 +272,11 @@ public class NetworkScoreManager { * @hide */ public boolean requestScores(NetworkKey[] networks) throws SecurityException { - String activeScorer = getActiveScorerPackage(); - if (activeScorer == null) { - return false; + try { + return mService.requestScores(networks); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - Intent intent = new Intent(ACTION_SCORE_NETWORKS); - intent.setPackage(activeScorer); - intent.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); - intent.putExtra(EXTRA_NETWORKS_TO_SCORE, networks); - // A scorer should never become active if its package doesn't hold SCORE_NETWORKS, but - // ensure the package still holds it to be extra safe. - // TODO: http://b/23422763 - mContext.sendBroadcastAsUser(intent, UserHandle.SYSTEM, Manifest.permission.SCORE_NETWORKS); - return true; } /** @@ -344,6 +336,8 @@ public class NetworkScoreManager { /** * Request a recommendation for which network to connect to. * + *

It is not safe to call this method from the main thread. + * * @param request a {@link RecommendationRequest} instance containing additional * request details * @return a {@link RecommendationResult} instance containing the recommended network diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java index 723d5d8369f4..f712f121f1d7 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -16,7 +16,11 @@ package com.android.server; +import static android.net.NetworkRecommendationProvider.EXTRA_RECOMMENDATION_RESULT; +import static android.net.NetworkRecommendationProvider.EXTRA_SEQUENCE; + import android.Manifest.permission; +import android.annotation.Nullable; import android.content.BroadcastReceiver; import android.content.ComponentName; import android.content.ContentResolver; @@ -26,6 +30,7 @@ import android.content.IntentFilter; import android.content.ServiceConnection; import android.content.pm.PackageManager; import android.database.ContentObserver; +import android.net.INetworkRecommendationProvider; import android.net.INetworkScoreCache; import android.net.INetworkScoreService; import android.net.NetworkKey; @@ -36,17 +41,22 @@ import android.net.RecommendationResult; import android.net.ScoredNetwork; import android.net.Uri; import android.net.wifi.WifiConfiguration; +import android.os.Bundle; import android.os.IBinder; +import android.os.IRemoteCallback; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.UserHandle; import android.provider.Settings.Global; import android.util.ArrayMap; import android.util.Log; +import android.util.TimedRemoteCaller; + import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.content.PackageMonitor; import com.android.internal.os.TransferPipe; + import java.io.FileDescriptor; import java.io.IOException; import java.io.PrintWriter; @@ -55,6 +65,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeoutException; import java.util.function.Consumer; /** @@ -63,17 +74,20 @@ import java.util.function.Consumer; */ public class NetworkScoreService extends INetworkScoreService.Stub { private static final String TAG = "NetworkScoreService"; - private static final boolean DBG = false; + private static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG); private final Context mContext; private final NetworkScorerAppManager mNetworkScorerAppManager; + private final RequestRecommendationCaller mRequestRecommendationCaller; @GuardedBy("mScoreCaches") private final Map> mScoreCaches; /** Lock used to update mPackageMonitor when scorer package changes occur. */ private final Object mPackageMonitorLock = new Object[0]; + private final Object mServiceConnectionLock = new Object[0]; @GuardedBy("mPackageMonitorLock") private NetworkScorerPackageMonitor mPackageMonitor; + @GuardedBy("mServiceConnectionLock") private ScoringServiceConnection mServiceConnection; private BroadcastReceiver mUserIntentReceiver = new BroadcastReceiver() { @@ -194,6 +208,9 @@ public class NetworkScoreService extends INetworkScoreService.Stub { mContext.registerReceiverAsUser( mUserIntentReceiver, UserHandle.SYSTEM, filter, null /* broadcastPermission*/, null /* scheduler */); + // TODO(jjoslin): 12/15/16 - Make timeout configurable. + mRequestRecommendationCaller = + new RequestRecommendationCaller(TimedRemoteCaller.DEFAULT_CALL_TIMEOUT_MILLIS); } /** Called when the system is ready to run third-party code but before it actually does so. */ @@ -264,19 +281,21 @@ public class NetworkScoreService extends INetworkScoreService.Stub { if (scorerData != null && scorerData.recommendationServiceClassName != null) { ComponentName componentName = new ComponentName(scorerData.packageName, scorerData.recommendationServiceClassName); - // If we're connected to a different component then drop it. - if (mServiceConnection != null - && !mServiceConnection.mComponentName.equals(componentName)) { - unbindFromScoringServiceIfNeeded(); - } + synchronized (mServiceConnectionLock) { + // If we're connected to a different component then drop it. + if (mServiceConnection != null + && !mServiceConnection.mComponentName.equals(componentName)) { + unbindFromScoringServiceIfNeeded(); + } - // If we're not connected at all then create a new connection. - if (mServiceConnection == null) { - mServiceConnection = new ScoringServiceConnection(componentName); - } + // If we're not connected at all then create a new connection. + if (mServiceConnection == null) { + mServiceConnection = new ScoringServiceConnection(componentName); + } - // Make sure the connection is connected (idempotent) - mServiceConnection.connect(mContext); + // Make sure the connection is connected (idempotent) + mServiceConnection.connect(mContext); + } } else { // otherwise make sure it isn't bound. unbindFromScoringServiceIfNeeded(); } @@ -284,10 +303,12 @@ public class NetworkScoreService extends INetworkScoreService.Stub { private void unbindFromScoringServiceIfNeeded() { if (DBG) Log.d(TAG, "unbindFromScoringServiceIfNeeded"); - if (mServiceConnection != null) { - mServiceConnection.disconnect(mContext); + synchronized (mServiceConnectionLock) { + if (mServiceConnection != null) { + mServiceConnection.disconnect(mContext); + } + mServiceConnection = null; } - mServiceConnection = null; clearInternal(); } @@ -441,7 +462,22 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public RecommendationResult requestRecommendation(RecommendationRequest request) { - // TODO(jjoslin): 11/25/16 - Update with real impl. + mContext.enforceCallingOrSelfPermission(permission.BROADCAST_NETWORK_PRIVILEGED, TAG); + throwIfCalledOnMainThread(); + final INetworkRecommendationProvider provider = getRecommendationProvider(); + if (provider != null) { + try { + return mRequestRecommendationCaller.getRecommendationResult(provider, request); + } catch (RemoteException | TimeoutException e) { + Log.w(TAG, "Failed to request a recommendation.", e); + // TODO(jjoslin): 12/15/16 - Keep track of failures. + } + } + + if (DBG) { + Log.d(TAG, "Returning the default network recommendation."); + } + WifiConfiguration selectedConfig = null; if (request != null) { selectedConfig = request.getCurrentSelectedConfig(); @@ -451,7 +487,19 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public boolean requestScores(NetworkKey[] networks) { - // TODO(jjoslin): 12/13/16 - Implement + mContext.enforceCallingOrSelfPermission(permission.BROADCAST_NETWORK_PRIVILEGED, TAG); + final INetworkRecommendationProvider provider = getRecommendationProvider(); + if (provider != null) { + try { + provider.requestScores(networks); + // TODO(jjoslin): 12/15/16 - Consider pushing null scores into the cache to prevent + // repeated requests for the same scores. + return true; + } catch (RemoteException e) { + Log.w(TAG, "Failed to request scores.", e); + // TODO(jjoslin): 12/15/16 - Keep track of failures. + } + } return false; } @@ -476,10 +524,12 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } }, getScoreCacheLists()); - if (mServiceConnection != null) { - mServiceConnection.dump(fd, writer, args); - } else { - writer.println("ScoringServiceConnection: null"); + synchronized (mServiceConnectionLock) { + if (mServiceConnection != null) { + mServiceConnection.dump(fd, writer, args); + } else { + writer.println("ScoringServiceConnection: null"); + } } writer.flush(); } @@ -512,10 +562,27 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } } + private void throwIfCalledOnMainThread() { + if (Thread.currentThread() == mContext.getMainLooper().getThread()) { + throw new RuntimeException("Cannot invoke on the main thread"); + } + } + + @Nullable + private INetworkRecommendationProvider getRecommendationProvider() { + synchronized (mServiceConnectionLock) { + if (mServiceConnection != null) { + return mServiceConnection.getRecommendationProvider(); + } + } + return null; + } + private static class ScoringServiceConnection implements ServiceConnection { private final ComponentName mComponentName; - private boolean mBound = false; - private boolean mConnected = false; + private volatile boolean mBound = false; + private volatile boolean mConnected = false; + private volatile INetworkRecommendationProvider mRecommendationProvider; ScoringServiceConnection(ComponentName componentName) { mComponentName = componentName; @@ -546,12 +613,19 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } catch (RuntimeException e) { Log.e(TAG, "Unbind failed.", e); } + + mRecommendationProvider = null; + } + + INetworkRecommendationProvider getRecommendationProvider() { + return mRecommendationProvider; } @Override public void onServiceConnected(ComponentName name, IBinder service) { if (DBG) Log.d(TAG, "ScoringServiceConnection: " + name.flattenToString()); mConnected = true; + mRecommendationProvider = INetworkRecommendationProvider.Stub.asInterface(service); } @Override @@ -560,6 +634,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { Log.d(TAG, "ScoringServiceConnection, disconnected: " + name.flattenToString()); } mConnected = false; + mRecommendationProvider = null; } public void dump(FileDescriptor fd, PrintWriter writer, String[] args) { @@ -567,4 +642,43 @@ public class NetworkScoreService extends INetworkScoreService.Stub { + ", connected: " + mConnected); } } + + /** + * Executes the async requestRecommendation() call with a timeout. + */ + private static final class RequestRecommendationCaller + extends TimedRemoteCaller { + private final IRemoteCallback mCallback; + + RequestRecommendationCaller(long callTimeoutMillis) { + super(callTimeoutMillis); + mCallback = new IRemoteCallback.Stub() { + @Override + public void sendResult(Bundle data) throws RemoteException { + final RecommendationResult result = + data.getParcelable(EXTRA_RECOMMENDATION_RESULT); + final int sequence = data.getInt(EXTRA_SEQUENCE, -1); + onRemoteMethodResult(result, sequence); + } + }; + } + + /** + * Runs the requestRecommendation() call on the given {@link INetworkRecommendationProvider} + * instance. + * + * @param target the {@link INetworkRecommendationProvider} to request a recommendation + * from + * @param request the {@link RecommendationRequest} from the calling client + * @return a {@link RecommendationResult} from the provider + * @throws RemoteException if the call failed + * @throws TimeoutException if the call took longer than the set timeout + */ + RecommendationResult getRecommendationResult(INetworkRecommendationProvider target, + RecommendationRequest request) throws RemoteException, TimeoutException { + final int sequence = onBeforeRemoteCall(); + target.requestRecommendation(request, mCallback, sequence); + return getResultTimed(sequence); + } + } } diff --git a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java index e59addb7b9c3..c653b8eecffa 100644 --- a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java @@ -16,20 +16,30 @@ package com.android.server; +import static android.net.NetworkRecommendationProvider.EXTRA_RECOMMENDATION_RESULT; +import static android.net.NetworkRecommendationProvider.EXTRA_SEQUENCE; import static android.net.NetworkScoreManager.CACHE_FILTER_NONE; + import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertTrue; import static junit.framework.Assert.fail; + import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyListOf; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isA; 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.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import android.Manifest.permission; @@ -40,23 +50,28 @@ import android.content.Intent; import android.content.ServiceConnection; import android.content.pm.PackageManager; import android.content.res.Resources; +import android.net.INetworkRecommendationProvider; import android.net.INetworkScoreCache; import android.net.NetworkKey; import android.net.NetworkScorerAppManager; import android.net.NetworkScorerAppManager.NetworkScorerAppData; +import android.net.RecommendationRequest; +import android.net.RecommendationResult; import android.net.ScoredNetwork; import android.net.WifiKey; +import android.net.wifi.WifiConfiguration; +import android.os.Bundle; import android.os.IBinder; +import android.os.IRemoteCallback; +import android.os.Looper; import android.os.RemoteException; import android.os.UserHandle; import android.support.test.InstrumentationRegistry; import android.support.test.filters.MediumTest; import android.support.test.runner.AndroidJUnit4; + import com.android.server.devicepolicy.MockUtils; -import java.io.FileDescriptor; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.util.List; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -64,6 +79,13 @@ import org.mockito.ArgumentCaptor; 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; +import java.io.StringWriter; +import java.util.List; /** * Tests for {@link NetworkScoreService}. @@ -83,10 +105,12 @@ public class NetworkScoreServiceTest { @Mock private Resources mResources; @Mock private INetworkScoreCache.Stub mNetworkScoreCache, mNetworkScoreCache2; @Mock private IBinder mIBinder, mIBinder2; + @Mock private INetworkRecommendationProvider mRecommendationProvider; @Captor private ArgumentCaptor> mScoredNetworkCaptor; private ContentResolver mContentResolver; private NetworkScoreService mNetworkScoreService; + private RecommendationRequest mRecommendationRequest; @Before public void setUp() throws Exception { @@ -97,6 +121,9 @@ public class NetworkScoreServiceTest { when(mContext.getContentResolver()).thenReturn(mContentResolver); when(mContext.getResources()).thenReturn(mResources); mNetworkScoreService = new NetworkScoreService(mContext, mNetworkScorerAppManager); + WifiConfiguration configuration = new WifiConfiguration(); + mRecommendationRequest = new RecommendationRequest.Builder() + .setCurrentRecommendedWifiConfig(configuration).build(); } @Test @@ -114,6 +141,118 @@ public class NetworkScoreServiceTest { } @Test + public void testRequestScores_noPermission() throws Exception { + doThrow(new SecurityException()).when(mContext) + .enforceCallingOrSelfPermission(eq(permission.BROADCAST_NETWORK_PRIVILEGED), + anyString()); + try { + mNetworkScoreService.requestScores(null); + fail("BROADCAST_NETWORK_PRIVILEGED not enforced."); + } catch (SecurityException e) { + // expected + } + } + + @Test + public void testRequestScores_providerNotConnected() throws Exception { + assertFalse(mNetworkScoreService.requestScores(new NetworkKey[0])); + verifyZeroInteractions(mRecommendationProvider); + } + + @Test + public void testRequestScores_providerThrowsRemoteException() throws Exception { + injectProvider(); + doThrow(new RemoteException()).when(mRecommendationProvider) + .requestScores(any(NetworkKey[].class)); + + assertFalse(mNetworkScoreService.requestScores(new NetworkKey[0])); + } + + @Test + public void testRequestScores_providerAvailable() throws Exception { + injectProvider(); + + final NetworkKey[] networks = new NetworkKey[0]; + assertTrue(mNetworkScoreService.requestScores(networks)); + verify(mRecommendationProvider).requestScores(networks); + } + + @Test + public void testRequestRecommendation_noPermission() throws Exception { + doThrow(new SecurityException()).when(mContext) + .enforceCallingOrSelfPermission(eq(permission.BROADCAST_NETWORK_PRIVILEGED), + anyString()); + try { + mNetworkScoreService.requestRecommendation(mRecommendationRequest); + fail("BROADCAST_NETWORK_PRIVILEGED not enforced."); + } catch (SecurityException e) { + // expected + } + } + + @Test + public void testRequestRecommendation_mainThread() throws Exception { + when(mContext.getMainLooper()).thenReturn(Looper.myLooper()); + try { + mNetworkScoreService.requestRecommendation(mRecommendationRequest); + fail("requestRecommendation run on main thread."); + } catch (RuntimeException e) { + // expected + } + } + + @Test + public void testRequestRecommendation_providerNotConnected() throws Exception { + when(mContext.getMainLooper()).thenReturn(Looper.getMainLooper()); + + final RecommendationResult result = + mNetworkScoreService.requestRecommendation(mRecommendationRequest); + assertNotNull(result); + assertEquals(mRecommendationRequest.getCurrentSelectedConfig(), + result.getWifiConfiguration()); + } + + @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()); + + final RecommendationResult result = + mNetworkScoreService.requestRecommendation(mRecommendationRequest); + assertNotNull(result); + assertEquals(mRecommendationRequest.getCurrentSelectedConfig(), + result.getWifiConfiguration()); + } + + @Test + public void testRequestRecommendation_resultReturned() throws Exception { + injectProvider(); + when(mContext.getMainLooper()).thenReturn(Looper.getMainLooper()); + final WifiConfiguration wifiConfiguration = new WifiConfiguration(); + wifiConfiguration.SSID = "testRequestRecommendation_resultReturned"; + final RecommendationResult providerResult = + new RecommendationResult(wifiConfiguration); + final Bundle bundle = new Bundle(); + bundle.putParcelable(EXTRA_RECOMMENDATION_RESULT, providerResult); + doAnswer(invocation -> { + bundle.putInt(EXTRA_SEQUENCE, invocation.getArgumentAt(2, int.class)); + invocation.getArgumentAt(1, IRemoteCallback.class).sendResult(bundle); + return null; + }).when(mRecommendationProvider) + .requestRecommendation(eq(mRecommendationRequest), isA(IRemoteCallback.class), + anyInt()); + + final RecommendationResult result = + mNetworkScoreService.requestRecommendation(mRecommendationRequest); + assertNotNull(result); + assertEquals(providerResult.getWifiConfiguration().SSID, + result.getWifiConfiguration().SSID); + } + + @Test public void testUpdateScores_notActiveScorer() { when(mNetworkScorerAppManager.isCallerActiveScorer(anyInt())).thenReturn(false); @@ -301,4 +440,24 @@ public class NetworkScoreServiceTest { assertFalse(stringWriter.toString().isEmpty()); } + + // "injects" the mock INetworkRecommendationProvider into the NetworkScoreService. + private void injectProvider() { + final ComponentName componentName = new ComponentName(NEW_SCORER.packageName, + NEW_SCORER.recommendationServiceClassName); + when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(NEW_SCORER); + when(mContext.bindServiceAsUser(isA(Intent.class), isA(ServiceConnection.class), anyInt(), + isA(UserHandle.class))).thenAnswer(new Answer() { + @Override + public Boolean answer(InvocationOnMock invocation) throws Throwable { + IBinder mockBinder = mock(IBinder.class); + when(mockBinder.queryLocalInterface(anyString())) + .thenReturn(mRecommendationProvider); + invocation.getArgumentAt(1, ServiceConnection.class) + .onServiceConnected(componentName, mockBinder); + return true; + } + }); + mNetworkScoreService.systemRunning(); + } } -- 2.11.0