Cleanup video provider binder code.

1. Ensure we always unlink for death from the VideoCallImpl.
2. Ensure we compare the binders in the Call instance properly so that
we are not always replacing the VideoCallImpl used.
3. Ensure we cleanup the VideoCallImpl when its being removed.

Test: Manual testing of video calls for regressions.
Bug: 121156974
Change-Id: I440e8fb4ab9a5051ee02358933c495de736e2bd0

3 files changed, 42 insertions, 6 deletions

diff --git a/telecomm/java/android/telecom/Call.java b/telecomm/java/android/telecom/Call.java
index 60290e3b785d..3b36364106a3 100644
--- a/telecomm/java/android/telecom/Call.java
+++ b/telecomm/java/android/telecom/Call.java
@@ -25,8 +25,11 @@ import android.net.Uri;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.Handler;
+import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
 
+import com.android.internal.telecom.IVideoProvider;
+
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.OutputStreamWriter;
@@ -2130,13 +2133,22 @@ public final class Call {
             cannedTextResponsesChanged = true;
         }
 
-        VideoCallImpl newVideoCallImpl = parcelableCall.getVideoCallImpl(mCallingPackage,
-                mTargetSdkVersion);
-        boolean videoCallChanged = parcelableCall.isVideoCallProviderChanged() &&
-                !Objects.equals(mVideoCallImpl, newVideoCallImpl);
+        IVideoProvider previousVideoProvider = mVideoCallImpl == null ? null :
+                mVideoCallImpl.getVideoProvider();
+        IVideoProvider newVideoProvider = parcelableCall.getVideoProvider();
+
+        // parcelableCall.isVideoCallProviderChanged is only true when we have a video provider
+        // specified; so we should check if the actual IVideoProvider changes as well.
+        boolean videoCallChanged = parcelableCall.isVideoCallProviderChanged()
+                && !Objects.equals(previousVideoProvider, newVideoProvider);
         if (videoCallChanged) {
-            mVideoCallImpl = newVideoCallImpl;
+            if (mVideoCallImpl != null) {
+                mVideoCallImpl.destroy();
+            }
+            mVideoCallImpl = parcelableCall.isVideoCallProviderChanged() ?
+                    parcelableCall.getVideoCallImpl(mCallingPackage, mTargetSdkVersion) : null;
         }
+
         if (mVideoCallImpl != null) {
             mVideoCallImpl.setVideoState(getDetails().getVideoState());
         }
diff --git a/telecomm/java/android/telecom/ParcelableCall.java b/telecomm/java/android/telecom/ParcelableCall.java
index aa50991b118c..fdc324308d7a 100644
--- a/telecomm/java/android/telecom/ParcelableCall.java
+++ b/telecomm/java/android/telecom/ParcelableCall.java
@@ -21,6 +21,7 @@ import android.annotation.UnsupportedAppUsage;
 import android.net.Uri;
 import android.os.Build;
 import android.os.Bundle;
+import android.os.IBinder;
 import android.os.Parcel;
 import android.os.Parcelable;
 import android.os.RemoteException;
@@ -225,6 +226,10 @@ public final class ParcelableCall implements Parcelable {
         return mVideoCall;
     }
 
+    public IVideoProvider getVideoProvider() {
+        return mVideoCallProvider;
+    }
+
     public boolean getIsRttCallChanged() {
         return mIsRttCallChanged;
     }
diff --git a/telecomm/java/android/telecom/VideoCallImpl.java b/telecomm/java/android/telecom/VideoCallImpl.java
index cb74012e2b81..4a1aa0a8ffa4 100644
--- a/telecomm/java/android/telecom/VideoCallImpl.java
+++ b/telecomm/java/android/telecom/VideoCallImpl.java
@@ -32,6 +32,8 @@ import com.android.internal.os.SomeArgs;
 import com.android.internal.telecom.IVideoCallback;
 import com.android.internal.telecom.IVideoProvider;
 
+import java.util.NoSuchElementException;
+
 /**
  * Implementation of a Video Call, which allows InCallUi to communicate commands to the underlying
  * {@link Connection.VideoProvider}, and direct callbacks from the
@@ -53,7 +55,11 @@ public class VideoCallImpl extends VideoCall {
     private IBinder.DeathRecipient mDeathRecipient = new IBinder.DeathRecipient() {
         @Override
         public void binderDied() {
-            mVideoProvider.asBinder().unlinkToDeath(this, 0);
+            try {
+                mVideoProvider.asBinder().unlinkToDeath(this, 0);
+            } catch (NoSuchElementException nse) {
+                // Already unlinked in destroy below.
+            }
         }
     };
 
@@ -222,6 +228,11 @@ public class VideoCallImpl extends VideoCall {
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 127403196)
     public void destroy() {
         unregisterCallback(mCallback);
+        try {
+            mVideoProvider.asBinder().unlinkToDeath(mDeathRecipient, 0);
+        } catch (NoSuchElementException nse) {
+            // Already unlinked in binderDied above.
+        }
     }
 
     /** {@inheritDoc} */
@@ -353,4 +364,12 @@ public class VideoCallImpl extends VideoCall {
     public void setVideoState(int videoState) {
         mVideoState = videoState;
     }
+
+    /**
+     * Get the video provider binder.
+     * @return the video provider binder.
+     */
+    public IVideoProvider getVideoProvider() {
+        return mVideoProvider;
+    }
 }