Merge "Address comments from API council."

10 files changed, 58 insertions, 53 deletions

diff --git a/api/current.txt b/api/current.txt
index f6889e85c53b..084e6cb8d94a 100644
--- a/api/current.txt
+++ b/api/current.txt
@@ -34099,7 +34099,7 @@ package android.provider {
     method public static android.net.Uri createDocument(android.content.ContentResolver, android.net.Uri, java.lang.String, java.lang.String);
     method public static android.content.IntentSender createWebLinkIntent(android.content.ContentResolver, android.net.Uri, android.os.Bundle);
     method public static boolean deleteDocument(android.content.ContentResolver, android.net.Uri);
-    method public static java.util.List<java.lang.String> findDocumentPath(android.content.ContentResolver, android.net.Uri);
+    method public static android.provider.DocumentsContract.Path findDocumentPath(android.content.ContentResolver, android.net.Uri);
     method public static java.lang.String getDocumentId(android.net.Uri);
     method public static android.graphics.Bitmap getDocumentThumbnail(android.content.ContentResolver, android.net.Uri, android.graphics.Point, android.os.CancellationSignal);
     method public static java.lang.String getRootId(android.net.Uri);
diff --git a/api/system-current.txt b/api/system-current.txt
index bf4b3ba88be7..897594ea90b6 100644
--- a/api/system-current.txt
+++ b/api/system-current.txt
@@ -37001,7 +37001,7 @@ package android.provider {
     method public static android.net.Uri createDocument(android.content.ContentResolver, android.net.Uri, java.lang.String, java.lang.String);
     method public static android.content.IntentSender createWebLinkIntent(android.content.ContentResolver, android.net.Uri, android.os.Bundle);
     method public static boolean deleteDocument(android.content.ContentResolver, android.net.Uri);
-    method public static java.util.List<java.lang.String> findDocumentPath(android.content.ContentResolver, android.net.Uri);
+    method public static android.provider.DocumentsContract.Path findDocumentPath(android.content.ContentResolver, android.net.Uri);
     method public static java.lang.String getDocumentId(android.net.Uri);
     method public static android.graphics.Bitmap getDocumentThumbnail(android.content.ContentResolver, android.net.Uri, android.graphics.Point, android.os.CancellationSignal);
     method public static java.lang.String getRootId(android.net.Uri);
diff --git a/api/test-current.txt b/api/test-current.txt
index 1fa3e162e2b5..8537ce6784fc 100644
--- a/api/test-current.txt
+++ b/api/test-current.txt
@@ -34226,7 +34226,7 @@ package android.provider {
     method public static android.net.Uri createDocument(android.content.ContentResolver, android.net.Uri, java.lang.String, java.lang.String);
     method public static android.content.IntentSender createWebLinkIntent(android.content.ContentResolver, android.net.Uri, android.os.Bundle);
     method public static boolean deleteDocument(android.content.ContentResolver, android.net.Uri);
-    method public static java.util.List<java.lang.String> findDocumentPath(android.content.ContentResolver, android.net.Uri);
+    method public static android.provider.DocumentsContract.Path findDocumentPath(android.content.ContentResolver, android.net.Uri);
     method public static java.lang.String getDocumentId(android.net.Uri);
     method public static android.graphics.Bitmap getDocumentThumbnail(android.content.ContentResolver, android.net.Uri, android.graphics.Point, android.os.CancellationSignal);
     method public static java.lang.String getRootId(android.net.Uri);
diff --git a/core/java/android/provider/DocumentsContract.java b/core/java/android/provider/DocumentsContract.java
index 33b59038c106..173603d53e1d 100644
--- a/core/java/android/provider/DocumentsContract.java
+++ b/core/java/android/provider/DocumentsContract.java
@@ -1367,24 +1367,25 @@ public final class DocumentsContract {
     }
 
     /**
-     * Finds the canonical path to the top of the tree. The return value starts
-     * from the top of the tree or the root document to the requested document,
-     * both inclusive.
+     * Finds the canonical path from the top of the document tree.
+     *
+     * The {@link Path#getPath()} of the return value contains the document ID
+     * of all documents along the path from the top the document tree to the
+     * requested document, both inclusive.
      *
-     * Document ID should be unique across roots.
+     * The {@link Path#getRootId()} of the return value returns {@code null}.
      *
      * @param treeUri treeUri of the document which path is requested.
-     * @return a list of documents ID starting from the top of the tree to the
-     *      requested document, or {@code null} if failed.
+     * @return the path of the document, or {@code null} if failed.
      * @see DocumentsProvider#findDocumentPath(String, String)
      */
-    public static List<String> findDocumentPath(ContentResolver resolver, Uri treeUri) {
+    public static Path findDocumentPath(ContentResolver resolver, Uri treeUri) {
         checkArgument(isTreeUri(treeUri), treeUri + " is not a tree uri.");
 
         final ContentProviderClient client = resolver.acquireUnstableContentProviderClient(
                 treeUri.getAuthority());
         try {
-            return findDocumentPath(client, treeUri).getPath();
+            return findDocumentPath(client, treeUri);
         } catch (Exception e) {
             Log.w(TAG, "Failed to find path", e);
             return null;
@@ -1394,12 +1395,14 @@ public final class DocumentsContract {
     }
 
     /**
-     * Finds the canonical path. If uri is a document uri returns path to a root and
-     * its associated root id. If uri is a tree uri returns the path to the top of
-     * the tree. The {@link Path#getPath()} in the return value starts from the top of
-     * the tree or the root document to the requested document, both inclusive.
+     * Finds the canonical path. If uri is a document uri returns path from a root and
+     * its associated root id. If uri is a tree uri returns the path from the top of
+     * the tree. The {@link Path#getPath()} of the return value contains document ID
+     * starts from the top of the tree or the root document to the requested document,
+     * both inclusive.
      *
-     * Document id should be unique across roots.
+     * Callers can expect the root ID returned from multiple calls to this method is
+     * consistent.
      *
      * @param uri uri of the document which path is requested. It can be either a
      *          plain document uri or a tree uri.
diff --git a/core/java/android/provider/DocumentsProvider.java b/core/java/android/provider/DocumentsProvider.java
index f5e558a01c56..89a80f054ded 100644
--- a/core/java/android/provider/DocumentsProvider.java
+++ b/core/java/android/provider/DocumentsProvider.java
@@ -65,6 +65,7 @@ import android.util.Log;
 import libcore.io.IoUtils;
 
 import java.io.FileNotFoundException;
+import java.util.LinkedList;
 import java.util.Objects;
 
 /**
@@ -352,14 +353,14 @@ public abstract class DocumentsProvider extends ContentProvider {
      * Different roots should use different document ID to refer to the same
      * document.
      *
-     * @param childDocumentId the document which path is requested.
      * @param parentDocumentId the document from which the path starts if not null,
      *     or null to indicate a path from the root is requested.
+     * @param childDocumentId the document which path is requested.
      * @return the path of the requested document. If parentDocumentId is null
      *     returned root ID must not be null. If parentDocumentId is not null
      *     returned root ID must be null.
      */
-    public Path findDocumentPath(String childDocumentId, @Nullable String parentDocumentId)
+    public Path findDocumentPath(@Nullable String parentDocumentId, String childDocumentId)
             throws FileNotFoundException {
         throw new UnsupportedOperationException("findDocumentPath not supported.");
     }
@@ -1048,13 +1049,19 @@ public abstract class DocumentsProvider extends ContentProvider {
                     ? DocumentsContract.getTreeDocumentId(documentUri)
                     : null;
 
-            Path path = findDocumentPath(documentId, parentDocumentId);
+            Path path = findDocumentPath(parentDocumentId, documentId);
 
             // Ensure provider doesn't leak information to unprivileged callers.
             if (isTreeUri) {
                 if (!Objects.equals(path.getPath().get(0), parentDocumentId)) {
                     Log.wtf(TAG, "Provider doesn't return path from the tree root. Expected: "
                             + parentDocumentId + " found: " + path.getPath().get(0));
+
+                    LinkedList<String> docs = new LinkedList<>(path.getPath());
+                    while (docs.size() > 1 && !Objects.equals(docs.getFirst(), parentDocumentId)) {
+                        docs.removeFirst();
+                    }
+                    path = new Path(null, docs);
                 }
 
                 if (path.getRootId() != null) {
diff --git a/core/tests/coretests/src/android/provider/DocumentsProviderTest.java b/core/tests/coretests/src/android/provider/DocumentsProviderTest.java
index d1c68a92b6e7..09cbbff9077f 100644
--- a/core/tests/coretests/src/android/provider/DocumentsProviderTest.java
+++ b/core/tests/coretests/src/android/provider/DocumentsProviderTest.java
@@ -52,7 +52,7 @@ public class DocumentsProviderTest extends ProviderTestCase2<TestDocumentsProvid
         mResolver = getMockContentResolver();
     }
 
-    public void testFindPath_docUri() throws Exception {
+    public void testFindDocumentPath_docUri() throws Exception {
         final Path expected = new Path(ROOT_ID, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));
         mProvider.nextPath = expected;
 
@@ -65,7 +65,7 @@ public class DocumentsProviderTest extends ProviderTestCase2<TestDocumentsProvid
         }
     }
 
-    public void testFindPath_treeUri() throws Exception {
+    public void testFindDocumentPath_treeUri() throws Exception {
         mProvider.nextIsChildDocument = true;
 
         final Path expected = new Path(null, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));
@@ -73,12 +73,13 @@ public class DocumentsProviderTest extends ProviderTestCase2<TestDocumentsProvid
 
         final Uri docUri = buildTreeDocumentUri(
                 TestDocumentsProvider.AUTHORITY, PARENT_DOCUMENT_ID, DOCUMENT_ID);
-        final List<String> actual = DocumentsContract.findDocumentPath(mResolver, docUri);
+        final Path actual = DocumentsContract.findDocumentPath(mResolver, docUri);
 
-        assertEquals(expected.getPath(), actual);
+        assertNull(actual.getRootId());
+        assertEquals(expected.getPath(), actual.getPath());
     }
 
-    public void testFindPath_treeUri_throwsOnNonChildDocument() throws Exception {
+    public void testFindDocumentPath_treeUri_throwsOnNonChildDocument() throws Exception {
         mProvider.nextPath = new Path(null, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));
 
         final Uri docUri = buildTreeDocumentUri(
@@ -86,18 +87,28 @@ public class DocumentsProviderTest extends ProviderTestCase2<TestDocumentsProvid
         assertNull(DocumentsContract.findDocumentPath(mResolver, docUri));
     }
 
-    public void testFindPath_treeUri_erasesNonNullRootId() throws Exception {
+    public void testFindDocumentPath_treeUri_erasesNonNullRootId() throws Exception {
         mProvider.nextIsChildDocument = true;
 
         mProvider.nextPath = new Path(ROOT_ID, Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID));
 
         final Uri docUri = buildTreeDocumentUri(
                 TestDocumentsProvider.AUTHORITY, PARENT_DOCUMENT_ID, DOCUMENT_ID);
-        try (ContentProviderClient client =
-                     mResolver.acquireUnstableContentProviderClient(docUri)) {
-            Path path = DocumentsContract.findDocumentPath(client, docUri);
-            assertNull(path.getRootId());
-        }
+        Path path = DocumentsContract.findDocumentPath(mResolver, docUri);
+        assertNull(path.getRootId());
+        assertEquals(Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID), path.getPath());
+    }
+
+    public void testFindDocumentPath_treeUri_erasesDocsOutsideTree() throws Exception {
+        mProvider.nextIsChildDocument = true;
+
+        mProvider.nextPath = new Path(
+                null, Arrays.asList(ANCESTOR_DOCUMENT_ID, PARENT_DOCUMENT_ID, DOCUMENT_ID));
+
+        final Uri docUri = buildTreeDocumentUri(
+                TestDocumentsProvider.AUTHORITY, PARENT_DOCUMENT_ID, DOCUMENT_ID);
+        Path path = DocumentsContract.findDocumentPath(mResolver, docUri);
+        assertEquals(Arrays.asList(PARENT_DOCUMENT_ID, DOCUMENT_ID), path.getPath());
     }
 
     private static Uri buildTreeDocumentUri(String authority, String parentDocId, String docId) {
diff --git a/core/tests/coretests/src/android/provider/TestDocumentsProvider.java b/core/tests/coretests/src/android/provider/TestDocumentsProvider.java
index d61049d74763..1bd8ff6bdea3 100644
--- a/core/tests/coretests/src/android/provider/TestDocumentsProvider.java
+++ b/core/tests/coretests/src/android/provider/TestDocumentsProvider.java
@@ -85,7 +85,7 @@ public class TestDocumentsProvider extends DocumentsProvider {
     }
 
     @Override
-    public Path findDocumentPath(String documentId, @Nullable String parentDocumentId) {
+    public Path findDocumentPath(@Nullable String parentDocumentId, String documentId) {
         lastDocumentId = documentId;
         lastParentDocumentId = parentDocumentId;
 
diff --git a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
index 3cc9f65ecd85..8802010c06d0 100644
--- a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
+++ b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
@@ -19,24 +19,14 @@ package com.android.externalstorage;
 import android.annotation.Nullable;
 import android.content.ContentResolver;
 import android.content.Context;
-import android.content.Intent;
 import android.content.UriPermission;
-import android.content.pm.ParceledListSlice;
-import android.content.res.AssetFileDescriptor;
 import android.database.Cursor;
 import android.database.MatrixCursor;
 import android.database.MatrixCursor.RowBuilder;
-import android.graphics.Point;
 import android.net.Uri;
 import android.os.Binder;
 import android.os.Bundle;
-import android.os.CancellationSignal;
 import android.os.Environment;
-import android.os.FileObserver;
-import android.os.FileUtils;
-import android.os.Handler;
-import android.os.ParcelFileDescriptor;
-import android.os.ParcelFileDescriptor.OnCloseListener;
 import android.os.UserHandle;
 import android.os.storage.DiskInfo;
 import android.os.storage.StorageManager;
@@ -45,15 +35,12 @@ import android.provider.DocumentsContract;
 import android.provider.DocumentsContract.Document;
 import android.provider.DocumentsContract.Path;
 import android.provider.DocumentsContract.Root;
-import android.provider.DocumentsProvider;
-import android.provider.MediaStore;
 import android.provider.Settings;
 import android.text.TextUtils;
 import android.util.ArrayMap;
 import android.util.DebugUtils;
 import android.util.Log;
 import android.util.Pair;
-import android.webkit.MimeTypeMap;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.content.FileSystemProvider;
@@ -62,10 +49,8 @@ import com.android.internal.util.IndentingPrintWriter;
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileNotFoundException;
-import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
 
@@ -408,7 +393,7 @@ public class ExternalStorageProvider extends FileSystemProvider {
     }
 
     @Override
-    public Path findDocumentPath(String childDocId, @Nullable String parentDocId)
+    public Path findDocumentPath(@Nullable String parentDocId, String childDocId)
             throws FileNotFoundException {
         final Pair<RootInfo, File> resolvedDocId = resolveDocId(childDocId, false);
         final RootInfo root = resolvedDocId.first;
diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
index b8c10a6a3366..e60b5a9a373a 100644
--- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
+++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
@@ -435,7 +435,7 @@ public class MtpDocumentsProvider extends DocumentsProvider {
     }
 
     @Override
-    public Path findDocumentPath(String childDocumentId, String parentDocumentId)
+    public Path findDocumentPath(String parentDocumentId, String childDocumentId)
             throws FileNotFoundException {
         final LinkedList<String> ids = new LinkedList<>();
         final Identifier childIdentifier = mDatabase.createIdentifier(childDocumentId);
diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
index 491e24deefa2..29783e4165c7 100644
--- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
+++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
@@ -35,7 +35,6 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.LinkedList;
-import java.util.Objects;
 import java.util.Queue;
 import java.util.concurrent.TimeoutException;
 
@@ -802,7 +801,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase {
         setupRoots(0, new MtpRoot[] { new MtpRoot(0, 0, "Storage", 1000, 1000, "") });
         setupHierarchyDocuments("1");
 
-        final Path path = mProvider.findDocumentPath("15", null);
+        final Path path = mProvider.findDocumentPath(null, "15");
         assertEquals("1", path.getRootId());
         assertEquals(4, path.getPath().size());
         assertEquals("1", path.getPath().get(0));
@@ -816,7 +815,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase {
         setupRoots(0, new MtpRoot[] { new MtpRoot(0, 0, "Storage", 1000, 1000, "") });
         setupHierarchyDocuments("1");
 
-        final Path path = mProvider.findDocumentPath("18", "3");
+        final Path path = mProvider.findDocumentPath("3", "18");
         assertNull(path.getRootId());
         assertEquals(3, path.getPath().size());
         assertEquals("3", path.getPath().get(0));
@@ -831,7 +830,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase {
                 new MtpRoot(0, 1, "Storage B", 1000, 1000, "") });
         setupHierarchyDocuments("2");
 
-        final Path path = mProvider.findDocumentPath("16", null);
+        final Path path = mProvider.findDocumentPath(null, "16");
         assertEquals("2", path.getRootId());
         assertEquals(4, path.getPath().size());
         assertEquals("2", path.getPath().get(0));
@@ -847,7 +846,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase {
                 new MtpRoot(0, 1, "Storage B", 1000, 1000, "") });
         setupHierarchyDocuments("2");
 
-        final Path path = mProvider.findDocumentPath("19", "4");
+        final Path path = mProvider.findDocumentPath("4", "19");
         assertNull(path.getRootId());
         assertEquals(3, path.getPath().size());
         assertEquals("4", path.getPath().get(0));