diff options
11 files changed, 171 insertions, 64 deletions
diff --git a/src/com/android/documentsui/DirectoryLoader.java b/src/com/android/documentsui/DirectoryLoader.java index eb4e0a9f0..816144758 100644 --- a/src/com/android/documentsui/DirectoryLoader.java +++ b/src/com/android/documentsui/DirectoryLoader.java @@ -56,7 +56,7 @@ import java.util.concurrent.Executor; public class DirectoryLoader extends AsyncTaskLoader<DirectoryResult> { private static final String TAG = "DirectoryLoader"; - private static final String[] SEARCH_REJECT_MIMES = new String[] { Document.MIME_TYPE_DIR }; + private static final String[] SEARCH_REJECT_MIMES = new String[]{Document.MIME_TYPE_DIR}; private static final String[] PHOTO_PICKING_ACCEPT_MIMES = new String[] {Document.MIME_TYPE_DIR, MimeTypes.IMAGE_MIME}; @@ -196,7 +196,7 @@ public class DirectoryLoader extends AsyncTaskLoader<DirectoryResult> { } else { cursor = mModel.sortCursor(cursor, mFileTypeLookup); } - result.cursor = cursor; + result.setCursor(cursor); } catch (Exception e) { Log.w(TAG, "Failed to query", e); result.exception = e; @@ -305,8 +305,8 @@ public class DirectoryLoader extends AsyncTaskLoader<DirectoryResult> { // Ensure the loader is stopped onStopLoading(); - if (mResult != null && mResult.cursor != null && mObserver != null) { - mResult.cursor.unregisterContentObserver(mObserver); + if (mResult != null && mResult.getCursor() != null && mObserver != null) { + mResult.getCursor().unregisterContentObserver(mObserver); } FileUtils.closeQuietly(mResult); @@ -314,10 +314,10 @@ public class DirectoryLoader extends AsyncTaskLoader<DirectoryResult> { } private boolean checkIfCursorStale(DirectoryResult result) { - if (result == null || result.cursor == null || result.cursor.isClosed()) { + if (result == null || result.getCursor() == null || result.getCursor().isClosed()) { return true; } - Cursor cursor = result.cursor; + Cursor cursor = result.getCursor(); try { cursor.moveToPosition(-1); for (int pos = 0; pos < cursor.getCount(); ++pos) { diff --git a/src/com/android/documentsui/DirectoryResult.java b/src/com/android/documentsui/DirectoryResult.java index f820db988..15ab0076d 100644 --- a/src/com/android/documentsui/DirectoryResult.java +++ b/src/com/android/documentsui/DirectoryResult.java @@ -16,29 +16,97 @@ package com.android.documentsui; +import static com.android.documentsui.base.DocumentInfo.getCursorString; + import android.content.ContentProviderClient; import android.database.Cursor; import android.os.FileUtils; +import android.provider.DocumentsContract; +import android.util.Log; import com.android.documentsui.archives.ArchivesProvider; import com.android.documentsui.base.DocumentInfo; +import java.util.HashSet; +import java.util.Set; + public class DirectoryResult implements AutoCloseable { - public Cursor cursor; + private static final String TAG = "DirectoryResult"; + public Exception exception; public DocumentInfo doc; ContentProviderClient client; + private Cursor mCursor; + private Set<String> mFileNames; + private String[] mModelIds; + @Override public void close() { - FileUtils.closeQuietly(cursor); + FileUtils.closeQuietly(mCursor); if (client != null && doc.isInArchive()) { ArchivesProvider.releaseArchive(client, doc.derivedUri); } FileUtils.closeQuietly(client); - cursor = null; client = null; doc = null; + + setCursor(null); + } + + public Cursor getCursor() { + return mCursor; + } + + public String[] getModelIds() { + return mModelIds; + } + + public Set<String> getFileNames() { + return mFileNames; + } + + /** Update the cursor and populate cursor-related fields. */ + public void setCursor(Cursor cursor) { + mCursor = cursor; + + if (mCursor == null) { + mFileNames = null; + mModelIds = null; + } else { + loadDataFromCursor(); + } + } + + /** Populate cursor-related field. Must not be called from UI thread. */ + private void loadDataFromCursor() { + ThreadHelper.assertNotOnMainThread(); + int cursorCount = mCursor.getCount(); + String[] modelIds = new String[cursorCount]; + Set<String> fileNames = new HashSet<>(); + try { + mCursor.moveToPosition(-1); + for (int pos = 0; pos < cursorCount; ++pos) { + if (!mCursor.moveToNext()) { + Log.e(TAG, "Fail to move cursor to next pos: " + pos); + return; + } + + // Generates a Model ID for a cursor entry that refers to a document. The Model + // ID is a unique string that can be used to identify the document referred to by + // the cursor. Prefix the ids with the authority to avoid collisions. + modelIds[pos] = ModelId.build(mCursor); + fileNames.add( + getCursorString(mCursor, DocumentsContract.Document.COLUMN_DISPLAY_NAME)); + } + } catch (Exception e) { + Log.e(TAG, "Exception when moving cursor. Stale cursor?", e); + return; + } + + // Model related data is only non-null when no error iterating through cursor. + mModelIds = modelIds; + mFileNames = fileNames; } } diff --git a/src/com/android/documentsui/Model.java b/src/com/android/documentsui/Model.java index 49e3c9b26..fdcebae3c 100644 --- a/src/com/android/documentsui/Model.java +++ b/src/com/android/documentsui/Model.java @@ -16,7 +16,6 @@ package com.android.documentsui; -import static com.android.documentsui.base.DocumentInfo.getCursorString; import static com.android.documentsui.base.SharedMinimal.DEBUG; import static com.android.documentsui.base.SharedMinimal.VERBOSE; @@ -25,7 +24,6 @@ import android.database.Cursor; import android.net.Uri; import android.os.Bundle; import android.provider.DocumentsContract; -import android.provider.DocumentsContract.Document; import android.util.Log; import androidx.annotation.IntDef; @@ -125,11 +123,21 @@ public class Model { return; } - mCursor = result.cursor; + mCursor = result.getCursor(); mCursorCount = mCursor.getCount(); doc = result.doc; - updateModelData(); + if (result.getModelIds() != null && result.getFileNames() != null) { + mIds = result.getModelIds(); + mFileNames.clear(); + mFileNames.addAll(result.getFileNames()); + + // Populate the positions. + mPositions.clear(); + for (int i = 0; i < mCursorCount; ++i) { + mPositions.put(mIds[i], i); + } + } final Bundle extras = mCursor.getExtras(); if (extras != null) { @@ -146,33 +154,6 @@ public class Model { return mCursorCount; } - /** - * Scan over the incoming cursor data, generate Model IDs for each row, and sort the IDs - * according to the current sort order. - */ - private void updateModelData() { - mIds = new String[mCursorCount]; - mFileNames.clear(); - mCursor.moveToPosition(-1); - for (int pos = 0; pos < mCursorCount; ++pos) { - if (!mCursor.moveToNext()) { - Log.e(TAG, "Fail to move cursor to next pos: " + pos); - return; - } - // Generates a Model ID for a cursor entry that refers to a document. The Model ID is a - // unique string that can be used to identify the document referred to by the cursor. - // Prefix the ids with the authority to avoid collisions. - mIds[pos] = ModelId.build(mCursor); - mFileNames.add(getCursorString(mCursor, Document.COLUMN_DISPLAY_NAME)); - } - - // Populate the positions. - mPositions.clear(); - for (int i = 0; i < mCursorCount; ++i) { - mPositions.put(mIds[i], i); - } - } - public boolean hasFileWithName(String name) { return mFileNames.contains(name); } diff --git a/src/com/android/documentsui/MultiRootDocumentsLoader.java b/src/com/android/documentsui/MultiRootDocumentsLoader.java index 74e44a14a..952aa575c 100644 --- a/src/com/android/documentsui/MultiRootDocumentsLoader.java +++ b/src/com/android/documentsui/MultiRootDocumentsLoader.java @@ -237,7 +237,7 @@ public abstract class MultiRootDocumentsLoader extends AsyncTaskLoader<Directory extras.putBoolean(DocumentsContract.EXTRA_LOADING, !allDone); sorted.setExtras(extras); - result.cursor = sorted; + result.setCursor(sorted); return result; } @@ -458,10 +458,10 @@ public abstract class MultiRootDocumentsLoader extends AsyncTaskLoader<Directory } private boolean checkIfCursorStale(DirectoryResult result) { - if (result == null || result.cursor == null || result.cursor.isClosed()) { + if (result == null || result.getCursor() == null || result.getCursor().isClosed()) { return true; } - Cursor cursor = result.cursor; + Cursor cursor = result.getCursor(); try { cursor.moveToPosition(-1); for (int pos = 0; pos < cursor.getCount(); ++pos) { diff --git a/src/com/android/documentsui/ThreadHelper.java b/src/com/android/documentsui/ThreadHelper.java new file mode 100644 index 000000000..bb123b69b --- /dev/null +++ b/src/com/android/documentsui/ThreadHelper.java @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.documentsui; + +import android.os.Looper; + +/** Class for handler/thread utils. */ +public final class ThreadHelper { + private ThreadHelper() { + } + + static final String MUST_NOT_ON_MAIN_THREAD_MSG = + "This method should not be called on main thread."; + static final String MUST_ON_MAIN_THREAD_MSG = + "This method should only be called on main thread."; + + /** Verifies that current thread is not the UI thread. */ + public static void assertNotOnMainThread() { + if (Looper.getMainLooper().getThread() == Thread.currentThread()) { + fatalAssert(MUST_NOT_ON_MAIN_THREAD_MSG); + } + } + + /** Verifies that current thread is the UI thread. */ + public static void assertOnMainThread() { + if (Looper.getMainLooper().getThread() != Thread.currentThread()) { + fatalAssert(MUST_ON_MAIN_THREAD_MSG); + } + } + + /** + * Exceptions thrown in background threads are silently swallowed on Android. Use the + * uncaught exception handler of the UI thread to force the app to crash. + */ + public static void fatalAssert(final String message) { + crashMainThread(new AssertionError(message)); + } + + private static void crashMainThread(Throwable t) { + Thread.UncaughtExceptionHandler uiThreadExceptionHandler = + Looper.getMainLooper().getThread().getUncaughtExceptionHandler(); + uiThreadExceptionHandler.uncaughtException(Thread.currentThread(), t); + } +} diff --git a/tests/common/com/android/documentsui/testing/TestModel.java b/tests/common/com/android/documentsui/testing/TestModel.java index 5a0ff89d3..452238189 100644 --- a/tests/common/com/android/documentsui/testing/TestModel.java +++ b/tests/common/com/android/documentsui/testing/TestModel.java @@ -64,7 +64,7 @@ public class TestModel extends Model { public void update() { DirectoryResult r = new DirectoryResult(); - r.cursor = mCursor; + r.setCursor(mCursor); super.update(r); } diff --git a/tests/unit/com/android/documentsui/DirectoryResultTest.java b/tests/unit/com/android/documentsui/DirectoryResultTest.java index 5994bdc1c..3cda1bc1a 100644 --- a/tests/unit/com/android/documentsui/DirectoryResultTest.java +++ b/tests/unit/com/android/documentsui/DirectoryResultTest.java @@ -48,7 +48,7 @@ public class DirectoryResultTest { DocumentInfo doc = new DocumentInfo(); result.client = mClient; - result.cursor = mCursor; + result.setCursor(mCursor); result.doc = doc; result.close(); diff --git a/tests/unit/com/android/documentsui/GlobalSearchLoaderTest.java b/tests/unit/com/android/documentsui/GlobalSearchLoaderTest.java index d41ca57a5..c52dbd3de 100644 --- a/tests/unit/com/android/documentsui/GlobalSearchLoaderTest.java +++ b/tests/unit/com/android/documentsui/GlobalSearchLoaderTest.java @@ -132,7 +132,7 @@ public class GlobalSearchLoaderTest { final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertEquals(1, c.getCount()); c.moveToNext(); @@ -151,18 +151,18 @@ public class GlobalSearchLoaderTest { assertEquals(false, mLoader.mState.showHiddenFiles); DirectoryResult result = mLoader.loadInBackground(); - assertEquals(0, result.cursor.getCount()); + assertEquals(0, result.getCursor().getCount()); mLoader.mState.showHiddenFiles = true; result = mLoader.loadInBackground(); - assertEquals(2, result.cursor.getCount()); + assertEquals(2, result.getCursor().getCount()); } @Test public void testSearchResult_isNotMovable() { final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertEquals(1, c.getCount()); c.moveToNext(); @@ -193,7 +193,7 @@ public class GlobalSearchLoaderTest { .setNextChildDocumentsReturns(otherUserDoc); final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertThat(c.getCount()).isEqualTo(1); c.moveToNext(); @@ -223,7 +223,7 @@ public class GlobalSearchLoaderTest { SortModel.SORT_DIMENSION_ID_TITLE, SortDimension.SORT_DIRECTION_ASCENDING); final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertEquals(2, c.getCount()); @@ -257,7 +257,7 @@ public class GlobalSearchLoaderTest { SortModel.SORT_DIMENSION_ID_TITLE, SortDimension.SORT_DIRECTION_ASCENDING); final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertEquals(3, c.getCount()); @@ -289,7 +289,7 @@ public class GlobalSearchLoaderTest { SortModel.SORT_DIMENSION_ID_TITLE, SortDimension.SORT_DIRECTION_ASCENDING); final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertEquals(1, c.getCount()); @@ -323,7 +323,7 @@ public class GlobalSearchLoaderTest { SortModel.SORT_DIMENSION_ID_TITLE, SortDimension.SORT_DIRECTION_ASCENDING); final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertEquals(3, c.getCount()); @@ -352,7 +352,7 @@ public class GlobalSearchLoaderTest { SortModel.SORT_DIMENSION_ID_TITLE, SortDimension.SORT_DIRECTION_ASCENDING); final DirectoryResult result = mLoader.loadInBackground(); - assertThat(result.cursor.getCount()).isEqualTo(0); + assertThat(result.getCursor().getCount()).isEqualTo(0); // We don't expect exception even if all roots are from other users. assertThat(result.exception).isNull(); diff --git a/tests/unit/com/android/documentsui/ModelTest.java b/tests/unit/com/android/documentsui/ModelTest.java index 7d44477c5..f9835b11c 100644 --- a/tests/unit/com/android/documentsui/ModelTest.java +++ b/tests/unit/com/android/documentsui/ModelTest.java @@ -93,7 +93,7 @@ public class ModelTest { cursor = c; DirectoryResult r = new DirectoryResult(); - r.cursor = cursor; + r.setCursor(cursor); // Instantiate the model with a dummy view adapter and listener that (for now) do nothing. model = new Model(features); @@ -132,7 +132,7 @@ public class ModelTest { // Update the model, then make sure it contains all the expected items. DirectoryResult r = new DirectoryResult(); - r.cursor = cIn; + r.setCursor(cIn); model.update(r); assertEquals(ITEM_COUNT * 2, model.getItemCount()); diff --git a/tests/unit/com/android/documentsui/RecentsLoaderTests.java b/tests/unit/com/android/documentsui/RecentsLoaderTests.java index f26b39a1c..d95e9243c 100644 --- a/tests/unit/com/android/documentsui/RecentsLoaderTests.java +++ b/tests/unit/com/android/documentsui/RecentsLoaderTests.java @@ -99,7 +99,7 @@ public class RecentsLoaderTests { final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertEquals(0, c.getCount()); } @@ -116,11 +116,11 @@ public class RecentsLoaderTests { assertEquals(false, mLoader.mState.showHiddenFiles); DirectoryResult result = mLoader.loadInBackground(); - assertEquals(0, result.cursor.getCount()); + assertEquals(0, result.getCursor().getCount()); mLoader.mState.showHiddenFiles = true; result = mLoader.loadInBackground(); - assertEquals(2, result.cursor.getCount()); + assertEquals(2, result.getCursor().getCount()); } @Test @@ -135,7 +135,7 @@ public class RecentsLoaderTests { final DirectoryResult result = mLoader.loadInBackground(); - final Cursor c = result.cursor; + final Cursor c = result.getCursor(); assertEquals(1, c.getCount()); for (int i = 0; i < c.getCount(); ++i) { c.moveToNext(); @@ -178,7 +178,7 @@ public class RecentsLoaderTests { TestProvidersAccess.OtherUser.USER_ID); final DirectoryResult result = mLoader.loadInBackground(); - assertThat(result.cursor).isNull(); + assertThat(result.getCursor()).isNull(); assertThat(result.exception).isInstanceOf(CrossProfileNoPermissionException.class); } @@ -187,7 +187,7 @@ public class RecentsLoaderTests { when(mActivity.userManager.isQuietModeEnabled(any())).thenReturn(true); final DirectoryResult result = mLoader.loadInBackground(); - assertThat(result.cursor).isNull(); + assertThat(result.getCursor()).isNull(); assertThat(result.exception).isInstanceOf(CrossProfileQuietModeException.class); } } diff --git a/tests/unit/com/android/documentsui/picker/MenuManagerTest.java b/tests/unit/com/android/documentsui/picker/MenuManagerTest.java index d3e2f2d9f..cf9699a3b 100644 --- a/tests/unit/com/android/documentsui/picker/MenuManagerTest.java +++ b/tests/unit/com/android/documentsui/picker/MenuManagerTest.java @@ -524,7 +524,7 @@ public final class MenuManagerTest { } DirectoryResult r = new DirectoryResult(); - r.cursor = c; + r.setCursor(c); Model model = new Model(new TestFeatures()); model.update(r); |