Fix Bug in ImsManager#addRegistrationListener
Listeners were not being unregistered from
the ImsService. The listeners were restructured
to instead be held in ImsManager, instead of the
ImsService.
Bug: 38304804
Test: Run Rogers VoWiFi activation
Merged-In: I2a0d4b2ed7d232834be7c74c318a60fa11e64389
Change-Id: I2a0d4b2ed7d232834be7c74c318a60fa11e64389
diff --git a/src/java/com/android/ims/ImsManager.java b/src/java/com/android/ims/ImsManager.java
index 214753f..7c1c182 100644
--- a/src/java/com/android/ims/ImsManager.java
+++ b/src/java/com/android/ims/ImsManager.java
@@ -204,7 +204,15 @@
// Keep track of the ImsRegistrationListenerProxys that have been created so that we can
// remove them from the ImsService.
- private Set<ImsRegistrationListenerProxy> mRegistrationListeners = new HashSet<>();
+ private final Set<ImsConnectionStateListener> mRegistrationListeners = new HashSet<>();
+
+ private final ImsRegistrationListenerProxy mRegistrationListenerProxy =
+ new ImsRegistrationListenerProxy();
+
+ // When true, we have registered the mRegistrationListenerProxy with the ImsService. Don't do
+ // it again.
+ private boolean mHasRegisteredForProxy = false;
+ private final Object mHasRegisteredLock = new Object();
// SystemProperties used as cache
private static final String VOLTE_PROVISIONED_PROP = "net.lte.ims.volte.provisioned";
@@ -1585,7 +1593,11 @@
try {
result = mImsServiceProxy.startSession(incomingCallPendingIntent,
- createRegistrationListenerProxy(serviceClass, listener));
+ mRegistrationListenerProxy);
+ synchronized (mHasRegisteredLock) {
+ mHasRegisteredForProxy = true;
+ addRegistrationListener(listener);
+ }
} catch (RemoteException e) {
throw new ImsException("open()", e,
ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
@@ -1609,23 +1621,43 @@
* @param listener To listen to IMS registration events; It cannot be null
* @throws NullPointerException if {@code listener} is null
* @throws ImsException if calling the IMS service results in an error
+ *
+ * @deprecated Use {@link #addRegistrationListener(ImsConnectionStateListener)} instead.
*/
public void addRegistrationListener(int serviceClass, ImsConnectionStateListener listener)
throws ImsException {
+ addRegistrationListener(listener);
+ }
+
+ /**
+ * Adds registration listener to the IMS service.
+ *
+ * @param listener To listen to IMS registration events; It cannot be null
+ * @throws NullPointerException if {@code listener} is null
+ * @throws ImsException if calling the IMS service results in an error
+ */
+ public void addRegistrationListener(ImsConnectionStateListener listener)
+ throws ImsException {
checkAndThrowExceptionIfServiceUnavailable();
if (listener == null) {
throw new NullPointerException("listener can't be null");
}
-
- try {
- ImsRegistrationListenerProxy p = createRegistrationListenerProxy(serviceClass,
- listener);
- mRegistrationListeners.add(p);
- mImsServiceProxy.addRegistrationListener(p);
- } catch (RemoteException e) {
- throw new ImsException("addRegistrationListener()", e,
- ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
+ // We only want this Proxy registered once. It can either happen here or in open().
+ synchronized (mHasRegisteredLock) {
+ if (!mHasRegisteredForProxy) {
+ try {
+ mImsServiceProxy.addRegistrationListener(mRegistrationListenerProxy);
+ // Only record if there isn't a RemoteException.
+ mHasRegisteredForProxy = true;
+ } catch (RemoteException e) {
+ throw new ImsException("addRegistrationListener()", e,
+ ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
+ }
+ }
+ }
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.add(listener);
}
}
@@ -1639,23 +1671,12 @@
*/
public void removeRegistrationListener(ImsConnectionStateListener listener)
throws ImsException {
- checkAndThrowExceptionIfServiceUnavailable();
-
if (listener == null) {
throw new NullPointerException("listener can't be null");
}
- try {
- Optional<ImsRegistrationListenerProxy> optionalProxy = mRegistrationListeners.stream()
- .filter(l -> listener.equals(l.mListener)).findFirst();
- if(optionalProxy.isPresent()) {
- ImsRegistrationListenerProxy p = optionalProxy.get();
- mRegistrationListeners.remove(p);
- mImsServiceProxy.removeRegistrationListener(p);
- }
- } catch (RemoteException e) {
- throw new ImsException("removeRegistrationListener()", e,
- ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.remove(listener);
}
}
@@ -2120,6 +2141,10 @@
Rlog.i(TAG, "Creating ImsService using ImsResolver");
mImsServiceProxy = getServiceProxy();
}
+ // We have created a new ImsService connection, signal for re-registration
+ synchronized (mHasRegisteredLock) {
+ mHasRegisteredForProxy = false;
+ }
}
// Deprecated method of binding with the ImsService defined in the ServiceManager.
@@ -2177,13 +2202,6 @@
}
}
- private ImsRegistrationListenerProxy createRegistrationListenerProxy(int serviceClass,
- ImsConnectionStateListener listener) {
- ImsRegistrationListenerProxy proxy =
- new ImsRegistrationListenerProxy(serviceClass, listener);
- return proxy;
- }
-
private static void log(String s) {
Rlog.d(TAG, s);
}
@@ -2298,18 +2316,6 @@
* Adapter class for {@link IImsRegistrationListener}.
*/
private class ImsRegistrationListenerProxy extends IImsRegistrationListener.Stub {
- private int mServiceClass;
- private ImsConnectionStateListener mListener;
-
- public ImsRegistrationListenerProxy(int serviceClass,
- ImsConnectionStateListener listener) {
- mServiceClass = serviceClass;
- mListener = listener;
- }
-
- public boolean isSameProxy(int serviceClass) {
- return (mServiceClass == serviceClass);
- }
@Deprecated
public void registrationConnected() {
@@ -2317,8 +2323,9 @@
log("registrationConnected ::");
}
- if (mListener != null) {
- mListener.onImsConnected(ServiceState.RIL_RADIO_TECHNOLOGY_UNKNOWN);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onImsConnected(
+ ServiceState.RIL_RADIO_TECHNOLOGY_UNKNOWN));
}
}
@@ -2328,8 +2335,9 @@
log("registrationProgressing ::");
}
- if (mListener != null) {
- mListener.onImsProgressing(ServiceState.RIL_RADIO_TECHNOLOGY_UNKNOWN);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onImsProgressing(
+ ServiceState.RIL_RADIO_TECHNOLOGY_UNKNOWN));
}
}
@@ -2341,8 +2349,8 @@
log("registrationConnectedWithRadioTech :: imsRadioTech=" + imsRadioTech);
}
- if (mListener != null) {
- mListener.onImsConnected(imsRadioTech);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onImsConnected(imsRadioTech));
}
}
@@ -2354,8 +2362,8 @@
log("registrationProgressingWithRadioTech :: imsRadioTech=" + imsRadioTech);
}
- if (mListener != null) {
- mListener.onImsProgressing(imsRadioTech);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onImsProgressing(imsRadioTech));
}
}
@@ -2366,9 +2374,8 @@
}
addToRecentDisconnectReasons(imsReasonInfo);
-
- if (mListener != null) {
- mListener.onImsDisconnected(imsReasonInfo);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onImsDisconnected(imsReasonInfo));
}
}
@@ -2378,8 +2385,8 @@
log("registrationResumed ::");
}
- if (mListener != null) {
- mListener.onImsResumed();
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(ImsConnectionStateListener::onImsResumed);
}
}
@@ -2389,8 +2396,8 @@
log("registrationSuspended ::");
}
- if (mListener != null) {
- mListener.onImsSuspended();
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(ImsConnectionStateListener::onImsSuspended);
}
}
@@ -2399,8 +2406,9 @@
log("registrationServiceCapabilityChanged :: serviceClass=" +
serviceClass + ", event=" + event);
- if (mListener != null) {
- mListener.onImsConnected(ServiceState.RIL_RADIO_TECHNOLOGY_UNKNOWN);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onImsConnected(
+ ServiceState.RIL_RADIO_TECHNOLOGY_UNKNOWN));
}
}
@@ -2409,9 +2417,10 @@
int[] enabledFeatures, int[] disabledFeatures) {
log("registrationFeatureCapabilityChanged :: serviceClass=" +
serviceClass);
- if (mListener != null) {
- mListener.onFeatureCapabilityChanged(serviceClass,
- enabledFeatures, disabledFeatures);
+
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onFeatureCapabilityChanged(serviceClass,
+ enabledFeatures, disabledFeatures));
}
}
@@ -2419,8 +2428,8 @@
public void voiceMessageCountUpdate(int count) {
log("voiceMessageCountUpdate :: count=" + count);
- if (mListener != null) {
- mListener.onVoiceMessageCountChanged(count);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onVoiceMessageCountChanged(count));
}
}
@@ -2428,8 +2437,8 @@
public void registrationAssociatedUriChanged(Uri[] uris) {
if (DBG) log("registrationAssociatedUriChanged ::");
- if (mListener != null) {
- mListener.registrationAssociatedUriChanged(uris);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.registrationAssociatedUriChanged(uris));
}
}
@@ -2438,8 +2447,9 @@
if (DBG) log("registrationChangeFailed :: targetAccessTech=" + targetAccessTech +
", imsReasonInfo=" + imsReasonInfo);
- if (mListener != null) {
- mListener.onRegistrationChangeFailed(targetAccessTech, imsReasonInfo);
+ synchronized (mRegistrationListeners) {
+ mRegistrationListeners.forEach(l -> l.onRegistrationChangeFailed(targetAccessTech,
+ imsReasonInfo));
}
}
}
