From cb925074d3e646c7aca07585a267bb17605d6030 Mon Sep 17 00:00:00 2001 From: Jeremy Joslin Date: Wed, 28 Dec 2016 15:56:46 -0800 Subject: [PATCH 1/3] 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: I8d210b686138cb42bf69185f0b2f2d25dfcb9dd1 Merged-In: I8f84b09f43a6c5fae5d8f03ec01e75c25b4b62d6 --- .../android/net/INetworkScoreService.aidl | 13 +++++ .../java/android/net/NetworkScoreManager.java | 56 +++++++++++++++++++ .../android/server/NetworkScoreService.java | 28 ++++++++++ 3 files changed, 97 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 a6854dcb8eb3e..c57145f69d2f0 100644 --- a/core/java/android/net/NetworkScoreManager.java +++ b/core/java/android/net/NetworkScoreManager.java @@ -16,17 +16,24 @@ 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.content.Intent; import android.net.NetworkScorerAppManager.NetworkScorerAppData; +import android.os.Bundle; +import android.os.Handler; import android.os.IBinder; +import android.os.RemoteCallback; import android.os.RemoteException; import android.os.ServiceManager; +import com.android.internal.util.Preconditions; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -356,4 +363,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 bc1308a3bec3a097af377a1c3ca58da5d7c85e66 Mon Sep 17 00:00:00 2001 From: Jeremy Joslin Date: Thu, 29 Dec 2016 14:49:38 -0800 Subject: [PATCH 2/3] 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; + } + } } From 44e2b84b27dea139796299528e5cdd3abc4640f7 Mon Sep 17 00:00:00 2001 From: Jeremy Joslin Date: Tue, 3 Jan 2017 17:31:23 -0800 Subject: [PATCH 3/3] New setting for recommendation request timeout. Added a new global setting, NETWORK_RECOMMENDATION_REQUEST_TIMEOUT_MS, to control the maximum amount of time a recommendation request can take. Updated the NetworkScoreService to monitor the value and to update its cached copy on observed changes. Test: runtest frameworks-services -c com.android.server.NetworkScoreServiceTest Bug: 34060959 Change-Id: I6ff80178440794e4a5da39ee7b5164621316e7bd Merged-In: I7650ee024e53dbc856cf20d7520a6eb252c73bdf --- core/java/android/provider/Settings.java | 10 +++ .../android/server/NetworkScoreService.java | 72 +++++++++++++---- .../server/NetworkScoreServiceTest.java | 81 +++++++++++++------ 3 files changed, 124 insertions(+), 39 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 3ea5dcba815d2..35fa2108d8aae 100755 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -7672,6 +7672,16 @@ public final class Settings { public static final String NETWORK_RECOMMENDATIONS_ENABLED = "network_recommendations_enabled"; + /** + * The number of milliseconds the {@link com.android.server.NetworkScoreService} + * will give a recommendation request to complete before returning a default response. + * + * Type: long + * @hide + */ + public static final String NETWORK_RECOMMENDATION_REQUEST_TIMEOUT_MS = + "network_recommendation_request_timeout_ms"; + /** * Settings to allow BLE scans to be enabled even when Bluetooth is turned off for * connectivity. diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java index 1b1b2060f739c..bd9f6840abbd2 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -53,6 +53,7 @@ import android.os.RemoteCallback; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.UserHandle; +import android.provider.Settings; import android.provider.Settings.Global; import android.util.ArrayMap; import android.util.Log; @@ -93,12 +94,13 @@ public class NetworkScoreService extends INetworkScoreService.Stub { private final Object mPackageMonitorLock = new Object(); private final Object mServiceConnectionLock = new Object(); private final Handler mHandler; + private final DispatchingContentObserver mContentObserver; @GuardedBy("mPackageMonitorLock") private NetworkScorerPackageMonitor mPackageMonitor; @GuardedBy("mServiceConnectionLock") private ScoringServiceConnection mServiceConnection; - private long mRecommendationRequestTimeoutMs; + private volatile long mRecommendationRequestTimeoutMs; private BroadcastReceiver mUserIntentReceiver = new BroadcastReceiver() { @Override @@ -194,12 +196,25 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } /** - * Reevaluates the service binding when the Settings toggle is changed. + * Dispatches observed content changes to a handler for further processing. */ - private class SettingsObserver extends ContentObserver { + @VisibleForTesting + public static class DispatchingContentObserver extends ContentObserver { + final private Map mUriEventMap; + final private Context mContext; + final private Handler mHandler; - public SettingsObserver() { - super(null /*handler*/); + public DispatchingContentObserver(Context context, Handler handler) { + super(handler); + mContext = context; + mHandler = handler; + mUriEventMap = new ArrayMap<>(); + } + + void observe(Uri uri, int what) { + mUriEventMap.put(uri, what); + final ContentResolver resolver = mContext.getContentResolver(); + resolver.registerContentObserver(uri, false /*notifyForDescendants*/, this); } @Override @@ -210,7 +225,12 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public void onChange(boolean selfChange, Uri uri) { if (DBG) Log.d(TAG, String.format("onChange(%s, %s)", selfChange, uri)); - bindToScoringServiceIfNeeded(); + final Integer what = mUriEventMap.get(uri); + if (what != null) { + mHandler.obtainMessage(what).sendToTarget(); + } else { + Log.w(TAG, "No matching event to send for URI = " + uri); + } } } @@ -229,18 +249,19 @@ 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); mRecommendationRequestTimeoutMs = TimedRemoteCaller.DEFAULT_CALL_TIMEOUT_MILLIS; mHandler = new ServiceHandler(looper); + mContentObserver = new DispatchingContentObserver(context, mHandler); } /** Called when the system is ready to run third-party code but before it actually does so. */ void systemReady() { if (DBG) Log.d(TAG, "systemReady"); registerPackageMonitorIfNeeded(); - registerRecommendationSettingObserverIfNeeded(); + registerRecommendationSettingsObserver(); + refreshRecommendationRequestTimeoutMs(); } /** Called when the system is ready for us to start third-party code. */ @@ -254,14 +275,18 @@ public class NetworkScoreService extends INetworkScoreService.Stub { bindToScoringServiceIfNeeded(); } - private void registerRecommendationSettingObserverIfNeeded() { + private void registerRecommendationSettingsObserver() { final List providerPackages = mNetworkScorerAppManager.getPotentialRecommendationProviderPackages(); if (!providerPackages.isEmpty()) { - final ContentResolver resolver = mContext.getContentResolver(); - final Uri uri = Global.getUriFor(Global.NETWORK_RECOMMENDATIONS_ENABLED); - resolver.registerContentObserver(uri, false, new SettingsObserver()); + final Uri enabledUri = Global.getUriFor(Global.NETWORK_RECOMMENDATIONS_ENABLED); + mContentObserver.observe(enabledUri, + ServiceHandler.MSG_RECOMMENDATIONS_ENABLED_CHANGED); } + + final Uri timeoutUri = Global.getUriFor(Global.NETWORK_RECOMMENDATION_REQUEST_TIMEOUT_MS); + mContentObserver.observe(timeoutUri, + ServiceHandler.MSG_RECOMMENDATION_REQUEST_TIMEOUT_CHANGED); } private void registerPackageMonitorIfNeeded() { @@ -714,8 +739,15 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } @VisibleForTesting - public void setRecommendationRequestTimeoutMs(long recommendationRequestTimeoutMs) { - mRecommendationRequestTimeoutMs = recommendationRequestTimeoutMs; + public void refreshRecommendationRequestTimeoutMs() { + final ContentResolver cr = mContext.getContentResolver(); + long timeoutMs = Settings.Global.getLong(cr, + Global.NETWORK_RECOMMENDATION_REQUEST_TIMEOUT_MS, -1L /*default*/); + if (timeoutMs < 0) { + timeoutMs = TimedRemoteCaller.DEFAULT_CALL_TIMEOUT_MILLIS; + } + if (DBG) Log.d(TAG, "Updating the recommendation request timeout to " + timeoutMs + " ms"); + mRecommendationRequestTimeoutMs = timeoutMs; } private static class ScoringServiceConnection implements ServiceConnection { @@ -865,8 +897,10 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } @VisibleForTesting - public static final class ServiceHandler extends Handler { + public final class ServiceHandler extends Handler { public static final int MSG_RECOMMENDATION_REQUEST_TIMEOUT = 1; + public static final int MSG_RECOMMENDATIONS_ENABLED_CHANGED = 2; + public static final int MSG_RECOMMENDATION_REQUEST_TIMEOUT_CHANGED = 3; public ServiceHandler(Looper looper) { super(looper); @@ -887,6 +921,14 @@ public class NetworkScoreService extends INetworkScoreService.Stub { sendDefaultRecommendationResponse(request, remoteCallback); break; + case MSG_RECOMMENDATIONS_ENABLED_CHANGED: + bindToScoringServiceIfNeeded(); + break; + + case MSG_RECOMMENDATION_REQUEST_TIMEOUT_CHANGED: + refreshRecommendationRequestTimeoutMs(); + 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 8851922506a31..75d9c3911fca6 100644 --- a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java @@ -61,21 +61,24 @@ import android.net.NetworkScorerAppManager.NetworkScorerAppData; import android.net.RecommendationRequest; import android.net.RecommendationResult; import android.net.ScoredNetwork; +import android.net.Uri; import android.net.WifiKey; import android.net.wifi.WifiConfiguration; import android.os.Binder; import android.os.Bundle; +import android.os.Handler; import android.os.HandlerThread; import android.os.IBinder; import android.os.IRemoteCallback; import android.os.Looper; +import android.os.Message; import android.os.RemoteCallback; import android.os.RemoteException; import android.os.UserHandle; +import android.provider.Settings; 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; @@ -144,6 +147,9 @@ public class NetworkScoreServiceTest { .setCurrentRecommendedWifiConfig(configuration).build(); mOnResultListener = new OnResultListener(); mRemoteCallback = new RemoteCallback(mOnResultListener); + Settings.Global.putLong(mContentResolver, + Settings.Global.NETWORK_RECOMMENDATION_REQUEST_TIMEOUT_MS, -1L); + mNetworkScoreService.refreshRecommendationRequestTimeoutMs(); } @After @@ -307,13 +313,22 @@ public class NetworkScoreServiceTest { @Test public void testRequestRecommendationAsync_requestTimesOut() throws Exception { injectProvider(); - mNetworkScoreService.setRecommendationRequestTimeoutMs(0L); + Settings.Global.putLong(mContentResolver, + Settings.Global.NETWORK_RECOMMENDATION_REQUEST_TIMEOUT_MS, 1L); + mNetworkScoreService.refreshRecommendationRequestTimeoutMs(); mNetworkScoreService.requestRecommendationAsync(mRecommendationRequest, mRemoteCallback); boolean callbackRan = mOnResultListener.countDownLatch.await(3, TimeUnit.SECONDS); assertTrue(callbackRan); verify(mRecommendationProvider).requestRecommendation(eq(mRecommendationRequest), isA(IRemoteCallback.Stub.class), anyInt()); + + 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 @@ -348,6 +363,31 @@ public class NetworkScoreServiceTest { assertTrue(callbackRan); } + @Test + public void dispatchingContentObserver_nullUri() throws Exception { + NetworkScoreService.DispatchingContentObserver observer = + new NetworkScoreService.DispatchingContentObserver(mContext, null /*handler*/); + + observer.onChange(false, null); + // nothing to assert or verify but since we passed in a null handler we'd see a NPE + // if it were interacted with. + } + + @Test + public void dispatchingContentObserver_dispatchUri() throws Exception { + final CountDownHandler handler = new CountDownHandler(mHandlerThread.getLooper()); + NetworkScoreService.DispatchingContentObserver observer = + new NetworkScoreService.DispatchingContentObserver(mContext, handler); + Uri uri = Uri.parse("content://settings/global/network_score_service_test"); + int expectedWhat = 24; + observer.observe(uri, expectedWhat); + + observer.onChange(false, uri); + final boolean msgHandled = handler.latch.await(3, TimeUnit.SECONDS); + assertTrue(msgHandled); + assertEquals(expectedWhat, handler.receivedWhat); + } + @Test public void oneTimeCallback_multipleCallbacks() throws Exception { NetworkScoreService.OneTimeCallback callback = @@ -357,28 +397,6 @@ public class NetworkScoreServiceTest { 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*/); @@ -646,4 +664,19 @@ public class NetworkScoreServiceTest { receivedBundle = result; } } + + private static class CountDownHandler extends Handler { + CountDownLatch latch = new CountDownLatch(1); + int receivedWhat; + + CountDownHandler(Looper looper) { + super(looper); + } + + @Override + public void handleMessage(Message msg) { + latch.countDown(); + receivedWhat = msg.what; + } + } }