OSDN Git Service

Implement the request and recommend calls.
authorJeremy Joslin <jjoslin@google.com>
Fri, 9 Dec 2016 21:11:51 +0000 (13:11 -0800)
committerJeremy Joslin <jjoslin@google.com>
Tue, 20 Dec 2016 01:53:47 +0000 (17:53 -0800)
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: I055251fa94d93d4ea4e5001c4ec6c1dfb626d1b2
Merged-In: I8e2ed73dc6876deb1a8bd47bcaeaca8db68f3a44

core/java/android/net/NetworkScoreManager.java
services/core/java/com/android/server/NetworkScoreService.java
services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java

index 9c6c769..1825956 100644 (file)
@@ -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.
      *
+     * <p>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
index 723d5d8..f712f12 100644 (file)
 
 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<Integer, RemoteCallbackList<INetworkScoreCache>> 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<RecommendationResult> {
+        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);
+        }
+    }
 }
index e59addb..c653b8e 100644 (file)
 
 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<List<ScoredNetwork>> 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<Boolean>() {
+            @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();
+    }
 }