From 79c9c9e365e30dc08452f7dd245559a431e06d0a Mon Sep 17 00:00:00 2001
From: Lorenzo Colitti <lorenzo@google.com>
Date: Fri, 18 Dec 2020 00:57:15 +0900
Subject: [PATCH] Allow RNDIS and NCM to be enabled at the same time.

Currently, UsbManager#setCurrentFunctions takes a bitmask of
functions to enable, but will only allow callers to enable one
function at a time.

Allow it to enable both RNDIS and NCM together as well. This is
because from the user's point of view the two functions are the
same (i.e., USB tethering).

Android has historically used RNDIS for tethering. NCM performs
better, but it doesn't work on older OSes like Windows 8.1.

Also change the import order to keep the linter happy.

Bug: 172793258
Test: added unit test for UsbManager.areSettableFunctions
Change-Id: I51b6752781528b09f8929e1fb3b1fa7e4ac36aae
---
 .../java/android/hardware/usb/UsbManager.java |  9 ++++-
 .../server/usb/UsbManagerNoPermTest.java      | 37 ++++++++++++++++++-
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/core/java/android/hardware/usb/UsbManager.java b/core/java/android/hardware/usb/UsbManager.java
index 0ef55f4a60f9c..3730790b92e08 100644
--- a/core/java/android/hardware/usb/UsbManager.java
+++ b/core/java/android/hardware/usb/UsbManager.java
@@ -954,7 +954,10 @@ public class UsbManager {
 
     /**
      * Returns whether the given functions are valid inputs to UsbManager.
-     * Currently the empty functions or any of MTP, PTP, RNDIS, MIDI are accepted.
+     * Currently the empty functions or any of MTP, PTP, RNDIS, MIDI, NCM are accepted.
+     *
+     * Only one function may be set at a time, except for RNDIS and NCM, which can be set together
+     * because from a user perspective they are the same function (tethering).
      *
      * @return Whether the mask is settable.
      *
@@ -962,7 +965,9 @@ public class UsbManager {
      */
     public static boolean areSettableFunctions(long functions) {
         return functions == FUNCTION_NONE
-                || ((~SETTABLE_FUNCTIONS & functions) == 0 && Long.bitCount(functions) == 1);
+                || ((~SETTABLE_FUNCTIONS & functions) == 0
+                        && ((Long.bitCount(functions) == 1)
+                                || (functions == (FUNCTION_RNDIS | FUNCTION_NCM))));
     }
 
     /**
diff --git a/tests/UsbTests/src/com/android/server/usb/UsbManagerNoPermTest.java b/tests/UsbTests/src/com/android/server/usb/UsbManagerNoPermTest.java
index a0fd9d40506b7..b8bd98ea3f219 100644
--- a/tests/UsbTests/src/com/android/server/usb/UsbManagerNoPermTest.java
+++ b/tests/UsbTests/src/com/android/server/usb/UsbManagerNoPermTest.java
@@ -16,6 +16,8 @@
 
 package com.android.server.usb;
 
+import static org.junit.Assert.assertEquals;
+
 import android.content.Context;
 import android.hardware.usb.UsbManager;
 
@@ -23,12 +25,12 @@ import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
+import com.android.server.usblib.UsbManagerTestLib;
+
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
-import com.android.server.usblib.UsbManagerTestLib;
-
 /**
  * Unit tests for {@link android.hardware.usb.UsbManager}.
  * Note: NOT claimed MANAGE_USB permission in Manifest
@@ -78,4 +80,35 @@ public class UsbManagerNoPermTest {
     public void testUsbApi_SetCurrentFunctions_OnSecurityException() throws Exception {
         mUsbManagerTestLib.testSetCurrentFunctionsEx(UsbManager.FUNCTION_NONE);
     }
+
+    public void assertSettableFunctions(boolean settable, long functions) {
+        assertEquals(
+                "areSettableFunctions(" + UsbManager.usbFunctionsToString(functions) + "):",
+                settable, UsbManager.areSettableFunctions(functions));
+    }
+
+    /**
+     * Tests the behaviour of the static areSettableFunctions method. This method performs no IPCs
+     * and requires no permissions.
+     */
+    @Test
+    public void testUsbManager_AreSettableFunctions() {
+        // NONE is settable.
+        assertSettableFunctions(true, UsbManager.FUNCTION_NONE);
+
+        // MTP, PTP, RNDIS, MIDI, NCM are all settable by themselves.
+        assertSettableFunctions(true, UsbManager.FUNCTION_MTP);
+        assertSettableFunctions(true, UsbManager.FUNCTION_PTP);
+        assertSettableFunctions(true, UsbManager.FUNCTION_RNDIS);
+        assertSettableFunctions(true, UsbManager.FUNCTION_MIDI);
+        assertSettableFunctions(true, UsbManager.FUNCTION_NCM);
+
+        // Setting two functions at the same time is not allowed...
+        assertSettableFunctions(false, UsbManager.FUNCTION_MTP | UsbManager.FUNCTION_PTP);
+        assertSettableFunctions(false, UsbManager.FUNCTION_PTP | UsbManager.FUNCTION_RNDIS);
+        assertSettableFunctions(false, UsbManager.FUNCTION_MIDI | UsbManager.FUNCTION_NCM);
+
+        // ... except in the special case of RNDIS and NCM.
+        assertSettableFunctions(true, UsbManager.FUNCTION_RNDIS | UsbManager.FUNCTION_NCM);
+    }
 }