Merge changes from topic 'allow_none_scorer' into oc-dev

* changes:
  Revert permission check on setActiveScorer.
  Allow the active scorer to be set to NULL.

core/java/android/net/INetworkScoreService.aidl

@@ -45,7 +45,7 @@ interface INetworkScoreService
      * Set the active scorer and clear existing scores.
      * @param packageName the package name of the new scorer to use.
      * @return true if the operation succeeded, or false if the new package is not a valid scorer.
-     * @throws SecurityException if the caller is not the system.
+     * @throws SecurityException if the caller is not the system or a network scorer.
      */
     boolean setActiveScorer(in String packageName);
 

core/java/android/net/NetworkScoreManager.java

@@ -16,27 +16,20 @@
 
 package android.net;
 
-import static android.net.NetworkRecommendationProvider.EXTRA_RECOMMENDATION_RESULT;
+import android.Manifest.permission;
-
 import android.annotation.IntDef;
-import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.SdkConstant;
 import android.annotation.SdkConstant.SdkConstantType;
 import android.annotation.SystemApi;
 import android.content.Context;
-import android.os.Bundle;
-import android.os.Handler;
-import android.os.RemoteCallback;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.os.ServiceManager.ServiceNotFoundException;
-import com.android.internal.util.Preconditions;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.List;
-import java.util.concurrent.CompletableFuture;
 
 /**
  * Class that manages communication between network subsystems and a network scorer.
@@ -49,9 +42,9 @@ import java.util.concurrent.CompletableFuture;
  *
  * <p>A network scorer is any application which:
  * <ul>
- * <li>Declares the {@link android.Manifest.permission#SCORE_NETWORKS} permission.
+ * <li>Declares the {@link permission#SCORE_NETWORKS} permission.
  * <li>Include a Service for the {@link #ACTION_RECOMMEND_NETWORKS} action
- *     protected by the {@link android.Manifest.permission#BIND_NETWORK_RECOMMENDATION_SERVICE}
+ *     protected by the {@link permission#BIND_NETWORK_RECOMMENDATION_SERVICE}
  *     permission.
  * </ul>
  *
@@ -319,7 +312,7 @@ public class NetworkScoreManager {
      *
      * @return true if the operation succeeded, or false if the new package is not a valid scorer.
      * @throws SecurityException if the caller is not a system process or does not hold the
-     *         {@link android.Manifest.permission#REQUEST_NETWORK_SCORES} permission
+     *         {@link permission#SCORE_NETWORKS} permission
      * @hide
      */
     @SystemApi
