From 998d7ca56a1268a1249e2678e529caf4e8fd95ca Mon Sep 17 00:00:00 2001 From: Jeremy Joslin Date: Wed, 28 Dec 2016 15:56:46 -0800 Subject: [PATCH 1/2] Async network recommendation requests. Exposing a new hidden API method that allows network recommendations to be requested asynchronously. Test: Built & run. BUG:33784158 Change-Id: I8f84b09f43a6c5fae5d8f03ec01e75c25b4b62d6 --- .../android/net/INetworkScoreService.aidl | 13 +++++ .../java/android/net/NetworkScoreManager.java | 57 +++++++++++++++++++ .../android/server/NetworkScoreService.java | 28 +++++++++ 3 files changed, 98 insertions(+) diff --git a/core/java/android/net/INetworkScoreService.aidl b/core/java/android/net/INetworkScoreService.aidl index 9573953e6e135..82432c758fa62 100644 --- a/core/java/android/net/INetworkScoreService.aidl +++ b/core/java/android/net/INetworkScoreService.aidl @@ -21,6 +21,7 @@ import android.net.NetworkKey; import android.net.RecommendationRequest; import android.net.RecommendationResult; import android.net.ScoredNetwork; +import android.os.RemoteCallback; /** * A service for updating network scores from a network scorer application. @@ -117,4 +118,16 @@ interface INetworkScoreService * scorer. */ String getActiveScorerPackage(); + + /** + * Request a recommendation for the best network to connect to + * taking into account the inputs from the {@link RecommendationRequest}. + * + * @param request a {@link RecommendationRequest} instance containing the details of the request + * @param remoteCallback a {@link RemoteCallback} instance to invoke when the recommendation + * is available. + * @throws SecurityException if the caller is not the system + */ + oneway void requestRecommendationAsync(in RecommendationRequest request, + in RemoteCallback remoteCallback); } diff --git a/core/java/android/net/NetworkScoreManager.java b/core/java/android/net/NetworkScoreManager.java index 2870dd6c57783..7a832d10bf3eb 100644 --- a/core/java/android/net/NetworkScoreManager.java +++ b/core/java/android/net/NetworkScoreManager.java @@ -16,15 +16,23 @@ package android.net; +import static android.net.NetworkRecommendationProvider.EXTRA_RECOMMENDATION_RESULT; + +import android.Manifest; import android.annotation.IntDef; +import android.annotation.NonNull; import android.annotation.SdkConstant; import android.annotation.SdkConstant.SdkConstantType; import android.annotation.SystemApi; import android.content.Context; import android.net.NetworkScorerAppManager.NetworkScorerAppData; +import android.os.Bundle; +import android.os.Handler; +import android.os.RemoteCallback; import android.os.RemoteException; import android.os.ServiceManager; import android.os.ServiceManager.ServiceNotFoundException; +import com.android.internal.util.Preconditions; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -354,4 +362,53 @@ public class NetworkScoreManager { throw e.rethrowFromSystemServer(); } } + + /** + * Request a recommendation for which network to connect to. + * + *

The callback will be run on the thread associated with provided {@link Handler}. + * + * @param request a {@link RecommendationRequest} instance containing additional + * request details + * @param callback a {@link RecommendationCallback} instance that will be invoked when + * the {@link RecommendationResult} is available + * @param handler a {@link Handler} instance representing the thread to run the callback on. + * @throws SecurityException + * @hide + */ + public void requestRecommendation( + final @NonNull RecommendationRequest request, + final @NonNull RecommendationCallback callback, + final @NonNull Handler handler) { + Preconditions.checkNotNull(request, "RecommendationRequest cannot be null."); + Preconditions.checkNotNull(callback, "RecommendationCallback cannot be null."); + Preconditions.checkNotNull(handler, "Handler cannot be null."); + + RemoteCallback remoteCallback = new RemoteCallback(new RemoteCallback.OnResultListener() { + @Override + public void onResult(Bundle data) { + RecommendationResult result = data.getParcelable(EXTRA_RECOMMENDATION_RESULT); + callback.onRecommendationAvailable(result); + } + }, handler); + + try { + mService.requestRecommendationAsync(request, remoteCallback); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + + /** + * Callers of {@link #requestRecommendation(RecommendationRequest, RecommendationCallback, Handler)} + * must pass in an implementation of this class. + * @hide + */ + public abstract static class RecommendationCallback { + /** + * Invoked when a {@link RecommendationResult} is available. + * @param result a {@link RecommendationResult} instance. + */ + public abstract void onRecommendationAvailable(RecommendationResult result); + } } diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java index e23844c3e633b..a170b46fd4f86 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -45,6 +45,7 @@ import android.os.Binder; import android.os.Bundle; import android.os.IBinder; import android.os.IRemoteCallback; +import android.os.RemoteCallback; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.UserHandle; @@ -553,6 +554,33 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } } + /** + * Request a recommendation for the best network to connect to + * taking into account the inputs from the {@link RecommendationRequest}. + * + * @param request a {@link RecommendationRequest} instance containing the details of the request + * @param remoteCallback a {@link IRemoteCallback} instance to invoke when the recommendation + * is available. + * @throws SecurityException if the caller is not the system + */ + @Override + public void requestRecommendationAsync(RecommendationRequest request, + RemoteCallback remoteCallback) { + // TODO(jjoslin): 12/28/16 - Provide actual impl. + + final RecommendationResult result; + if (request != null && request.getCurrentSelectedConfig() != null) { + result = RecommendationResult.createConnectRecommendation( + request.getCurrentSelectedConfig()); + } else { + result = RecommendationResult.createDoNotConnectRecommendation(); + } + + final Bundle data = new Bundle(); + data.putParcelable(EXTRA_RECOMMENDATION_RESULT, result); + remoteCallback.sendResult(data); + } + @Override public boolean requestScores(NetworkKey[] networks) { mContext.enforceCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES, TAG); From ce73c6f296f583223cee4385e20c640ead3ae4de Mon Sep 17 00:00:00 2001 From: Jeremy Joslin Date: Thu, 29 Dec 2016 14:49:38 -0800 Subject: [PATCH 2/2] Implemented the async recommendation request call. Implemented requestAsyncRecommendation() by introducing a Handler implementation to handle requests that time out and a OneTimeCallback class to prevent multiple callbacks from being sent back for the same request. Change-Id: Ida2ff860d78d86185ab9ab22232b5b6dc1e4b310 Test: runtest frameworks-services -c com.android.server.NetworkScoreServiceTest BUG:33784158 --- .../android/server/NetworkScoreService.java | 138 ++++++++++++++++-- .../server/NetworkScoreServiceTest.java | 133 ++++++++++++++++- 2 files changed, 255 insertions(+), 16 deletions(-) diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java index a170b46fd4f86..1b1b2060f739c 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -42,9 +42,13 @@ import android.net.RecommendationResult; import android.net.ScoredNetwork; import android.net.Uri; import android.os.Binder; +import android.os.Build; import android.os.Bundle; +import android.os.Handler; import android.os.IBinder; import android.os.IRemoteCallback; +import android.os.Looper; +import android.os.Message; import android.os.RemoteCallback; import android.os.RemoteCallbackList; import android.os.RemoteException; @@ -52,6 +56,7 @@ import android.os.UserHandle; import android.provider.Settings.Global; import android.util.ArrayMap; import android.util.Log; +import android.util.Pair; import android.util.TimedRemoteCaller; import com.android.internal.annotations.GuardedBy; @@ -68,6 +73,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; /** @@ -76,7 +82,7 @@ import java.util.function.Consumer; */ public class NetworkScoreService extends INetworkScoreService.Stub { private static final String TAG = "NetworkScoreService"; - private static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG); + private static final boolean DBG = Build.IS_DEBUGGABLE && Log.isLoggable(TAG, Log.DEBUG); private final Context mContext; private final NetworkScorerAppManager mNetworkScorerAppManager; @@ -84,13 +90,15 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @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]; + private final Object mPackageMonitorLock = new Object(); + private final Object mServiceConnectionLock = new Object(); + private final Handler mHandler; @GuardedBy("mPackageMonitorLock") private NetworkScorerPackageMonitor mPackageMonitor; @GuardedBy("mServiceConnectionLock") private ScoringServiceConnection mServiceConnection; + private long mRecommendationRequestTimeoutMs; private BroadcastReceiver mUserIntentReceiver = new BroadcastReceiver() { @Override @@ -207,11 +215,12 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } public NetworkScoreService(Context context) { - this(context, new NetworkScorerAppManager(context)); + this(context, new NetworkScorerAppManager(context), Looper.myLooper()); } @VisibleForTesting - NetworkScoreService(Context context, NetworkScorerAppManager networkScoreAppManager) { + NetworkScoreService(Context context, NetworkScorerAppManager networkScoreAppManager, + Looper looper) { mContext = context; mNetworkScorerAppManager = networkScoreAppManager; mScoreCaches = new ArrayMap<>(); @@ -223,6 +232,8 @@ public class NetworkScoreService extends INetworkScoreService.Stub { // TODO(jjoslin): 12/15/16 - Make timeout configurable. mRequestRecommendationCaller = new RequestRecommendationCaller(TimedRemoteCaller.DEFAULT_CALL_TIMEOUT_MILLIS); + mRecommendationRequestTimeoutMs = TimedRemoteCaller.DEFAULT_CALL_TIMEOUT_MILLIS; + mHandler = new ServiceHandler(looper); } /** Called when the system is ready to run third-party code but before it actually does so. */ @@ -566,19 +577,42 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public void requestRecommendationAsync(RecommendationRequest request, RemoteCallback remoteCallback) { - // TODO(jjoslin): 12/28/16 - Provide actual impl. + mContext.enforceCallingOrSelfPermission(permission.BROADCAST_NETWORK_PRIVILEGED, TAG); - final RecommendationResult result; - if (request != null && request.getCurrentSelectedConfig() != null) { - result = RecommendationResult.createConnectRecommendation( - request.getCurrentSelectedConfig()); - } else { - result = RecommendationResult.createDoNotConnectRecommendation(); + final OneTimeCallback oneTimeCallback = new OneTimeCallback(remoteCallback); + final Pair pair = + Pair.create(request, oneTimeCallback); + final Message timeoutMsg = mHandler.obtainMessage( + ServiceHandler.MSG_RECOMMENDATION_REQUEST_TIMEOUT, pair); + final INetworkRecommendationProvider provider = getRecommendationProvider(); + final long token = Binder.clearCallingIdentity(); + try { + if (provider != null) { + try { + mHandler.sendMessageDelayed(timeoutMsg, mRecommendationRequestTimeoutMs); + provider.requestRecommendation(request, new IRemoteCallback.Stub() { + @Override + public void sendResult(Bundle data) throws RemoteException { + // Remove the timeout message + mHandler.removeMessages(timeoutMsg.what, pair); + oneTimeCallback.sendResult(data); + } + }, 0 /*sequence*/); + return; + } catch (RemoteException e) { + Log.w(TAG, "Failed to request a recommendation.", e); + // TODO(jjoslin): 12/15/16 - Keep track of failures. + // Remove the timeout message + mHandler.removeMessages(timeoutMsg.what, pair); + // Will fall through and send back the default recommendation. + } + } + } finally { + Binder.restoreCallingIdentity(token); } - final Bundle data = new Bundle(); - data.putParcelable(EXTRA_RECOMMENDATION_RESULT, result); - remoteCallback.sendResult(data); + // Else send back the default recommendation. + sendDefaultRecommendationResponse(request, oneTimeCallback); } @Override @@ -679,6 +713,11 @@ public class NetworkScoreService extends INetworkScoreService.Stub { return null; } + @VisibleForTesting + public void setRecommendationRequestTimeoutMs(long recommendationRequestTimeoutMs) { + mRecommendationRequestTimeoutMs = recommendationRequestTimeoutMs; + } + private static class ScoringServiceConnection implements ServiceConnection { private final ComponentName mComponentName; private final int mScoringAppUid; @@ -784,4 +823,73 @@ public class NetworkScoreService extends INetworkScoreService.Stub { return getResultTimed(sequence); } } + + /** + * A wrapper around {@link RemoteCallback} that guarantees + * {@link RemoteCallback#sendResult(Bundle)} will be invoked at most once. + */ + @VisibleForTesting + public static final class OneTimeCallback { + private final RemoteCallback mRemoteCallback; + private final AtomicBoolean mCallbackRun; + + public OneTimeCallback(RemoteCallback remoteCallback) { + mRemoteCallback = remoteCallback; + mCallbackRun = new AtomicBoolean(false); + } + + public void sendResult(Bundle data) { + if (mCallbackRun.compareAndSet(false, true)) { + mRemoteCallback.sendResult(data); + } + } + } + + private static void sendDefaultRecommendationResponse(RecommendationRequest request, + OneTimeCallback remoteCallback) { + if (DBG) { + Log.d(TAG, "Returning the default network recommendation."); + } + + final RecommendationResult result; + if (request != null && request.getCurrentSelectedConfig() != null) { + result = RecommendationResult.createConnectRecommendation( + request.getCurrentSelectedConfig()); + } else { + result = RecommendationResult.createDoNotConnectRecommendation(); + } + + final Bundle data = new Bundle(); + data.putParcelable(EXTRA_RECOMMENDATION_RESULT, result); + remoteCallback.sendResult(data); + } + + @VisibleForTesting + public static final class ServiceHandler extends Handler { + public static final int MSG_RECOMMENDATION_REQUEST_TIMEOUT = 1; + + public ServiceHandler(Looper looper) { + super(looper); + } + + @Override + public void handleMessage(Message msg) { + final int what = msg.what; + switch (what) { + case MSG_RECOMMENDATION_REQUEST_TIMEOUT: + if (DBG) { + Log.d(TAG, "Network recommendation request timed out."); + } + final Pair pair = + (Pair) msg.obj; + final RecommendationRequest request = pair.first; + final OneTimeCallback remoteCallback = pair.second; + sendDefaultRecommendationResponse(request, remoteCallback); + break; + + default: + Log.w(TAG,"Unknown message: " + what); + } + } + } } diff --git a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java index 1189dae7b21ce..8851922506a31 100644 --- a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java @@ -24,6 +24,7 @@ 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; @@ -64,17 +65,21 @@ import android.net.WifiKey; import android.net.wifi.WifiConfiguration; import android.os.Binder; import android.os.Bundle; +import android.os.HandlerThread; import android.os.IBinder; import android.os.IRemoteCallback; import android.os.Looper; +import android.os.RemoteCallback; 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 android.util.Pair; import com.android.server.devicepolicy.MockUtils; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -89,6 +94,8 @@ import java.io.FileDescriptor; import java.io.PrintWriter; import java.io.StringWriter; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; /** * Tests for {@link NetworkScoreService}. @@ -114,6 +121,9 @@ public class NetworkScoreServiceTest { private ContentResolver mContentResolver; private NetworkScoreService mNetworkScoreService; private RecommendationRequest mRecommendationRequest; + private RemoteCallback mRemoteCallback; + private OnResultListener mOnResultListener; + private HandlerThread mHandlerThread; @Before public void setUp() throws Exception { @@ -123,12 +133,22 @@ public class NetworkScoreServiceTest { mContentResolver = InstrumentationRegistry.getContext().getContentResolver(); when(mContext.getContentResolver()).thenReturn(mContentResolver); when(mContext.getResources()).thenReturn(mResources); - mNetworkScoreService = new NetworkScoreService(mContext, mNetworkScorerAppManager); + mHandlerThread = new HandlerThread("NetworkScoreServiceTest"); + mHandlerThread.start(); + mNetworkScoreService = new NetworkScoreService(mContext, mNetworkScorerAppManager, + mHandlerThread.getLooper()); WifiConfiguration configuration = new WifiConfiguration(); configuration.SSID = "NetworkScoreServiceTest_SSID"; configuration.BSSID = "NetworkScoreServiceTest_BSSID"; mRecommendationRequest = new RecommendationRequest.Builder() .setCurrentRecommendedWifiConfig(configuration).build(); + mOnResultListener = new OnResultListener(); + mRemoteCallback = new RemoteCallback(mOnResultListener); + } + + @After + public void tearDown() throws Exception { + mHandlerThread.quitSafely(); } @Test @@ -261,6 +281,104 @@ public class NetworkScoreServiceTest { result.getWifiConfiguration().BSSID); } + @Test + public void testRequestRecommendationAsync_noPermission() throws Exception { + doThrow(new SecurityException()).when(mContext) + .enforceCallingOrSelfPermission(eq(permission.BROADCAST_NETWORK_PRIVILEGED), + anyString()); + try { + mNetworkScoreService.requestRecommendationAsync(mRecommendationRequest, + mRemoteCallback); + fail("BROADCAST_NETWORK_PRIVILEGED not enforced."); + } catch (SecurityException e) { + // expected + } + } + + @Test + public void testRequestRecommendationAsync_providerNotConnected() throws Exception { + mNetworkScoreService.requestRecommendationAsync(mRecommendationRequest, + mRemoteCallback); + boolean callbackRan = mOnResultListener.countDownLatch.await(3, TimeUnit.SECONDS); + assertTrue(callbackRan); + verifyZeroInteractions(mRecommendationProvider); + } + + @Test + public void testRequestRecommendationAsync_requestTimesOut() throws Exception { + injectProvider(); + mNetworkScoreService.setRecommendationRequestTimeoutMs(0L); + mNetworkScoreService.requestRecommendationAsync(mRecommendationRequest, + mRemoteCallback); + boolean callbackRan = mOnResultListener.countDownLatch.await(3, TimeUnit.SECONDS); + assertTrue(callbackRan); + verify(mRecommendationProvider).requestRecommendation(eq(mRecommendationRequest), + isA(IRemoteCallback.Stub.class), anyInt()); + } + + @Test + public void testRequestRecommendationAsync_requestSucceeds() throws Exception { + injectProvider(); + final Bundle bundle = new Bundle(); + doAnswer(invocation -> { + invocation.getArgumentAt(1, IRemoteCallback.class).sendResult(bundle); + return null; + }).when(mRecommendationProvider) + .requestRecommendation(eq(mRecommendationRequest), isA(IRemoteCallback.class), + anyInt()); + + mNetworkScoreService.requestRecommendationAsync(mRecommendationRequest, + mRemoteCallback); + boolean callbackRan = mOnResultListener.countDownLatch.await(3, TimeUnit.SECONDS); + assertTrue(callbackRan); + // If it's not the same instance then something else ran the callback. + assertSame(bundle, mOnResultListener.receivedBundle); + } + + @Test + public void testRequestRecommendationAsync_requestThrowsRemoteException() throws Exception { + injectProvider(); + doThrow(new RemoteException()).when(mRecommendationProvider) + .requestRecommendation(eq(mRecommendationRequest), isA(IRemoteCallback.class), + anyInt()); + + mNetworkScoreService.requestRecommendationAsync(mRecommendationRequest, + mRemoteCallback); + boolean callbackRan = mOnResultListener.countDownLatch.await(3, TimeUnit.SECONDS); + assertTrue(callbackRan); + } + + @Test + public void oneTimeCallback_multipleCallbacks() throws Exception { + NetworkScoreService.OneTimeCallback callback = + new NetworkScoreService.OneTimeCallback(mRemoteCallback); + callback.sendResult(null); + callback.sendResult(null); + assertEquals(1, mOnResultListener.resultCount); + } + + @Test + public void serviceHandler_timeoutMsg() throws Exception { + NetworkScoreService.ServiceHandler handler = + new NetworkScoreService.ServiceHandler(mHandlerThread.getLooper()); + NetworkScoreService.OneTimeCallback callback = + new NetworkScoreService.OneTimeCallback(mRemoteCallback); + final Pair pair = + Pair.create(mRecommendationRequest, callback); + handler.obtainMessage( + NetworkScoreService.ServiceHandler.MSG_RECOMMENDATION_REQUEST_TIMEOUT, pair) + .sendToTarget(); + + boolean callbackRan = mOnResultListener.countDownLatch.await(3, TimeUnit.SECONDS); + assertTrue(callbackRan); + assertTrue(mOnResultListener.receivedBundle.containsKey(EXTRA_RECOMMENDATION_RESULT)); + RecommendationResult result = + mOnResultListener.receivedBundle.getParcelable(EXTRA_RECOMMENDATION_RESULT); + assertTrue(result.hasRecommendation()); + assertEquals(mRecommendationRequest.getCurrentSelectedConfig().SSID, + result.getWifiConfiguration().SSID); + } + @Test public void testUpdateScores_notActiveScorer() { bindToScorer(false /*callerIsScorer*/); @@ -515,4 +633,17 @@ public class NetworkScoreServiceTest { isA(UserHandle.class))).thenReturn(true); mNetworkScoreService.systemRunning(); } + + private static class OnResultListener implements RemoteCallback.OnResultListener { + private final CountDownLatch countDownLatch = new CountDownLatch(1); + private int resultCount; + private Bundle receivedBundle; + + @Override + public void onResult(Bundle result) { + countDownLatch.countDown(); + resultCount++; + receivedBundle = result; + } + } }