Never close underlying cursor if there are active users

Bug: 3249537
Bug: 3238216
Change-Id: I281f0b0d6319adaffd78fe8e4c3da6f126eb71da

src/com/android/email/provider/ContentCache.java

@@ -118,16 +118,20 @@ public final class ContentCache extends LinkedHashMap<String, Cursor> {
             mMap = new HashMap<T, Integer>();
         }
 
-        /*package*/ synchronized void subtract(T object) {
+        /*package*/ synchronized int subtract(T object) {
             Integer refCount = mMap.get(object);
+            int newCount;
             if (refCount == null || refCount.intValue() == 0) {
                 throw new IllegalStateException();
             }
             if (refCount > 1) {
-                mMap.put(object, refCount - 1);
+                newCount = refCount - 1;
+                mMap.put(object, newCount);
             } else {
+                newCount = 0;
                 mMap.remove(object);
             }
+            return newCount;
         }
 
         /*package*/ synchronized void add(T object) {
@@ -286,15 +290,16 @@ public final class ContentCache extends LinkedHashMap<String, Cursor> {
         }
 
         /**
-         * Close this cursor; if the cursor's cache no longer contains the cursor, we'll close the
+         * Close this cursor; if the cursor's cache no longer contains the underlying cursor, and
-         * underlying cursor.  In any event we'll remove the cursor from our set of active cursors
+         * there are no other users of that cursor, we'll close it here. In any event,
+         * we'll remove the cursor from our set of active cursors.
          */
         @Override
         public void close() {
-            if (!mCache.containsValue(mCursor)) {
+            int count = sActiveCursors.subtract(mCursor);
+            if ((count == 0) && !mCache.containsValue(mCursor)) {
                 super.close();
             }
-            sActiveCursors.subtract(mCursor);
             isClosed = true;
         }
 

tests/src/com/android/email/provider/ContentCacheTests.java

@@ -256,4 +256,43 @@ public class ContentCacheTests extends ProviderTestCase2<EmailProvider> {
         assertFalse(cursor2.isClosed());
         assertFalse(cursor3.isClosed());
     }
+    
+    public void testCloseCachedCursor() {
+        // Create a cache of size 2
+        ContentCache cache = new ContentCache("Name", SIMPLE_PROJECTION, 2);
+        // Random cursor; what's in it doesn't matter
+        Cursor underlyingCursor = getOneRowCursor();
+        Cursor cachedCursor1 = new CachedCursor(underlyingCursor, cache, "1");
+        Cursor cachedCursor2 = new CachedCursor(underlyingCursor, cache, "1");
+        assertEquals(2, ContentCache.sActiveCursors.getCount(underlyingCursor));
+        cachedCursor1.close();
+        assertTrue(cachedCursor1.isClosed());
+        // Underlying cursor should be open (still one cached cursor open)
+        assertFalse(underlyingCursor.isClosed());
+        cachedCursor2.close();
+        assertTrue(cachedCursor2.isClosed());
+        assertEquals(0, ContentCache.sActiveCursors.getCount(underlyingCursor));
+        // Underlying cursor should be closed (no cached cursors open)
+        assertTrue(underlyingCursor.isClosed());
+        
+        underlyingCursor = getOneRowCursor();
+        cache.put("2", underlyingCursor);
+        cachedCursor1 = new CachedCursor(underlyingCursor, cache, "2");
+        cachedCursor2 = new CachedCursor(underlyingCursor, cache, "2");
+        assertEquals(2, ContentCache.sActiveCursors.getCount(underlyingCursor));
+        cachedCursor1.close();
+        cachedCursor2.close();
+        assertEquals(0, ContentCache.sActiveCursors.getCount(underlyingCursor));
+        // Underlying cursor should still be open; it's in the cache
+        assertFalse(underlyingCursor.isClosed());
+        // Cache a new cursor
+        cachedCursor2 = new CachedCursor(underlyingCursor, cache, "2");
+        assertEquals(1, ContentCache.sActiveCursors.getCount(underlyingCursor));
+        // Remove "2" from the cache and close the cursor
+        cache.remove("2");
+        cachedCursor2.close();
+        // The underlying cursor should now be closed (not in the cache and no cached cursors)
+        assertEquals(0, ContentCache.sActiveCursors.getCount(underlyingCursor));
+        assertTrue(underlyingCursor.isClosed());
+    }
 }