New setting for recommendation request timeout.

am: 44e2b84b27

Change-Id: I8fd6e0c75d22d0f83db414a389c4d8caaf745469

core/java/android/provider/Settings.java

@@ -7665,6 +7665,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.

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<Uri, Integer> mUriEventMap;
+        final private Context mContext;
+        final private Handler mHandler;
 
-        public SettingsObserver() {
+        public DispatchingContentObserver(Context context, Handler handler) {
-            super(null /*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<String> providerPackages =
             mNetworkScorerAppManager.getPotentialRecommendationProviderPackages();
         if (!providerPackages.isEmpty()) {
-            final ContentResolver resolver = mContext.getContentResolver();
+            final Uri enabledUri = Global.getUriFor(Global.NETWORK_RECOMMENDATIONS_ENABLED);
-            final Uri uri = Global.getUriFor(Global.NETWORK_RECOMMENDATIONS_ENABLED);
+            mContentObserver.observe(enabledUri,
-            resolver.registerContentObserver(uri, false, new SettingsObserver());
+                    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) {
+    public void refreshRecommendationRequestTimeoutMs() {
-        mRecommendationRequestTimeoutMs = recommendationRequestTimeoutMs;
+        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);
             }

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<RecommendationRequest, NetworkScoreService.OneTimeCallback> 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;
+        }
+    }
 }