diff options
author | 2020-09-25 16:36:08 -0700 | |
---|---|---|
committer | 2020-09-29 10:18:30 -0700 | |
commit | eda268a425395c1ab605cfa9333e3b0d53dada9c (patch) | |
tree | 5b0dd4e7706aaf5fa7941c749b753246d54b5845 | |
parent | a9219f8a58cf339ce0765de0da88949f50a2cf75 (diff) |
Unblock UI thread from updating model.
The model data is loaded from DirectoryResult.cursor, so it makes more sense
to populate all relevant data directly when we generate DirectoryResult
in the background.
Bug: 169449744
Test: atest DocumentsUIGoogleTests
Change-Id: I26557f9fe93ab6cb01599c51f43c0f393e17a2a3
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); |