From bc1308a3bec3a097af377a1c3ca58da5d7c85e66 Mon Sep 17 00:00:00 2001 From: Jeremy Joslin Date: Thu, 29 Dec 2016 14:49:38 -0800 Subject: [PATCH] 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: I03875cf1d789cbc92aa4c6b500c6b519bff8e165 Merged-In: 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; + } + } }