@@ -351,7 +344,7 @@ public class NetworkScoreManager {
      *
      * @return true if the broadcast was sent, or false if there is no active scorer.
      * @throws SecurityException if the caller does not hold the
-     *         {@link android.Manifest.permission#REQUEST_NETWORK_SCORES} permission.
+     *         {@link permission#REQUEST_NETWORK_SCORES} permission.
      * @hide
      */
     public boolean requestScores(NetworkKey[] networks) throws SecurityException {
@@ -368,7 +361,7 @@ public class NetworkScoreManager {
      * @param networkType the type of network this cache can handle. See {@link NetworkKey#type}.
      * @param scoreCache implementation of {@link INetworkScoreCache} to store the scores.
      * @throws SecurityException if the caller does not hold the
-     *         {@link android.Manifest.permission#REQUEST_NETWORK_SCORES} permission.
+     *         {@link permission#REQUEST_NETWORK_SCORES} permission.
      * @throws IllegalArgumentException if a score cache is already registered for this type.
      * @deprecated equivalent to registering for cache updates with CACHE_FILTER_NONE.
      * @hide
@@ -385,7 +378,7 @@ public class NetworkScoreManager {
      * @param scoreCache implementation of {@link INetworkScoreCache} to store the scores
      * @param filterType the {@link CacheUpdateFilter} to apply
      * @throws SecurityException if the caller does not hold the
-     *         {@link android.Manifest.permission#REQUEST_NETWORK_SCORES} permission.
+     *         {@link permission#REQUEST_NETWORK_SCORES} permission.
      * @throws IllegalArgumentException if a score cache is already registered for this type.
      * @hide
      */
@@ -404,7 +397,7 @@ public class NetworkScoreManager {
      * @param networkType the type of network this cache can handle. See {@link NetworkKey#type}.
      * @param scoreCache implementation of {@link INetworkScoreCache} to store the scores.
      * @throws SecurityException if the caller does not hold the
-     *         {@link android.Manifest.permission#REQUEST_NETWORK_SCORES} permission.
+     *         {@link permission#REQUEST_NETWORK_SCORES} permission.
      * @throws IllegalArgumentException if a score cache is already registered for this type.
      * @hide
      */
@@ -416,25 +409,6 @@ 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
-     *         to connect to
-     * @throws SecurityException if the caller does not hold the
-     *         {@link android.Manifest.permission#REQUEST_NETWORK_SCORES} permission.
-     * @hide
-     * @deprecated to be removed.
-     */
-    public RecommendationResult requestRecommendation(RecommendationRequest request)
-            throws SecurityException {
-        return null;
-    }
-
     /**
      * Determine whether the application with the given UID is the enabled scorer.
      *

services/core/java/com/android/server/NetworkScoreService.java

@@ -631,6 +631,11 @@ public class NetworkScoreService extends INetworkScoreService.Stub {
                  PackageManager.PERMISSION_GRANTED;
     }
 
+    private boolean callerCanScoreNetworks() {
+        return mContext.checkCallingOrSelfPermission(permission.SCORE_NETWORKS) ==
+                PackageManager.PERMISSION_GRANTED;
+    }
+
     @Override
     public boolean clearScores() {
         // Only the active scorer or the system should be allowed to flush all scores.
@@ -651,9 +656,9 @@ public class NetworkScoreService extends INetworkScoreService.Stub {
     @Override
     public boolean setActiveScorer(String packageName) {
         // Only the system can set the active scorer
-        if (!isCallerSystemProcess(getCallingUid()) && !callerCanRequestScores()) {
+        if (!isCallerSystemProcess(getCallingUid()) && !callerCanScoreNetworks()) {
             throw new SecurityException(
-                    "Caller is neither the system process nor a score requester.");
+                    "Caller is neither the system process or a network scorer.");
         }
 
         return mNetworkScorerAppManager.setActiveScorer(packageName);

services/core/java/com/android/server/NetworkScorerAppManager.java

@@ -208,11 +208,11 @@ public class NetworkScorerAppManager {
      *
      * <p>The caller must have permission to write to {@link Settings.Global}.
      *
-     * @param packageName the packageName of the new scorer to use. If null, the scoring app will
+     * @param packageName the packageName of the new scorer to use. If null, scoring will be forced
-     *                    revert back to the configured default. Otherwise, the scorer will only
+     *                    off, otherwise the scorer will only be set if it is a valid scorer
-     *                    be set if it is a valid scorer application.
+     *                    application.
-     * @return true if the scorer was changed, or false if the package is not a valid scorer or
+     * @return true if the package was a valid scorer (including <code>null</code>) and now
-     *         a valid network recommendation provider exists.
+     *         represents the active scorer, false otherwise.
      */
     @VisibleForTesting
     public boolean setActiveScorer(String packageName) {
@@ -223,16 +223,19 @@ public class NetworkScorerAppManager {
             return true;
         }
 
-        if (packageName == null) {
+        if (TextUtils.isEmpty(packageName)) {
-            // revert to the default setting.
+            Log.i(TAG, "Network scorer forced off, was: " + oldPackageName);
-            packageName = getDefaultPackageSetting();
+            setNetworkRecommendationsPackage(null);
+            setNetworkRecommendationsEnabledSetting(
+                    NetworkScoreManager.RECOMMENDATIONS_ENABLED_FORCED_OFF);
+            return true;
         }
 
-        Log.i(TAG, "Changing network scorer from " + oldPackageName + " to " + packageName);
-
         // We only make the change if the new package is valid.
         if (getScorer(packageName) != null) {
+            Log.i(TAG, "Changing network scorer from " + oldPackageName + " to " + packageName);
             setNetworkRecommendationsPackage(packageName);
+            setNetworkRecommendationsEnabledSetting(NetworkScoreManager.RECOMMENDATIONS_ENABLED_ON);
             return true;
         } else {
             Log.w(TAG, "Requested network scorer is not valid: " + packageName);

services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java

@@ -368,8 +368,8 @@ public class NetworkScoreServiceTest {
     }
 
     @Test
-    public void testSetActiveScorer_noRequestNetworkScoresPermission() {
+    public void testSetActiveScorer_noScoreNetworksPermission() {
-        when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES))
+        when(mContext.checkCallingOrSelfPermission(permission.SCORE_NETWORKS))
                 .thenReturn(PackageManager.PERMISSION_DENIED);
 
         try {

services/tests/servicestests/src/com/android/server/NetworkScorerAppManagerTest.java

@@ -228,30 +228,23 @@ public class NetworkScorerAppManagerTest {
     }
 
     @Test
-    public void testSetActiveScorer_nullPackage_validDefault() throws Exception {
+    public void testSetActiveScorer_nullPackage_currentIsSet() throws Exception {
-        String packageName = "package";
+        setNetworkRecoPackageSetting("package");
-        String defaultPackage = "defaultPackage";
-        setNetworkRecoPackageSetting(packageName);
-        setDefaultNetworkRecommendationPackage(defaultPackage);
-        final ComponentName recoComponent = new ComponentName(defaultPackage, "class1");
-        mockScoreNetworksGranted(recoComponent.getPackageName());
-        mockRecommendationServiceAvailable(recoComponent, 924 /* packageUid */, null);
 
         assertTrue(mNetworkScorerAppManager.setActiveScorer(null));
         verify(mSettingsFacade).putString(mMockContext,
-                Settings.Global.NETWORK_RECOMMENDATIONS_PACKAGE, defaultPackage);
+                Settings.Global.NETWORK_RECOMMENDATIONS_PACKAGE, null);
+        verify(mSettingsFacade).putInt(mMockContext,
+                Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED,
+                NetworkScoreManager.RECOMMENDATIONS_ENABLED_FORCED_OFF);
     }
 
     @Test
-    public void testSetActiveScorer_nullPackage_invalidDefault() throws Exception {
+    public void testSetActiveScorer_nullPackage_currentIsNull() throws Exception {
-        String packageName = "package";
+        setNetworkRecoPackageSetting(null);
-        String defaultPackage = "defaultPackage";
-        setNetworkRecoPackageSetting(packageName);
-        setDefaultNetworkRecommendationPackage(defaultPackage);
 
-        assertFalse(mNetworkScorerAppManager.setActiveScorer(null));
+        assertTrue(mNetworkScorerAppManager.setActiveScorer(null));
-        verify(mSettingsFacade, never()).putString(any(),
+        verify(mSettingsFacade, never()).putString(any(), any(), any());
-                eq(Settings.Global.NETWORK_RECOMMENDATIONS_PACKAGE), any());
     }
 
     @Test
@@ -266,6 +259,9 @@ public class NetworkScorerAppManagerTest {
         assertTrue(mNetworkScorerAppManager.setActiveScorer(newPackage));
         verify(mSettingsFacade).putString(mMockContext,
                 Settings.Global.NETWORK_RECOMMENDATIONS_PACKAGE, newPackage);
+        verify(mSettingsFacade).putInt(mMockContext,
+                Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED,
+                NetworkScoreManager.RECOMMENDATIONS_ENABLED_ON);
     }
 
     @Test
@@ -340,6 +336,32 @@ public class NetworkScorerAppManagerTest {
                 NetworkScoreManager.RECOMMENDATIONS_ENABLED_OFF);
     }
 
+    @Test
+    public void testUpdateState_currentPackageNull_defaultNull() throws Exception {
+        setDefaultNetworkRecommendationPackage(null);
+        setNetworkRecoPackageSetting(null);
+
+        mNetworkScorerAppManager.updateState();
+
+        verify(mSettingsFacade, never()).putString(any(),
+                eq(Settings.Global.NETWORK_RECOMMENDATIONS_PACKAGE), anyString());
+        verify(mSettingsFacade, never()).putInt(any(),
+                eq(Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED), anyInt());
+    }
+
+    @Test
+    public void testUpdateState_currentPackageEmpty_defaultEmpty() throws Exception {
+        setDefaultNetworkRecommendationPackage("");
+        setNetworkRecoPackageSetting("");
+
+        mNetworkScorerAppManager.updateState();
+
+        verify(mSettingsFacade, never()).putString(any(),
+                eq(Settings.Global.NETWORK_RECOMMENDATIONS_PACKAGE), anyString());
+        verify(mSettingsFacade, never()).putInt(any(),
+                eq(Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED), anyInt());
+    }
+
     @Test
     public void testUpdateState_currentPackageNotValid_sameAsDefault() throws Exception {
         String defaultPackage = "defaultPackage";