Merge "Ensure callbacks are run properly on executor" am: 411d2bd07d am: f8224f5364

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2111166

Change-Id: I70f6ccf9f42717b846e816067e90916792cec143
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Remi NGUYEN VAN
2022-06-03 09:42:25 +00:00
committed by Automerger Merge Worker
2 changed files with 103 additions and 36 deletions

View File

@@ -306,9 +306,12 @@ public final class NsdManager {
@Override
public void onAvailable(@NonNull Network network) {
final DelegatingDiscoveryListener wrappedListener = new DelegatingDiscoveryListener(
network, mBaseListener);
network, mBaseListener, mBaseExecutor);
mPerNetworkListeners.put(network, wrappedListener);
discoverServices(mServiceType, mProtocolType, network, mBaseExecutor,
// Run discovery callbacks inline on the service handler thread, which is the
// same thread used by this NetworkCallback, but DelegatingDiscoveryListener will
// use the base executor to run the wrapped callbacks.
discoverServices(mServiceType, mProtocolType, network, Runnable::run,
wrappedListener);
}
@@ -328,7 +331,8 @@ public final class NsdManager {
public void start(@NonNull NetworkRequest request) {
final ConnectivityManager cm = mContext.getSystemService(ConnectivityManager.class);
cm.registerNetworkCallback(request, mNetworkCb, mHandler);
mHandler.post(() -> mBaseListener.onDiscoveryStarted(mServiceType));
mHandler.post(() -> mBaseExecutor.execute(() ->
mBaseListener.onDiscoveryStarted(mServiceType)));
}
/**
@@ -345,7 +349,7 @@ public final class NsdManager {
final ConnectivityManager cm = mContext.getSystemService(ConnectivityManager.class);
cm.unregisterNetworkCallback(mNetworkCb);
if (mPerNetworkListeners.size() == 0) {
mBaseListener.onDiscoveryStopped(mServiceType);
mBaseExecutor.execute(() -> mBaseListener.onDiscoveryStopped(mServiceType));
return;
}
for (int i = 0; i < mPerNetworkListeners.size(); i++) {
@@ -393,14 +397,23 @@ public final class NsdManager {
}
}
/**
* A listener wrapping calls to an app-provided listener, while keeping track of found
* services, so they can all be reported lost when the underlying network is lost.
*
* This should be registered to run on the service handler.
*/
private class DelegatingDiscoveryListener implements DiscoveryListener {
private final Network mNetwork;
private final DiscoveryListener mWrapped;
private final Executor mWrappedExecutor;
private final ArraySet<TrackedNsdInfo> mFoundInfo = new ArraySet<>();
private DelegatingDiscoveryListener(Network network, DiscoveryListener listener) {
private DelegatingDiscoveryListener(Network network, DiscoveryListener listener,
Executor executor) {
mNetwork = network;
mWrapped = listener;
mWrappedExecutor = executor;
}
void notifyAllServicesLost() {
@@ -409,7 +422,7 @@ public final class NsdManager {
final NsdServiceInfo serviceInfo = new NsdServiceInfo(
trackedInfo.mServiceName, trackedInfo.mServiceType);
serviceInfo.setNetwork(mNetwork);
mWrapped.onServiceLost(serviceInfo);
mWrappedExecutor.execute(() -> mWrapped.onServiceLost(serviceInfo));
}
}
@@ -438,7 +451,7 @@ public final class NsdManager {
// Do not report onStopDiscoveryFailed when some underlying listeners failed:
// this does not mean that all listeners did, and onStopDiscoveryFailed is not
// actionable anyway. Just report that discovery stopped.
mWrapped.onDiscoveryStopped(serviceType);
mWrappedExecutor.execute(() -> mWrapped.onDiscoveryStopped(serviceType));
}
}
@@ -446,20 +459,20 @@ public final class NsdManager {
public void onDiscoveryStopped(String serviceType) {
mPerNetworkListeners.remove(mNetwork);
if (mStopRequested && mPerNetworkListeners.size() == 0) {
mWrapped.onDiscoveryStopped(serviceType);
mWrappedExecutor.execute(() -> mWrapped.onDiscoveryStopped(serviceType));
}
}
@Override
public void onServiceFound(NsdServiceInfo serviceInfo) {
mFoundInfo.add(new TrackedNsdInfo(serviceInfo));
mWrapped.onServiceFound(serviceInfo);
mWrappedExecutor.execute(() -> mWrapped.onServiceFound(serviceInfo));
}
@Override
public void onServiceLost(NsdServiceInfo serviceInfo) {
mFoundInfo.remove(new TrackedNsdInfo(serviceInfo));
mWrapped.onServiceLost(serviceInfo);
mWrappedExecutor.execute(() -> mWrapped.onServiceLost(serviceInfo));
}
}
}
@@ -642,8 +655,12 @@ public final class NsdManager {
@Override
public void handleMessage(Message message) {
// Do not use message in the executor lambdas, as it will be recycled once this method
// returns. Keep references to its content instead.
final int what = message.what;
final int errorCode = message.arg1;
final int key = message.arg2;
final Object obj = message.obj;
final Object listener;
final NsdServiceInfo ns;
final Executor executor;
@@ -653,7 +670,7 @@ public final class NsdManager {
executor = mExecutorMap.get(key);
}
if (listener == null) {
Log.d(TAG, "Stale key " + message.arg2);
Log.d(TAG, "Stale key " + key);
return;
}
if (DBG) {
@@ -661,28 +678,28 @@ public final class NsdManager {
}
switch (what) {
case DISCOVER_SERVICES_STARTED:
final String s = getNsdServiceInfoType((NsdServiceInfo) message.obj);
final String s = getNsdServiceInfoType((NsdServiceInfo) obj);
executor.execute(() -> ((DiscoveryListener) listener).onDiscoveryStarted(s));
break;
case DISCOVER_SERVICES_FAILED:
removeListener(key);
executor.execute(() -> ((DiscoveryListener) listener).onStartDiscoveryFailed(
getNsdServiceInfoType(ns), message.arg1));
getNsdServiceInfoType(ns), errorCode));
break;
case SERVICE_FOUND:
executor.execute(() -> ((DiscoveryListener) listener).onServiceFound(
(NsdServiceInfo) message.obj));
(NsdServiceInfo) obj));
break;
case SERVICE_LOST:
executor.execute(() -> ((DiscoveryListener) listener).onServiceLost(
(NsdServiceInfo) message.obj));
(NsdServiceInfo) obj));
break;
case STOP_DISCOVERY_FAILED:
// TODO: failure to stop discovery should be internal and retried internally, as
// the effect for the client is indistinguishable from STOP_DISCOVERY_SUCCEEDED
removeListener(key);
executor.execute(() -> ((DiscoveryListener) listener).onStopDiscoveryFailed(
getNsdServiceInfoType(ns), message.arg1));
getNsdServiceInfoType(ns), errorCode));
break;
case STOP_DISCOVERY_SUCCEEDED:
removeListener(key);
@@ -692,33 +709,33 @@ public final class NsdManager {
case REGISTER_SERVICE_FAILED:
removeListener(key);
executor.execute(() -> ((RegistrationListener) listener).onRegistrationFailed(
ns, message.arg1));
ns, errorCode));
break;
case REGISTER_SERVICE_SUCCEEDED:
executor.execute(() -> ((RegistrationListener) listener).onServiceRegistered(
(NsdServiceInfo) message.obj));
(NsdServiceInfo) obj));
break;
case UNREGISTER_SERVICE_FAILED:
removeListener(key);
executor.execute(() -> ((RegistrationListener) listener).onUnregistrationFailed(
ns, message.arg1));
ns, errorCode));
break;
case UNREGISTER_SERVICE_SUCCEEDED:
// TODO: do not unregister listener until service is unregistered, or provide
// alternative way for unregistering ?
removeListener(message.arg2);
removeListener(key);
executor.execute(() -> ((RegistrationListener) listener).onServiceUnregistered(
ns));
break;
case RESOLVE_SERVICE_FAILED:
removeListener(key);
executor.execute(() -> ((ResolveListener) listener).onResolveFailed(
ns, message.arg1));
ns, errorCode));
break;
case RESOLVE_SERVICE_SUCCEEDED:
removeListener(key);
executor.execute(() -> ((ResolveListener) listener).onServiceResolved(
(NsdServiceInfo) message.obj));
(NsdServiceInfo) obj));
break;
default:
Log.d(TAG, "Ignored " + message);