From 37e877beccb9f3edd2a1dd33e01a76a7cd71c711 Mon Sep 17 00:00:00 2001 From: Jeremy Joslin Date: Thu, 2 Feb 2017 11:06:14 -0800 Subject: [PATCH] Cache NetworkScorerAppData in the ScoringServiceConnection. Store the entire NetworkScorerAppData instance in the ScoringServiceConnection instead of just most of its fields. Test: runtest frameworks-services -c com.android.server.NetworkScoreServiceTest Bug: 34773276 Change-Id: Id2ed7c431dee9895e85e1966903ac919f1704eba --- .../android/net/NetworkScorerAppManager.java | 58 +++++++++++-------- .../net/NetworkScorerAppManagerTest.java | 48 ++++++++------- .../android/server/NetworkScoreService.java | 46 ++++++++------- .../server/NetworkScoreServiceTest.java | 19 +++--- 4 files changed, 97 insertions(+), 74 deletions(-) diff --git a/core/java/android/net/NetworkScorerAppManager.java b/core/java/android/net/NetworkScorerAppManager.java index 9e4dd8748f390..9dcf4f44a5596 100644 --- a/core/java/android/net/NetworkScorerAppManager.java +++ b/core/java/android/net/NetworkScorerAppManager.java @@ -18,6 +18,7 @@ package android.net; import android.Manifest.permission; import android.annotation.Nullable; +import android.content.ComponentName; import android.content.ContentResolver; import android.content.Context; import android.content.Intent; @@ -33,6 +34,7 @@ import com.android.internal.R; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; /** * Internal class for discovering and managing the network scorer/recommendation application. @@ -53,33 +55,43 @@ public class NetworkScorerAppManager { * Holds metadata about a discovered network scorer/recommendation application. */ public static class NetworkScorerAppData { - /** Package name of this scorer app. */ - public final String packageName; - /** UID of the scorer app. */ public final int packageUid; + private final ComponentName mRecommendationService; - /** - * Name of the recommendation service we can bind to. - */ - public final String recommendationServiceClassName; - - public NetworkScorerAppData(String packageName, int packageUid, - String recommendationServiceClassName) { - this.packageName = packageName; + public NetworkScorerAppData(int packageUid, ComponentName recommendationServiceComp) { this.packageUid = packageUid; - this.recommendationServiceClassName = recommendationServiceClassName; + this.mRecommendationService = recommendationServiceComp; + } + + public String getRecommendationServicePackageName() { + return mRecommendationService.getPackageName(); + } + + public ComponentName getRecommendationServiceComponent() { + return mRecommendationService; } @Override public String toString() { - final StringBuilder sb = new StringBuilder("NetworkScorerAppData{"); - sb.append("mPackageName='").append(packageName).append('\''); - sb.append(", packageUid=").append(packageUid); - sb.append(", recommendationServiceClassName='") - .append(recommendationServiceClassName).append('\''); - sb.append('}'); - return sb.toString(); + return "NetworkScorerAppData{" + + "packageUid=" + packageUid + + ", mRecommendationService=" + mRecommendationService + + '}'; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + NetworkScorerAppData that = (NetworkScorerAppData) o; + return packageUid == that.packageUid && + Objects.equals(mRecommendationService, that.mRecommendationService); + } + + @Override + public int hashCode() { + return Objects.hash(packageUid, mRecommendationService); } } @@ -110,16 +122,16 @@ public class NetworkScorerAppManager { return null; } - final PackageManager pm = mContext.getPackageManager(); for (int i = 0; i < potentialPkgs.size(); i++) { final String potentialPkg = potentialPkgs.get(i); // Look for the recommendation service class and required receiver. final ResolveInfo resolveServiceInfo = findRecommendationService(potentialPkg); if (resolveServiceInfo != null) { - return new NetworkScorerAppData(potentialPkg, - resolveServiceInfo.serviceInfo.applicationInfo.uid, - resolveServiceInfo.serviceInfo.name); + final ComponentName componentName = + new ComponentName(potentialPkg, resolveServiceInfo.serviceInfo.name); + return new NetworkScorerAppData(resolveServiceInfo.serviceInfo.applicationInfo.uid, + componentName); } else { if (DEBUG) { Log.d(TAG, potentialPkg + " does not have the required components, skipping."); diff --git a/core/tests/coretests/src/android/net/NetworkScorerAppManagerTest.java b/core/tests/coretests/src/android/net/NetworkScorerAppManagerTest.java index 5bfff26b08131..ce5d3ef665bbe 100644 --- a/core/tests/coretests/src/android/net/NetworkScorerAppManagerTest.java +++ b/core/tests/coretests/src/android/net/NetworkScorerAppManagerTest.java @@ -19,6 +19,7 @@ package android.net; import static org.mockito.Mockito.when; import android.Manifest.permission; +import android.content.ComponentName; import android.content.ContentResolver; import android.content.Context; import android.content.Intent; @@ -30,13 +31,16 @@ import android.content.res.Resources; import android.net.NetworkScorerAppManager.NetworkScorerAppData; import android.provider.Settings; import android.test.InstrumentationTestCase; + import com.android.internal.R; -import java.util.List; + import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import java.util.List; + public class NetworkScorerAppManagerTest extends InstrumentationTestCase { @Mock private Context mMockContext; @Mock private PackageManager mMockPm; @@ -114,39 +118,40 @@ public class NetworkScorerAppManagerTest extends InstrumentationTestCase { public void testGetNetworkRecommendationProviderData_scoreNetworksNotGranted() throws Exception { - setNetworkRecommendationPackageNames("package1"); - mockScoreNetworksDenied("package1"); - mockRecommendationServiceAvailable("package1", 924 /* packageUid */); + final ComponentName recoComponent = new ComponentName("package1", "class1"); + setNetworkRecommendationPackageNames(recoComponent.getPackageName()); + mockScoreNetworksDenied(recoComponent.getPackageName()); + mockRecommendationServiceAvailable(recoComponent, 924 /* packageUid */); assertNull(mNetworkScorerAppManager.getNetworkRecommendationProviderData()); } public void testGetNetworkRecommendationProviderData_available() throws Exception { - setNetworkRecommendationPackageNames("package1"); - mockScoreNetworksGranted("package1"); - mockRecommendationServiceAvailable("package1", 924 /* packageUid */); + final ComponentName recoComponent = new ComponentName("package1", "class1"); + setNetworkRecommendationPackageNames(recoComponent.getPackageName()); + mockScoreNetworksGranted(recoComponent.getPackageName()); + mockRecommendationServiceAvailable(recoComponent, 924 /* packageUid */); NetworkScorerAppData appData = mNetworkScorerAppManager.getNetworkRecommendationProviderData(); assertNotNull(appData); - assertEquals("package1", appData.packageName); + assertEquals(recoComponent, appData.getRecommendationServiceComponent()); assertEquals(924, appData.packageUid); - assertEquals(".RecommendationService", appData.recommendationServiceClassName); } public void testGetActiveScorer_providerAvailable() throws Exception { - setNetworkRecommendationPackageNames("package1"); - mockScoreNetworksGranted("package1"); - mockRecommendationServiceAvailable("package1", 924 /* packageUid */); + final ComponentName recoComponent = new ComponentName("package1", "class1"); + setNetworkRecommendationPackageNames(recoComponent.getPackageName()); + mockScoreNetworksGranted(recoComponent.getPackageName()); + mockRecommendationServiceAvailable(recoComponent, 924 /* packageUid */); ContentResolver cr = mTargetContext.getContentResolver(); Settings.Global.putInt(cr, Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED, 1); final NetworkScorerAppData activeScorer = mNetworkScorerAppManager.getActiveScorer(); assertNotNull(activeScorer); - assertEquals("package1", activeScorer.packageName); + assertEquals(recoComponent, activeScorer.getRecommendationServiceComponent()); assertEquals(924, activeScorer.packageUid); - assertEquals(".RecommendationService", activeScorer.recommendationServiceClassName); } public void testGetActiveScorer_providerNotAvailable() @@ -159,9 +164,10 @@ public class NetworkScorerAppManagerTest extends InstrumentationTestCase { } public void testGetActiveScorer_recommendationsDisabled() throws Exception { - setNetworkRecommendationPackageNames("package1"); - mockScoreNetworksGranted("package1"); - mockRecommendationServiceAvailable("package1", 924 /* packageUid */); + final ComponentName recoComponent = new ComponentName("package1", "class1"); + setNetworkRecommendationPackageNames(recoComponent.getPackageName()); + mockScoreNetworksGranted(recoComponent.getPackageName()); + mockRecommendationServiceAvailable(recoComponent, 924 /* packageUid */); ContentResolver cr = mTargetContext.getContentResolver(); Settings.Global.putInt(cr, Settings.Global.NETWORK_RECOMMENDATIONS_ENABLED, 0); @@ -187,11 +193,11 @@ public class NetworkScorerAppManagerTest extends InstrumentationTestCase { .thenReturn(PackageManager.PERMISSION_DENIED); } - private void mockRecommendationServiceAvailable(final String packageName, int packageUid) { + private void mockRecommendationServiceAvailable(final ComponentName compName, int packageUid) { final ResolveInfo serviceInfo = new ResolveInfo(); serviceInfo.serviceInfo = new ServiceInfo(); - serviceInfo.serviceInfo.name = ".RecommendationService"; - serviceInfo.serviceInfo.packageName = packageName; + serviceInfo.serviceInfo.name = compName.getClassName(); + serviceInfo.serviceInfo.packageName = compName.getPackageName(); serviceInfo.serviceInfo.applicationInfo = new ApplicationInfo(); serviceInfo.serviceInfo.applicationInfo.uid = packageUid; @@ -203,7 +209,7 @@ public class NetworkScorerAppManagerTest extends InstrumentationTestCase { Intent intent = (Intent) object; return NetworkScoreManager.ACTION_RECOMMEND_NETWORKS .equals(intent.getAction()) - && packageName.equals(intent.getPackage()); + && compName.getPackageName().equals(intent.getPackage()); } }), Mockito.eq(flags))).thenReturn(serviceInfo); } diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java index 25016f6b9cf50..abb6a5381356b 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -183,11 +183,12 @@ public class NetworkScoreService extends INetworkScoreService.Stub { // connection. if (DBG) Log.d(TAG, "No active scorers available."); unbindFromScoringServiceIfNeeded(); - } else if (activeScorer.packageName.equals(scorerPackageName)) { + } else if (activeScorer.getRecommendationServicePackageName().equals(scorerPackageName)) + { // The active scoring service changed in some way. if (DBG) { Log.d(TAG, "Possible change to the active scorer: " - + activeScorer.packageName); + + activeScorer.getRecommendationServicePackageName()); } if (forceUnbind) { unbindFromScoringServiceIfNeeded(); @@ -198,7 +199,8 @@ public class NetworkScoreService extends INetworkScoreService.Stub { // bound to the correct scoring app. The logic in bindToScoringServiceIfNeeded() // will sort that out to leave us bound to the most recent active scorer. if (DBG) { - Log.d(TAG, "Binding to " + activeScorer.packageName + " if needed."); + Log.d(TAG, "Binding to " + activeScorer.getRecommendationServiceComponent() + + " if needed."); } bindToScoringServiceIfNeeded(activeScorer); } @@ -334,22 +336,19 @@ public class NetworkScoreService extends INetworkScoreService.Stub { bindToScoringServiceIfNeeded(scorerData); } - private void bindToScoringServiceIfNeeded(NetworkScorerAppData scorerData) { - if (DBG) Log.d(TAG, "bindToScoringServiceIfNeeded(" + scorerData + ")"); - if (scorerData != null && scorerData.recommendationServiceClassName != null) { - ComponentName componentName = new ComponentName(scorerData.packageName, - scorerData.recommendationServiceClassName); + private void bindToScoringServiceIfNeeded(NetworkScorerAppData appData) { + if (DBG) Log.d(TAG, "bindToScoringServiceIfNeeded(" + appData + ")"); + if (appData != null) { synchronized (mServiceConnectionLock) { // If we're connected to a different component then drop it. if (mServiceConnection != null - && !mServiceConnection.mComponentName.equals(componentName)) { + && !mServiceConnection.mAppData.equals(appData)) { unbindFromScoringServiceIfNeeded(); } // If we're not connected at all then create a new connection. if (mServiceConnection == null) { - mServiceConnection = new ScoringServiceConnection(componentName, - scorerData.packageUid); + mServiceConnection = new ScoringServiceConnection(appData); } // Make sure the connection is connected (idempotent) @@ -672,7 +671,8 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public boolean isCallerActiveScorer(int callingUid) { synchronized (mServiceConnectionLock) { - return mServiceConnection != null && mServiceConnection.mScoringAppUid == callingUid; + return mServiceConnection != null + && mServiceConnection.mAppData.packageUid == callingUid; } } @@ -686,7 +686,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { public String getActiveScorerPackage() { synchronized (mServiceConnectionLock) { if (mServiceConnection != null) { - return mServiceConnection.mComponentName.getPackageName(); + return mServiceConnection.getPackageName(); } } return null; @@ -881,7 +881,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { writer.println("Scoring is disabled."); return; } - writer.println("Current scorer: " + currentScorer.packageName); + writer.println("Current scorer: " + currentScorer); sendCacheUpdateCallback(new BiConsumer() { @Override @@ -966,21 +966,19 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } private static class ScoringServiceConnection implements ServiceConnection { - private final ComponentName mComponentName; - private final int mScoringAppUid; + private final NetworkScorerAppData mAppData; private volatile boolean mBound = false; private volatile boolean mConnected = false; private volatile INetworkRecommendationProvider mRecommendationProvider; - ScoringServiceConnection(ComponentName componentName, int scoringAppUid) { - mComponentName = componentName; - mScoringAppUid = scoringAppUid; + ScoringServiceConnection(NetworkScorerAppData appData) { + mAppData = appData; } void connect(Context context) { if (!mBound) { Intent service = new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS); - service.setComponent(mComponentName); + service.setComponent(mAppData.getRecommendationServiceComponent()); mBound = context.bindServiceAsUser(service, this, Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE, UserHandle.SYSTEM); @@ -1010,6 +1008,10 @@ public class NetworkScoreService extends INetworkScoreService.Stub { return mRecommendationProvider; } + String getPackageName() { + return mAppData.getRecommendationServiceComponent().getPackageName(); + } + @Override public void onServiceConnected(ComponentName name, IBinder service) { if (DBG) Log.d(TAG, "ScoringServiceConnection: " + name.flattenToString()); @@ -1027,7 +1029,9 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } public void dump(FileDescriptor fd, PrintWriter writer, String[] args) { - writer.println("ScoringServiceConnection: " + mComponentName + ", bound: " + mBound + writer.println("ScoringServiceConnection: " + + mAppData.getRecommendationServiceComponent() + + ", bound: " + mBound + ", connected: " + mConnected); } } diff --git a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java index 152b9c9ba0315..fa9e9a804e619 100644 --- a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java @@ -117,6 +117,8 @@ public class NetworkScoreServiceTest { private static final String SSID = "ssid"; private static final String SSID_2 = "ssid_2"; private static final String SSID_3 = "ssid_3"; + private static final ComponentName RECOMMENDATION_SERVICE_COMP = + new ComponentName("newPackageName", "newScoringServiceClass"); private static final ScoredNetwork SCORED_NETWORK = new ScoredNetwork(new NetworkKey(new WifiKey(quote(SSID), "00:00:00:00:00:00")), null /* rssiCurve*/); @@ -124,7 +126,7 @@ public class NetworkScoreServiceTest { new ScoredNetwork(new NetworkKey(new WifiKey(quote(SSID_2), "00:00:00:00:00:00")), null /* rssiCurve*/); private static final NetworkScorerAppData NEW_SCORER = - new NetworkScorerAppData("newPackageName", 1, "newScoringServiceClass"); + new NetworkScorerAppData(1, RECOMMENDATION_SERVICE_COMP); @Mock private NetworkScorerAppManager mNetworkScorerAppManager; @Mock private Context mContext; @@ -203,8 +205,7 @@ public class NetworkScoreServiceTest { verify(mContext).bindServiceAsUser(MockUtils.checkIntent( new Intent(NetworkScoreManager.ACTION_RECOMMEND_NETWORKS) - .setComponent(new ComponentName(NEW_SCORER.packageName, - NEW_SCORER.recommendationServiceClassName))), + .setComponent(RECOMMENDATION_SERVICE_COMP)), any(ServiceConnection.class), eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE), eq(UserHandle.SYSTEM)); @@ -657,7 +658,8 @@ public class NetworkScoreServiceTest { when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(NEW_SCORER); mNetworkScoreService.systemRunning(); - assertEquals(NEW_SCORER.packageName, mNetworkScoreService.getActiveScorerPackage()); + assertEquals(NEW_SCORER.getRecommendationServicePackageName(), + mNetworkScoreService.getActiveScorerPackage()); } @Test @@ -829,8 +831,6 @@ public class NetworkScoreServiceTest { // "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() { @@ -840,7 +840,8 @@ public class NetworkScoreServiceTest { when(mockBinder.queryLocalInterface(anyString())) .thenReturn(mRecommendationProvider); invocation.getArgumentAt(1, ServiceConnection.class) - .onServiceConnected(componentName, mockBinder); + .onServiceConnected(NEW_SCORER.getRecommendationServiceComponent(), + mockBinder); return true; } }); @@ -849,8 +850,8 @@ public class NetworkScoreServiceTest { private void bindToScorer(boolean callerIsScorer) { final int callingUid = callerIsScorer ? Binder.getCallingUid() : 0; - NetworkScorerAppData appData = new NetworkScorerAppData(NEW_SCORER.packageName, - callingUid, NEW_SCORER.recommendationServiceClassName); + NetworkScorerAppData appData = + new NetworkScorerAppData(callingUid, RECOMMENDATION_SERVICE_COMP); when(mNetworkScorerAppManager.getActiveScorer()).thenReturn(appData); when(mContext.bindServiceAsUser(isA(Intent.class), isA(ServiceConnection.class), anyInt(), isA(UserHandle.class))).thenReturn(true);