commit 4ae57f958743a8e6e2d7387628cf95aa6b3aaf95
author Brad Ebinger <breadley@google.com> Wed Jan 09 16:51:30 2019 -0800
committer Brad Ebinger <breadley@google.com> Thu Jan 17 19:35:02 2019 +0000
tree 68c505a4175a1754148715d5bfcab605eee66d05
parent 74b643db64ffb2629158e9136978439abdfc7561

Use Better Exception handling in IMS APIs

Throw more expressive expressions when something bad happens.

Bug: 122742715
Bug: 122480210
Test: atest FrameworksTelephonyTests
Change-Id: I93569395453a07203c84a47b449acf4dbe7d0ddf

src/com/android/phone/PhoneInterfaceManager.java

diff --git a/src/com/android/phone/PhoneInterfaceManager.java b/src/com/android/phone/PhoneInterfaceManager.java
index 7b7eb0f..8a4c3d3 100755
--- a/src/com/android/phone/PhoneInterfaceManager.java
+++ b/src/com/android/phone/PhoneInterfaceManager.java
@@ -2721,9 +2721,9 @@
     public void registerImsRegistrationCallback(int subId, IImsRegistrationCallback c)
             throws RemoteException {
         enforceReadPrivilegedPermission("registerImsRegistrationCallback");
-        // TODO: Refactor to remove ImsManager dependence and query through ImsPhone directly.
         final long token = Binder.clearCallingIdentity();
         try {
+            // TODO: Refactor to remove ImsManager dependence and query through ImsPhone directly.
             ImsManager.getInstance(mApp, getSlotIndexOrException(subId))
                     .addRegistrationCallbackForSubscription(c, subId);
         } finally {
@@ -2734,10 +2734,21 @@
     @Override
     public void unregisterImsRegistrationCallback(int subId, IImsRegistrationCallback c) {
         enforceReadPrivilegedPermission("unregisterImsRegistrationCallback");
-        // TODO: Refactor to remove ImsManager dependence and query through ImsPhone directly.
-        Binder.withCleanCallingIdentity(() ->
+        if (!SubscriptionManager.isValidSubscriptionId(subId)) {
+            throw new IllegalArgumentException("Invalid Subscription ID: " + subId);
+        }
+        Binder.withCleanCallingIdentity(() -> {
+            try {
+                // TODO: Refactor to remove ImsManager dependence and query through ImsPhone.
                 ImsManager.getInstance(mApp, getSlotIndexOrException(subId))
-                        .removeRegistrationCallbackForSubscription(c, subId));
+                        .removeRegistrationCallbackForSubscription(c, subId);
+            } catch (IllegalArgumentException e) {
+                Log.i(LOG_TAG, "unregisterImsRegistrationCallback: " + subId
+                        + "is inactive, ignoring unregister.");
+                // If the subscription is no longer active, just return, since the callback
+                // will already have been removed internally.
+            }
+        });
     }
 
     @Override
@@ -2757,10 +2768,22 @@
     @Override
     public void unregisterMmTelCapabilityCallback(int subId, IImsCapabilityCallback c) {
         enforceReadPrivilegedPermission("unregisterMmTelCapabilityCallback");
-        // TODO: Refactor to remove ImsManager dependence and query through ImsPhone directly.
-        Binder.withCleanCallingIdentity(() ->
+
+        if (!SubscriptionManager.isValidSubscriptionId(subId)) {
+            throw new IllegalArgumentException("Invalid Subscription ID: " + subId);
+        }
+        Binder.withCleanCallingIdentity(() -> {
+            try {
+                // TODO: Refactor to remove ImsManager dependence and query through ImsPhone.
                 ImsManager.getInstance(mApp, getSlotIndexOrException(subId))
-                        .removeCapabilitiesCallbackForSubscription(c, subId));
+                        .removeCapabilitiesCallbackForSubscription(c, subId);
+            } catch (IllegalArgumentException e) {
+                Log.i(LOG_TAG, "unregisterMmTelCapabilityCallback: " + subId
+                        + "is inactive, ignoring unregister.");
+                // If the subscription is no longer active, just return, since the callback
+                // will already have been removed internally.
+            }
+        });
     }
 
     @Override
@@ -3000,9 +3023,7 @@
         try {
             // TODO: Refactor to remove ImsManager dependence and query through ImsPhone directly.
             ImsManager.getInstance(mApp, getSlotIndexOrException(subId))
-                    .getConfigInterface().addConfigCallback(callback);
-        } catch (ImsException e) {
-            throw new IllegalArgumentException(e.getMessage());
+                    .addProvisioningCallbackForSubscription(callback, subId);
         } finally {
             Binder.restoreCallingIdentity(identity);
         }
@@ -3012,12 +3033,18 @@
     public void unregisterImsProvisioningChangedCallback(int subId, IImsConfigCallback callback) {
         enforceReadPrivilegedPermission("unregisterImsProvisioningChangedCallback");
         final long identity = Binder.clearCallingIdentity();
+        if (!SubscriptionManager.isValidSubscriptionId(subId)) {
+            throw new IllegalArgumentException("Invalid Subscription ID: " + subId);
+        }
         try {
             // TODO: Refactor to remove ImsManager dependence and query through ImsPhone directly.
             ImsManager.getInstance(mApp, getSlotIndexOrException(subId))
-                    .getConfigInterface().removeConfigCallback(callback);
-        } catch (ImsException e) {
-            throw new IllegalArgumentException(e.getMessage());
+                    .removeProvisioningCallbackForSubscription(callback, subId);
+        } catch (IllegalArgumentException e) {
+            Log.i(LOG_TAG, "unregisterImsProvisioningChangedCallback: " + subId
+                    + "is inactive, ignoring unregister.");
+            // If the subscription is no longer active, just return, since the callback will already
+            // have been removed internally.
         } finally {
             Binder.restoreCallingIdentity(identity);
         }