From 7ae966c3f942691b94e3faa51acca1148bd24945 Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Thu, 30 Jun 2022 14:46:09 -0600
Subject: [PATCH] Handle null callbacks, add missing breaks.

The ISystemDataTransferCallback argument can be null, so quietly
handle that case.  Also fix bug where we forgot "break" statements
when processing responses.

Finally, always detach transport before closing streams to avoid
misleading warning logs being emitted from system.

Bug: 237030169
Test: manual
Change-Id: Id6d2d93a1c6872152112f877a2b92f4656277803
---
 .../companion/CompanionDeviceManager.java     | 10 +++---
 .../SystemDataTransferProcessor.java          | 36 ++++++++++++-------
 .../transport/CompanionTransportManager.java  |  8 +++--
 3 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/core/java/android/companion/CompanionDeviceManager.java b/core/java/android/companion/CompanionDeviceManager.java
index 633b734d4adc6..8ffc6a28bef3e 100644
--- a/core/java/android/companion/CompanionDeviceManager.java
+++ b/core/java/android/companion/CompanionDeviceManager.java
@@ -1161,17 +1161,17 @@ public final class CompanionDeviceManager {
         public void stop() {
             mStopped = true;
 
-            IoUtils.closeQuietly(mRemoteIn);
-            IoUtils.closeQuietly(mRemoteOut);
-            IoUtils.closeQuietly(mLocalIn);
-            IoUtils.closeQuietly(mLocalOut);
-
             try {
                 mService.detachSystemDataTransport(mContext.getPackageName(),
                         mContext.getUserId(), mAssociationId);
             } catch (RemoteException e) {
                 Log.w(LOG_TAG, "Failed to detach transport", e);
             }
+
+            IoUtils.closeQuietly(mRemoteIn);
+            IoUtils.closeQuietly(mRemoteOut);
+            IoUtils.closeQuietly(mLocalIn);
+            IoUtils.closeQuietly(mLocalOut);
         }
 
         /**
diff --git a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java
index f0a0492e3806f..88ebb971f292d 100644
--- a/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java
+++ b/services/companion/java/com/android/server/companion/datatransfer/SystemDataTransferProcessor.java
@@ -25,6 +25,7 @@ import static android.content.ComponentName.createRelative;
 import static com.android.server.companion.Utils.prepareForIpc;
 
 import android.annotation.NonNull;
+import android.annotation.Nullable;
 import android.annotation.UserIdInt;
 import android.app.PendingIntent;
 import android.companion.AssociationInfo;
@@ -191,20 +192,9 @@ public class SystemDataTransferProcessor {
             // TODO: refactor to work with streams of data
             mPermissionControllerManager.getRuntimePermissionBackup(UserHandle.of(userId),
                     mExecutor, backup -> {
-                        Future<?> result = mTransportManager
+                        Future<?> future = mTransportManager
                                 .requestPermissionRestore(associationId, backup);
-                        try {
-                            result.get(15, TimeUnit.SECONDS);
-                            try {
-                                callback.onResult();
-                            } catch (RemoteException ignored) {
-                            }
-                        } catch (Exception e) {
-                            try {
-                                callback.onError(e.getMessage());
-                            } catch (RemoteException ignored) {
-                            }
-                        }
+                        translateFutureToCallback(future, callback);
                     });
         } finally {
             Binder.restoreCallingIdentity(callingIdentityToken);
@@ -225,6 +215,26 @@ public class SystemDataTransferProcessor {
         }
     }
 
+    private static void translateFutureToCallback(@NonNull Future<?> future,
+            @Nullable ISystemDataTransferCallback callback) {
+        try {
+            future.get(15, TimeUnit.SECONDS);
+            try {
+                if (callback != null) {
+                    callback.onResult();
+                }
+            } catch (RemoteException ignored) {
+            }
+        } catch (Exception e) {
+            try {
+                if (callback != null) {
+                    callback.onError(e.getMessage());
+                }
+            } catch (RemoteException ignored) {
+            }
+        }
+    }
+
     private final ResultReceiver mOnSystemDataTransferRequestConfirmationReceiver =
             new ResultReceiver(Handler.getMain()) {
                 @Override
diff --git a/services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java b/services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java
index 4a990095cd8f1..77d51ea9a5ac3 100644
--- a/services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java
+++ b/services/companion/java/com/android/server/companion/transport/CompanionTransportManager.java
@@ -260,7 +260,7 @@ public class CompanionTransportManager {
             } else if (isResponse(message)) {
                 processResponse(message, sequence, data);
             } else {
-                Slog.w(TAG, "Unknown message " + Integer.toHexString(message));
+                Slog.w(TAG, "Unknown message 0x" + Integer.toHexString(message));
             }
         }
 
@@ -288,7 +288,7 @@ public class CompanionTransportManager {
                     break;
                 }
                 default: {
-                    Slog.w(TAG, "Unknown request " + Integer.toHexString(message));
+                    Slog.w(TAG, "Unknown request 0x" + Integer.toHexString(message));
                     sendMessage(MESSAGE_RESPONSE_FAILURE, sequence, EmptyArray.BYTE);
                     break;
                 }
@@ -308,12 +308,14 @@ public class CompanionTransportManager {
             switch (message) {
                 case MESSAGE_RESPONSE_SUCCESS: {
                     future.complete(data);
+                    break;
                 }
                 case MESSAGE_RESPONSE_FAILURE: {
                     future.completeExceptionally(new RuntimeException("Remote failure"));
+                    break;
                 }
                 default: {
-                    Slog.w(TAG, "Ignoring unknown response " + Integer.toHexString(message));
+                    Slog.w(TAG, "Ignoring unknown response 0x" + Integer.toHexString(message));
                 }
             }
         }