Merge "Release the correct WiFi Multicast Wakelock"

3 files changed, 102 insertions, 7 deletions

diff --git a/service/java/com/android/server/wifi/WifiMulticastLockManager.java b/service/java/com/android/server/wifi/WifiMulticastLockManager.java
index 40b259d6c..9d85b969a 100644
--- a/service/java/com/android/server/wifi/WifiMulticastLockManager.java
+++ b/service/java/com/android/server/wifi/WifiMulticastLockManager.java
@@ -91,6 +91,10 @@ public class WifiMulticastLockManager {
             return mUid;
         }
 
+        public String getTag() {
+            return mTag;
+        }
+
         public String toString() {
             return "Multicaster{" + mTag + " uid=" + mUid  + "}";
         }
@@ -153,15 +157,16 @@ public class WifiMulticastLockManager {
     }
 
     /** Releases a multicast lock */
-    public void releaseLock() {
+    public void releaseLock(String tag) {
         int uid = Binder.getCallingUid();
         synchronized (mMulticasters) {
             mMulticastDisabled++;
             int size = mMulticasters.size();
             for (int i = size - 1; i >= 0; i--) {
                 Multicaster m = mMulticasters.get(i);
-                if ((m != null) && (m.getUid() == uid)) {
+                if ((m != null) && (m.getUid() == uid) && (m.getTag().equals(tag))) {
                     removeMulticasterLocked(i, uid);
+                    break;
                 }
             }
         }
diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index bf481edf7..a8d7c3772 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -2589,10 +2589,10 @@ public class WifiServiceImpl extends IWifiManager.Stub {
     }
 
     @Override
-    public void releaseMulticastLock() {
+    public void releaseMulticastLock(String tag) {
         enforceMulticastChangePermission();
         mLog.info("releaseMulticastLock uid=%").c(Binder.getCallingUid()).flush();
-        mWifiMulticastLockManager.releaseLock();
+        mWifiMulticastLockManager.releaseLock(tag);
     }
 
     @Override
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiMulticastLockManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiMulticastLockManagerTest.java
index 8939636af..1c560a146 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiMulticastLockManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiMulticastLockManagerTest.java
@@ -27,6 +27,7 @@ import com.android.internal.app.IBatteryStats;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
@@ -35,6 +36,9 @@ import org.mockito.MockitoAnnotations;
  */
 @SmallTest
 public class WifiMulticastLockManagerTest {
+    private static final String WL_1_TAG = "Wakelock-1";
+    private static final String WL_2_TAG = "Wakelock-2";
+
     @Mock WifiMulticastLockManager.FilterController mHandler;
     @Mock IBatteryStats mBatteryStats;
     WifiMulticastLockManager mManager;
@@ -64,7 +68,7 @@ public class WifiMulticastLockManagerTest {
     @Test
     public void oneLock() throws RemoteException {
         IBinder binder = mock(IBinder.class);
-        mManager.acquireLock(binder, "Test");
+        mManager.acquireLock(binder, WL_1_TAG);
         assertTrue(mManager.isMulticastEnabled());
         verify(mHandler).stopFilteringMulticastPackets();
         mManager.initializeFiltering();
@@ -72,9 +76,95 @@ public class WifiMulticastLockManagerTest {
         verify(mBatteryStats).noteWifiMulticastEnabled(anyInt());
         verify(mBatteryStats, times(0)).noteWifiMulticastDisabled(anyInt());
 
-        mManager.releaseLock();
+        mManager.releaseLock(WL_1_TAG);
         verify(mBatteryStats).noteWifiMulticastDisabled(anyInt());
         assertFalse(mManager.isMulticastEnabled());
     }
-}
 
+    /**
+     * Test behavior when one lock is aquired then released with the wrong tag.
+     */
+    @Test
+    public void oneLock_wrongName() throws RemoteException {
+        IBinder binder = mock(IBinder.class);
+        mManager.acquireLock(binder, WL_1_TAG);
+        assertTrue(mManager.isMulticastEnabled());
+        verify(mHandler).stopFilteringMulticastPackets();
+        mManager.initializeFiltering();
+        verify(mHandler, never()).startFilteringMulticastPackets();
+        verify(mBatteryStats).noteWifiMulticastEnabled(anyInt());
+        verify(mBatteryStats, never()).noteWifiMulticastDisabled(anyInt());
+
+        mManager.releaseLock(WL_2_TAG);
+        verify(mBatteryStats, never()).noteWifiMulticastDisabled(anyInt());
+        assertTrue(mManager.isMulticastEnabled());
+    }
+
+    /**
+     * Test behavior when multiple locks are aquired then released in nesting order.
+     */
+    @Test
+    public void multipleLocksInOrder() throws RemoteException {
+        IBinder binder = mock(IBinder.class);
+
+        InOrder inOrderHandler = inOrder(mHandler);
+        InOrder inOrderBatteryStats = inOrder(mBatteryStats);
+
+        mManager.acquireLock(binder, WL_1_TAG);
+        inOrderHandler.verify(mHandler).stopFilteringMulticastPackets();
+        inOrderBatteryStats.verify(mBatteryStats).noteWifiMulticastEnabled(anyInt());
+        assertTrue(mManager.isMulticastEnabled());
+
+        mManager.acquireLock(binder, WL_2_TAG);
+        inOrderHandler.verify(mHandler).stopFilteringMulticastPackets();
+        inOrderBatteryStats.verify(mBatteryStats).noteWifiMulticastEnabled(anyInt());
+        assertTrue(mManager.isMulticastEnabled());
+
+        mManager.initializeFiltering();
+        inOrderHandler.verify(mHandler, never()).startFilteringMulticastPackets();
+
+        mManager.releaseLock(WL_2_TAG);
+        inOrderHandler.verify(mHandler, never()).startFilteringMulticastPackets();
+        inOrderBatteryStats.verify(mBatteryStats).noteWifiMulticastDisabled(anyInt());
+        assertTrue(mManager.isMulticastEnabled());
+
+        mManager.releaseLock(WL_1_TAG);
+        inOrderHandler.verify(mHandler).startFilteringMulticastPackets();
+        inOrderBatteryStats.verify(mBatteryStats).noteWifiMulticastDisabled(anyInt());
+        assertFalse(mManager.isMulticastEnabled());
+    }
+
+    /**
+     * Test behavior when multiple locks are aquired then released out of nesting order.
+     */
+    @Test
+    public void multipleLocksOutOfOrder() throws RemoteException {
+        IBinder binder = mock(IBinder.class);
+
+        InOrder inOrderHandler = inOrder(mHandler);
+        InOrder inOrderBatteryStats = inOrder(mBatteryStats);
+
+        mManager.acquireLock(binder, WL_1_TAG);
+        inOrderHandler.verify(mHandler).stopFilteringMulticastPackets();
+        inOrderBatteryStats.verify(mBatteryStats).noteWifiMulticastEnabled(anyInt());
+        assertTrue(mManager.isMulticastEnabled());
+
+        mManager.acquireLock(binder, WL_2_TAG);
+        inOrderHandler.verify(mHandler).stopFilteringMulticastPackets();
+        inOrderBatteryStats.verify(mBatteryStats).noteWifiMulticastEnabled(anyInt());
+        assertTrue(mManager.isMulticastEnabled());
+
+        mManager.initializeFiltering();
+        inOrderHandler.verify(mHandler, never()).startFilteringMulticastPackets();
+
+        mManager.releaseLock(WL_1_TAG);
+        inOrderHandler.verify(mHandler, never()).startFilteringMulticastPackets();
+        inOrderBatteryStats.verify(mBatteryStats).noteWifiMulticastDisabled(anyInt());
+        assertTrue(mManager.isMulticastEnabled());
+
+        mManager.releaseLock(WL_2_TAG);
+        inOrderHandler.verify(mHandler).startFilteringMulticastPackets();
+        inOrderBatteryStats.verify(mBatteryStats).noteWifiMulticastDisabled(anyInt());
+        assertFalse(mManager.isMulticastEnabled());
+    }
+}