Merge "Use IBinder as key for ConnectivityDiagnostics storage in CS."

This commit is contained in:
Cody Kesting
2020-03-31 17:00:03 +00:00
committed by Gerrit Code Review
2 changed files with 18 additions and 22 deletions

View File

@@ -653,8 +653,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
final MultipathPolicyTracker mMultipathPolicyTracker; final MultipathPolicyTracker mMultipathPolicyTracker;
@VisibleForTesting @VisibleForTesting
final Map<IConnectivityDiagnosticsCallback, ConnectivityDiagnosticsCallbackInfo> final Map<IBinder, ConnectivityDiagnosticsCallbackInfo> mConnectivityDiagnosticsCallbacks =
mConnectivityDiagnosticsCallbacks = new HashMap<>(); new HashMap<>();
/** /**
* Implements support for the legacy "one network per network type" model. * Implements support for the legacy "one network per network type" model.
@@ -7826,11 +7826,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
ensureRunningOnConnectivityServiceThread(); ensureRunningOnConnectivityServiceThread();
final IConnectivityDiagnosticsCallback cb = cbInfo.mCb; final IConnectivityDiagnosticsCallback cb = cbInfo.mCb;
final IBinder iCb = cb.asBinder();
final NetworkRequestInfo nri = cbInfo.mRequestInfo; final NetworkRequestInfo nri = cbInfo.mRequestInfo;
// This means that the client registered the same callback multiple times. Do // This means that the client registered the same callback multiple times. Do
// not override the previous entry, and exit silently. // not override the previous entry, and exit silently.
if (mConnectivityDiagnosticsCallbacks.containsKey(cb)) { if (mConnectivityDiagnosticsCallbacks.containsKey(iCb)) {
if (VDBG) log("Diagnostics callback is already registered"); if (VDBG) log("Diagnostics callback is already registered");
// Decrement the reference count for this NetworkRequestInfo. The reference count is // Decrement the reference count for this NetworkRequestInfo. The reference count is
@@ -7840,10 +7841,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
return; return;
} }
mConnectivityDiagnosticsCallbacks.put(cb, cbInfo); mConnectivityDiagnosticsCallbacks.put(iCb, cbInfo);
try { try {
cb.asBinder().linkToDeath(cbInfo, 0); iCb.linkToDeath(cbInfo, 0);
} catch (RemoteException e) { } catch (RemoteException e) {
cbInfo.binderDied(); cbInfo.binderDied();
return; return;
@@ -7880,13 +7881,14 @@ public class ConnectivityService extends IConnectivityManager.Stub
private void handleUnregisterConnectivityDiagnosticsCallback( private void handleUnregisterConnectivityDiagnosticsCallback(
@NonNull IConnectivityDiagnosticsCallback cb, int uid) { @NonNull IConnectivityDiagnosticsCallback cb, int uid) {
ensureRunningOnConnectivityServiceThread(); ensureRunningOnConnectivityServiceThread();
final IBinder iCb = cb.asBinder();
if (!mConnectivityDiagnosticsCallbacks.containsKey(cb)) { if (!mConnectivityDiagnosticsCallbacks.containsKey(iCb)) {
if (VDBG) log("Removing diagnostics callback that is not currently registered"); if (VDBG) log("Removing diagnostics callback that is not currently registered");
return; return;
} }
final NetworkRequestInfo nri = mConnectivityDiagnosticsCallbacks.get(cb).mRequestInfo; final NetworkRequestInfo nri = mConnectivityDiagnosticsCallbacks.get(iCb).mRequestInfo;
if (uid != nri.mUid) { if (uid != nri.mUid) {
if (VDBG) loge("Different uid than registrant attempting to unregister cb"); if (VDBG) loge("Different uid than registrant attempting to unregister cb");
@@ -7898,7 +7900,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
// enforceRequestCountLimit(). // enforceRequestCountLimit().
decrementNetworkRequestPerUidCount(nri); decrementNetworkRequestPerUidCount(nri);
cb.asBinder().unlinkToDeath(mConnectivityDiagnosticsCallbacks.remove(cb), 0); final ConnectivityDiagnosticsCallbackInfo cbInfo =
mConnectivityDiagnosticsCallbacks.remove(iCb);
iCb.unlinkToDeath(cbInfo, 0);
} }
private void handleNetworkTestedWithExtras( private void handleNetworkTestedWithExtras(
@@ -7973,14 +7977,14 @@ public class ConnectivityService extends IConnectivityManager.Stub
private List<IConnectivityDiagnosticsCallback> getMatchingPermissionedCallbacks( private List<IConnectivityDiagnosticsCallback> getMatchingPermissionedCallbacks(
@NonNull NetworkAgentInfo nai) { @NonNull NetworkAgentInfo nai) {
final List<IConnectivityDiagnosticsCallback> results = new ArrayList<>(); final List<IConnectivityDiagnosticsCallback> results = new ArrayList<>();
for (Entry<IConnectivityDiagnosticsCallback, ConnectivityDiagnosticsCallbackInfo> entry : for (Entry<IBinder, ConnectivityDiagnosticsCallbackInfo> entry :
mConnectivityDiagnosticsCallbacks.entrySet()) { mConnectivityDiagnosticsCallbacks.entrySet()) {
final ConnectivityDiagnosticsCallbackInfo cbInfo = entry.getValue(); final ConnectivityDiagnosticsCallbackInfo cbInfo = entry.getValue();
final NetworkRequestInfo nri = cbInfo.mRequestInfo; final NetworkRequestInfo nri = cbInfo.mRequestInfo;
if (nai.satisfies(nri.request)) { if (nai.satisfies(nri.request)) {
if (checkConnectivityDiagnosticsPermissions( if (checkConnectivityDiagnosticsPermissions(
nri.mPid, nri.mUid, nai, cbInfo.mCallingPackageName)) { nri.mPid, nri.mUid, nai, cbInfo.mCallingPackageName)) {
results.add(entry.getKey()); results.add(entry.getValue().mCb);
} }
} }
} }

View File

@@ -6744,16 +6744,12 @@ public class ConnectivityServiceTest {
verify(mIBinder).linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); verify(mIBinder).linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt());
verify(mConnectivityDiagnosticsCallback).asBinder(); verify(mConnectivityDiagnosticsCallback).asBinder();
assertTrue( assertTrue(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder));
mService.mConnectivityDiagnosticsCallbacks.containsKey(
mConnectivityDiagnosticsCallback));
mService.unregisterConnectivityDiagnosticsCallback(mConnectivityDiagnosticsCallback); mService.unregisterConnectivityDiagnosticsCallback(mConnectivityDiagnosticsCallback);
verify(mIBinder, timeout(TIMEOUT_MS)) verify(mIBinder, timeout(TIMEOUT_MS))
.unlinkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); .unlinkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt());
assertFalse( assertFalse(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder));
mService.mConnectivityDiagnosticsCallbacks.containsKey(
mConnectivityDiagnosticsCallback));
verify(mConnectivityDiagnosticsCallback, atLeastOnce()).asBinder(); verify(mConnectivityDiagnosticsCallback, atLeastOnce()).asBinder();
} }
@@ -6771,9 +6767,7 @@ public class ConnectivityServiceTest {
verify(mIBinder).linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); verify(mIBinder).linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt());
verify(mConnectivityDiagnosticsCallback).asBinder(); verify(mConnectivityDiagnosticsCallback).asBinder();
assertTrue( assertTrue(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder));
mService.mConnectivityDiagnosticsCallbacks.containsKey(
mConnectivityDiagnosticsCallback));
// Register the same callback again // Register the same callback again
mService.registerConnectivityDiagnosticsCallback( mService.registerConnectivityDiagnosticsCallback(
@@ -6782,9 +6776,7 @@ public class ConnectivityServiceTest {
// Block until all other events are done processing. // Block until all other events are done processing.
HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS); HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS);
assertTrue( assertTrue(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder));
mService.mConnectivityDiagnosticsCallbacks.containsKey(
mConnectivityDiagnosticsCallback));
} }
@Test @Test