Make DocumentCursor opt-out of having a self-observer

A content observer is registered by default when setting a notification
uri for a Cursor, in order to make the Cursor correctly notify listeners
of all changes to its URI, not just the ones made locally.

This is not required for DocumentCursor, because it already has a
separate mechanism for watching for all changes made to the data backed
by the cursor.

This avoids DocumentProviders having to call into system_server to
answer queries about directory trees, which can otherwise add up to
significant amounts of time for large directory trees. In my tests,
this improves the performance of iterating through a directory by
roughly 20%. This number is likely to be higher on non-test devices,
that probably see more binder contention, and will also depend on the
structure of the file tree.

Bug: 130276310
Test: SAF test app
Change-Id: I386363b0608c420e9847caf6fbf6686641c955e2

diff --git a/core/java/android/database/AbstractCursor.java b/core/java/android/database/AbstractCursor.java
index b44458a..cc5d3b1 100644 (file)
--- a/core/java/android/database/AbstractCursor.java
+++ b/core/java/android/database/AbstractCursor.java
@@ -419,26 +419,36 @@ public abstract class AbstractCursor implements CrossProcessCursor {
         Preconditions.checkNotNull(cr);
         Preconditions.checkNotNull(notifyUris);
 
-        setNotificationUris(cr, notifyUris, cr.getUserId());
+        setNotificationUris(cr, notifyUris, cr.getUserId(), true);
     }
 
-    /** @hide - set the notification uri but with an observer for a particular user's view */
-    public void setNotificationUris(ContentResolver cr, List<Uri> notifyUris, int userHandle) {
+    /**
+     * Set the notification uri but with an observer for a particular user's view. Also allows
+     * disabling the use of a self observer, which is sensible if either
+     * a) the cursor's owner calls {@link #onChange(boolean)} whenever the content changes, or
+     * b) the cursor is known not to have any content observers.
+     * @hide
+     */
+    public void setNotificationUris(ContentResolver cr, List<Uri> notifyUris, int userHandle,
+            boolean registerSelfObserver) {
         synchronized (mSelfObserverLock) {
             mNotifyUris = notifyUris;
             mNotifyUri = mNotifyUris.get(0);
             mContentResolver = cr;
             if (mSelfObserver != null) {
                 mContentResolver.unregisterContentObserver(mSelfObserver);
+                mSelfObserverRegistered = false;
             }
-            mSelfObserver = new SelfContentObserver(this);
-            final int size = mNotifyUris.size();
-            for (int i = 0; i < size; ++i) {
-                final Uri notifyUri = mNotifyUris.get(i);
-                mContentResolver.registerContentObserver(
-                        notifyUri, true, mSelfObserver, userHandle);
+            if (registerSelfObserver) {
+                mSelfObserver = new SelfContentObserver(this);
+                final int size = mNotifyUris.size();
+                for (int i = 0; i < size; ++i) {
+                    final Uri notifyUri = mNotifyUris.get(i);
+                    mContentResolver.registerContentObserver(
+                            notifyUri, true, mSelfObserver, userHandle);
+                }
+                mSelfObserverRegistered = true;
             }
-            mSelfObserverRegistered = true;
         }
     }
 

diff --git a/core/java/com/android/internal/content/FileSystemProvider.java b/core/java/com/android/internal/content/FileSystemProvider.java
index 9e8bd64..cc2caca 100644 (file)
--- a/core/java/com/android/internal/content/FileSystemProvider.java
+++ b/core/java/com/android/internal/content/FileSystemProvider.java
@@ -60,9 +60,11 @@ import java.nio.file.FileVisitor;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.attribute.BasicFileAttributes;
+import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 /**
  * A helper class for {@link android.provider.DocumentsProvider} to perform file operations on local
@@ -613,28 +615,28 @@ public abstract class FileSystemProvider extends DocumentsProvider {
         return projection == null ? mDefaultProjection : projection;
     }
 
-    private void startObserving(File file, Uri notifyUri) {
+    private void startObserving(File file, Uri notifyUri, DirectoryCursor cursor) {
         synchronized (mObservers) {
             DirectoryObserver observer = mObservers.get(file);
             if (observer == null) {
-                observer = new DirectoryObserver(
-                        file, getContext().getContentResolver(), notifyUri);
+                observer =
+                        new DirectoryObserver(file, getContext().getContentResolver(), notifyUri);
                 observer.startWatching();
                 mObservers.put(file, observer);
             }
-            observer.mRefCount++;
+            observer.mCursors.add(cursor);
 
             if (LOG_INOTIFY) Log.d(TAG, "after start: " + observer);
         }
     }
 
-    private void stopObserving(File file) {
+    private void stopObserving(File file, DirectoryCursor cursor) {
         synchronized (mObservers) {
             DirectoryObserver observer = mObservers.get(file);
             if (observer == null) return;
 
-            observer.mRefCount--;
-            if (observer.mRefCount == 0) {
+            observer.mCursors.remove(cursor);
+            if (observer.mCursors.size() == 0) {
                 mObservers.remove(file);
                 observer.stopWatching();
             }
@@ -650,27 +652,31 @@ public abstract class FileSystemProvider extends DocumentsProvider {
         private final File mFile;
         private final ContentResolver mResolver;
         private final Uri mNotifyUri;
+        private final CopyOnWriteArrayList<DirectoryCursor> mCursors;
 
-        private int mRefCount = 0;
-
-        public DirectoryObserver(File file, ContentResolver resolver, Uri notifyUri) {
+        DirectoryObserver(File file, ContentResolver resolver, Uri notifyUri) {
             super(file.getAbsolutePath(), NOTIFY_EVENTS);
             mFile = file;
             mResolver = resolver;
             mNotifyUri = notifyUri;
+            mCursors = new CopyOnWriteArrayList<>();
         }
 
         @Override
         public void onEvent(int event, String path) {
             if ((event & NOTIFY_EVENTS) != 0) {
                 if (LOG_INOTIFY) Log.d(TAG, "onEvent() " + event + " at " + path);
+                for (DirectoryCursor cursor : mCursors) {
+                    cursor.notifyChanged();
+                }
                 mResolver.notifyChange(mNotifyUri, null, false);
             }
         }
 
         @Override
         public String toString() {
-            return "DirectoryObserver{file=" + mFile.getAbsolutePath() + ", ref=" + mRefCount + "}";
+            String filePath = mFile.getAbsolutePath();
+            return "DirectoryObserver{file=" + filePath + ", ref=" + mCursors.size() + "}";
         }
     }
 
@@ -681,16 +687,22 @@ public abstract class FileSystemProvider extends DocumentsProvider {
             super(columnNames);
 
             final Uri notifyUri = buildNotificationUri(docId);
-            setNotificationUri(getContext().getContentResolver(), notifyUri);
+            boolean registerSelfObserver = false; // Our FileObserver sees all relevant changes.
+            setNotificationUris(getContext().getContentResolver(), Arrays.asList(notifyUri),
+                    getContext().getContentResolver().getUserId(), registerSelfObserver);
 
             mFile = file;
-            startObserving(mFile, notifyUri);
+            startObserving(mFile, notifyUri, this);
+        }
+
+        public void notifyChanged() {
+            onChange(false);
         }
 
         @Override
         public void close() {
             super.close();
-            stopObserving(mFile);
+            stopObserving(mFile, this);
         }
     }
 }