Merge "ImsServiceController should properly unbind when remote crashes" am: 63054caff6 (3ffa195e) · Commits · e / os / android_frameworks_opt_telephony

src/java/com/android/internal/telephony/ims/ImsServiceController.java

+36 −37

Original line number	Original line	Diff line number	Diff line
	@@ -71,8 +71,8 @@ public class ImsServiceController {

	@Override		@Override
	public void onServiceConnected(ComponentName name, IBinder service) {		public void onServiceConnected(ComponentName name, IBinder service) {
	mBackoff.stop();
	synchronized (mLock) {		synchronized (mLock) {
			mBackoff.stop();
	mIsBound = true;		mIsBound = true;
	mIsBinding = false;		mIsBinding = false;
	try {		try {
	@@ -88,8 +88,10 @@ public class ImsServiceController {
	} catch (RemoteException e) {		} catch (RemoteException e) {
	mIsBound = false;		mIsBound = false;
	mIsBinding = false;		mIsBinding = false;
	// Remote exception means that the binder already died.		// RemoteException means that the process holding the binder died or something
			// unexpected happened... try a full rebind.
	cleanupConnection();		cleanupConnection();
			unbindService();
	startDelayedRebindToService();		startDelayedRebindToService();
	mLocalLog.log("onConnected exception=" + e.getMessage() + ", retry in "		mLocalLog.log("onConnected exception=" + e.getMessage() + ", retry in "
	+ mBackoff.getCurrentDelay() + " mS");		+ mBackoff.getCurrentDelay() + " mS");
	@@ -103,8 +105,8 @@ public class ImsServiceController {
	public void onServiceDisconnected(ComponentName name) {		public void onServiceDisconnected(ComponentName name) {
	synchronized (mLock) {		synchronized (mLock) {
	mIsBinding = false;		mIsBinding = false;
	}
	cleanupConnection();		cleanupConnection();
			}
	mLocalLog.log("onServiceDisconnected");		mLocalLog.log("onServiceDisconnected");
	Log.w(LOG_TAG, "ImsService(" + name + "): onServiceDisconnected. Waiting...");		Log.w(LOG_TAG, "ImsService(" + name + "): onServiceDisconnected. Waiting...");
	// Service disconnected, but we are still technically bound. Waiting for reconnect.		// Service disconnected, but we are still technically bound. Waiting for reconnect.
	@@ -115,14 +117,12 @@ public class ImsServiceController {
	synchronized (mLock) {		synchronized (mLock) {
	mIsBinding = false;		mIsBinding = false;
	mIsBound = false;		mIsBound = false;
	}
	if (mImsServiceConnection != null) {
	// according to the docs, we should fully unbind before rebinding again.		// according to the docs, we should fully unbind before rebinding again.
	mContext.unbindService(mImsServiceConnection);
	}
	cleanupConnection();		cleanupConnection();
	Log.w(LOG_TAG, "ImsService(" + name + "): onBindingDied. Starting rebind...");		unbindService();
	startDelayedRebindToService();		startDelayedRebindToService();
			}
			Log.w(LOG_TAG, "ImsService(" + name + "): onBindingDied. Starting rebind...");
	mLocalLog.log("onBindingDied, retrying in " + mBackoff.getCurrentDelay() + " mS");		mLocalLog.log("onBindingDied, retrying in " + mBackoff.getCurrentDelay() + " mS");
	}		}

	@@ -132,19 +132,23 @@ public class ImsServiceController {
	mLocalLog.log("onNullBinding");		mLocalLog.log("onNullBinding");
	synchronized (mLock) {		synchronized (mLock) {
	mIsBinding = false;		mIsBinding = false;
	mIsBound = false;		// Service connection exists, so we are bound but the binder is null. Wait for
	}		// ImsResolver to trigger the unbind here.
			mIsBound = true;
	cleanupConnection();		cleanupConnection();
			}
	if (mCallbacks != null) {		if (mCallbacks != null) {
	// Will trigger an unbind.		// Will trigger an unbind.
	mCallbacks.imsServiceBindPermanentError(getComponentName());		mCallbacks.imsServiceBindPermanentError(getComponentName());
	}		}
	}		}

	// Does not clear features, just removes all active features.		// Does not clear feature configuration, just cleans up the active callbacks and
			// invalidates remote FeatureConnections.
			// This should only be called when locked
	private void cleanupConnection() {		private void cleanupConnection() {
	cleanupAllFeatures();		cleanupAllFeatures();
	cleanUpService();		setServiceController(null);
	}		}
	}		}

	@@ -424,18 +428,13 @@ public class ImsServiceController {
	public void unbind() throws RemoteException {		public void unbind() throws RemoteException {
	synchronized (mLock) {		synchronized (mLock) {
	mBackoff.stop();		mBackoff.stop();
	if (mImsServiceConnection == null) {
	return;
	}
	// Clean up all features		// Clean up all features
	changeImsServiceFeatures(new HashSet<>());		changeImsServiceFeatures(new HashSet<>());
	removeImsServiceFeatureCallbacks();		removeImsServiceFeatureCallbacks();
	Log.i(LOG_TAG, "Unbinding ImsService: " + mComponentName);
	mLocalLog.log("unbinding");
	mContext.unbindService(mImsServiceConnection);
	mIsBound = false;		mIsBound = false;
	mIsBinding = false;		mIsBinding = false;
	cleanUpService();		setServiceController(null);
			unbindService();
	}		}
	}		}

	@@ -622,15 +621,6 @@ public class ImsServiceController {
	mIImsServiceController = IImsServiceController.Stub.asInterface(serviceController);		mIImsServiceController = IImsServiceController.Stub.asInterface(serviceController);
	}		}

	/**
	* @return true if the controller is currently bound.
	*/
	public boolean isBound() {
	synchronized (mLock) {
	return mIsBound;
	}
	}

	/**		/**
	* Check to see if the service controller is available, overridden for compat versions,		* Check to see if the service controller is available, overridden for compat versions,
	* @return true if available, false otherwise;		* @return true if available, false otherwise;
	@@ -649,6 +639,22 @@ public class ImsServiceController {
	mBackoff.start();		mBackoff.start();
	}		}

			private void unbindService() {
			synchronized (mLock) {
			if (mImsServiceConnection != null) {
			Log.i(LOG_TAG, "Unbinding ImsService: " + mComponentName);
			mLocalLog.log("unbinding: " + mComponentName);
			mContext.unbindService(mImsServiceConnection);
			mImsServiceConnection = null;
			} else {
			Log.i(LOG_TAG, "unbindService called on already unbound ImsService: "
			+ mComponentName);
			mLocalLog.log("Note: unbindService called with no ServiceConnection on "
			+ mComponentName);
			}
			}
			}

	// Grant runtime permissions to ImsService. PackageManager ensures that the ImsService is		// Grant runtime permissions to ImsService. PackageManager ensures that the ImsService is
	// system/signed before granting permissions.		// system/signed before granting permissions.
	private void grantPermissionsToService() {		private void grantPermissionsToService() {
	@@ -766,8 +772,8 @@ public class ImsServiceController {
	// Don't update ImsService for emergency MMTEL feature.		// Don't update ImsService for emergency MMTEL feature.
	Log.i(LOG_TAG, "doesn't support emergency calling on slot " + featurePair.slotId);		Log.i(LOG_TAG, "doesn't support emergency calling on slot " + featurePair.slotId);
	}		}
	// Send callback to ImsServiceProxy to change supported ImsFeatures		// Send callback to FeatureConnection to change supported ImsFeatures
	// Ensure that ImsServiceProxy callback occurs after ImsResolver callback. If an		// Ensure that FeatureConnection callback occurs after ImsResolver callback. If an
	// ImsManager requests the ImsService while it is being removed in ImsResolver, this		// ImsManager requests the ImsService while it is being removed in ImsResolver, this
	// callback will clean it up after.		// callback will clean it up after.
	sendImsFeatureRemovedCallback(featurePair.slotId, featurePair.featureType);		sendImsFeatureRemovedCallback(featurePair.slotId, featurePair.featureType);
	@@ -828,13 +834,6 @@ public class ImsServiceController {
	}		}
	}		}

	private void cleanUpService() {
	synchronized (mLock) {
	mImsServiceConnection = null;
	setServiceController(null);
	}
	}

	@Override		@Override
	public String toString() {		public String toString() {
	synchronized (mLock) {		synchronized (mLock) {

tests/telephonytests/src/com/android/internal/telephony/ims/ImsServiceControllerTest.java

+39 −0

Original line number	Original line	Diff line number	Diff line
	@@ -281,6 +281,44 @@ public class ImsServiceControllerTest extends ImsTestBase {
	verify(mMockProxyCallbacks).imsFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_RCS));		verify(mMockProxyCallbacks).imsFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_RCS));
	}		}

			/**
			* Tests that when unbind is called while the ImsService is disconnected, we still handle
			* unbinding to the service correctly.
			*/
			@SmallTest
			@Test
			public void testBindServiceAndConnectedDisconnectedUnbind() throws RemoteException {
			HashSet<ImsFeatureConfiguration.FeatureSlotPair> testFeatures = new HashSet<>();
			testFeatures.add(new ImsFeatureConfiguration.FeatureSlotPair(SLOT_0,
			ImsFeature.FEATURE_MMTEL));
			testFeatures.add(new ImsFeatureConfiguration.FeatureSlotPair(SLOT_0,
			ImsFeature.FEATURE_RCS));
			ServiceConnection conn = bindAndConnectService(testFeatures);

			conn.onServiceDisconnected(mTestComponentName);

			verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_MMTEL),
			eq(mTestImsServiceController));
			verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_RCS),
			eq(mTestImsServiceController));
			verify(mMockProxyCallbacks).imsFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_MMTEL));
			verify(mMockProxyCallbacks).imsFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_RCS));

			mTestImsServiceController.unbind();
			verify(mMockContext).unbindService(eq(conn));

			// Even though we unbound, this was already sent after service disconnected, so we shouldn't
			// see it again
			verify(mMockCallbacks, times(1)).imsServiceFeatureRemoved(eq(SLOT_0),
			eq(ImsFeature.FEATURE_MMTEL), eq(mTestImsServiceController));
			verify(mMockCallbacks, times(1)).imsServiceFeatureRemoved(eq(SLOT_0),
			eq(ImsFeature.FEATURE_RCS), eq(mTestImsServiceController));
			verify(mMockProxyCallbacks, times(1)).imsFeatureRemoved(eq(SLOT_0),
			eq(ImsFeature.FEATURE_MMTEL));
			verify(mMockProxyCallbacks, times(1)).imsFeatureRemoved(eq(SLOT_0),
			eq(ImsFeature.FEATURE_RCS));
			}

	/**		/**
	* Tests ImsServiceController callbacks are properly called when an ImsService is bound and		* Tests ImsServiceController callbacks are properly called when an ImsService is bound and
	* subsequently unbound.		* subsequently unbound.
	@@ -325,6 +363,7 @@ public class ImsServiceControllerTest extends ImsTestBase {

	conn.onBindingDied(null /null/);		conn.onBindingDied(null /null/);

			verify(mMockContext).unbindService(eq(conn));
	verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_MMTEL),		verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_MMTEL),
	eq(mTestImsServiceController));		eq(mTestImsServiceController));
	verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_RCS),		verify(mMockCallbacks).imsServiceFeatureRemoved(eq(SLOT_0), eq(ImsFeature.FEATURE_RCS),