From deffadeb7485e8660ecce12822e259d96fa06dce Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 24 Sep 2013 12:07:12 -0700 Subject: [PATCH] Freshen restored stacks, more UX bug fixes. When restoring a persisted stack, such as last location or a location where save occurred, freshen the DocumentStack to get the latest details from backend, and ensure the path is still valid. Filter Recent directories in create mode based on roots allowed by incoming request. Remember when user last picked an external app, and open drawer next time user launches for GET_CONTENT. Fix state list drawable ordering, and avoid clobbering Drawable callback when wrapping in InsetDrawable; tricksy hobbitses! Make grid items smaller to always fit two columns on phones. Draw grid items all the way to screen edge; don't clip to padding. Better error message when folder creation failed. Show Recents in grid mode when picking any visual content, not just images. Bug: 10846100, 10903211, 10898993, 10892808, 10892058, 10892009, 10885012 Change-Id: Ia0f88d911abc6ea03645d6fd3e04271c21d5936a --- .../res/drawable/item_background.xml | 18 +++-- .../res/layout/fragment_directory.xml | 1 + packages/DocumentsUI/res/values/dimens.xml | 4 +- packages/DocumentsUI/res/values/strings.xml | 2 + .../documentsui/CreateDirectoryFragment.java | 2 +- .../documentsui/DirectoryFragment.java | 12 +++- .../documentsui/DocumentsActivity.java | 71 ++++++++++++++----- .../documentsui/RecentsCreateFragment.java | 22 +++++- .../android/documentsui/RecentsProvider.java | 24 ++++--- .../com/android/documentsui/TestActivity.java | 27 ++++++- .../documentsui/model/DocumentInfo.java | 52 +++++++++----- .../documentsui/model/DocumentStack.java | 25 +++++++ 12 files changed, 199 insertions(+), 61 deletions(-) diff --git a/packages/DocumentsUI/res/drawable/item_background.xml b/packages/DocumentsUI/res/drawable/item_background.xml index 7447aa8c0527a..6fcab3c8264a7 100644 --- a/packages/DocumentsUI/res/drawable/item_background.xml +++ b/packages/DocumentsUI/res/drawable/item_background.xml @@ -16,18 +16,16 @@ - - - + - - + + - - - - - + + + + + diff --git a/packages/DocumentsUI/res/layout/fragment_directory.xml b/packages/DocumentsUI/res/layout/fragment_directory.xml index 98ded3426fd2a..77cdc3ba067e0 100644 --- a/packages/DocumentsUI/res/layout/fragment_directory.xml +++ b/packages/DocumentsUI/res/layout/fragment_directory.xml @@ -42,6 +42,7 @@ android:paddingEnd="@dimen/grid_padding_horiz" android:paddingTop="@dimen/grid_padding" android:paddingBottom="@dimen/grid_padding" + android:clipToPadding="false" android:scrollbarStyle="outsideOverlay" android:listSelector="@android:color/transparent" android:visibility="gone" /> diff --git a/packages/DocumentsUI/res/values/dimens.xml b/packages/DocumentsUI/res/values/dimens.xml index 4a93cdbf657d1..924a8d6c6d5aa 100644 --- a/packages/DocumentsUI/res/values/dimens.xml +++ b/packages/DocumentsUI/res/values/dimens.xml @@ -17,8 +17,8 @@ 32dp 24dp - 180dp - 180dp + 160dp + 170dp 4dp 4dp diff --git a/packages/DocumentsUI/res/values/strings.xml b/packages/DocumentsUI/res/values/strings.xml index 682ae4ab7ef2e..b7dcb71ee76da 100644 --- a/packages/DocumentsUI/res/values/strings.xml +++ b/packages/DocumentsUI/res/values/strings.xml @@ -62,6 +62,8 @@ Failed to save document + + Failed to create folder Recent diff --git a/packages/DocumentsUI/src/com/android/documentsui/CreateDirectoryFragment.java b/packages/DocumentsUI/src/com/android/documentsui/CreateDirectoryFragment.java index d8e60aa2eed34..9d92cd89596ff 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/CreateDirectoryFragment.java +++ b/packages/DocumentsUI/src/com/android/documentsui/CreateDirectoryFragment.java @@ -76,7 +76,7 @@ public class CreateDirectoryFragment extends DialogFragment { final DocumentInfo childDoc = DocumentInfo.fromUri(resolver, childUri); activity.onDocumentPicked(childDoc); } catch (Exception e) { - Toast.makeText(context, R.string.save_error, Toast.LENGTH_SHORT).show(); + Toast.makeText(context, R.string.create_error, Toast.LENGTH_SHORT).show(); } } }); diff --git a/packages/DocumentsUI/src/com/android/documentsui/DirectoryFragment.java b/packages/DocumentsUI/src/com/android/documentsui/DirectoryFragment.java index de1f13022bbd6..79ab28d08ec14 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/DirectoryFragment.java +++ b/packages/DocumentsUI/src/com/android/documentsui/DirectoryFragment.java @@ -715,8 +715,16 @@ public class DirectoryFragment extends Fragment { final FrameLayout grid = (FrameLayout) convertView; final int gridPadding = getResources() .getDimensionPixelSize(R.dimen.grid_padding); - grid.setForeground(new InsetDrawable(grid.getForeground(), gridPadding)); - grid.setBackground(new InsetDrawable(grid.getBackground(), gridPadding)); + + // Tricksy hobbitses! We need to fully clear the drawable so + // the view doesn't clobber the new InsetDrawable callback + // when setting back later. + final Drawable fg = grid.getForeground(); + final Drawable bg = grid.getBackground(); + grid.setForeground(null); + grid.setBackground(null); + grid.setForeground(new InsetDrawable(fg, gridPadding)); + grid.setBackground(new InsetDrawable(bg, gridPadding)); } else { throw new IllegalStateException(); } diff --git a/packages/DocumentsUI/src/com/android/documentsui/DocumentsActivity.java b/packages/DocumentsUI/src/com/android/documentsui/DocumentsActivity.java index 6d5475d75ee76..8d55ec4619daf 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/DocumentsActivity.java +++ b/packages/DocumentsUI/src/com/android/documentsui/DocumentsActivity.java @@ -285,6 +285,7 @@ public class DocumentsActivity extends Activity { private class RestoreStackTask extends AsyncTask { private volatile boolean mRestoredStack; + private volatile boolean mExternal; @Override protected Void doInBackground(Void... params) { @@ -298,6 +299,7 @@ public class DocumentsActivity extends Activity { cursor.getColumnIndex(ResumeColumns.STACK)); DurableUtils.readFromArray(rawStack, mState.stack); mRestoredStack = true; + mExternal = cursor.getInt(cursor.getColumnIndex(ResumeColumns.EXTERNAL)) != 0; } } catch (IOException e) { Log.w(TAG, "Failed to resume", e); @@ -305,12 +307,17 @@ public class DocumentsActivity extends Activity { IoUtils.closeQuietly(cursor); } - // If restored root isn't valid, fall back to recents - final RootInfo root = getCurrentRoot(); - final Collection matchingRoots = mRoots.getMatchingRootsBlocking(mState); - if (!matchingRoots.contains(root)) { - mState.stack.reset(); - mRestoredStack = false; + if (mRestoredStack) { + // Update the restored stack to ensure we have freshest data + final Collection matchingRoots = mRoots.getMatchingRootsBlocking(mState); + try { + mState.stack.updateRoot(matchingRoots); + mState.stack.updateDocuments(getContentResolver()); + } catch (FileNotFoundException e) { + Log.w(TAG, "Failed to restore stack: " + e); + mState.stack.reset(); + mRestoredStack = false; + } } return null; @@ -321,10 +328,22 @@ public class DocumentsActivity extends Activity { if (isDestroyed()) return; mState.restored = true; - // Only open drawer when not restoring stack, and when not showing - // visual content. - if (!mRestoredStack - && !MimePredicate.mimeMatches(MimePredicate.VISUAL_MIMES, mState.acceptMimes)) { + // Show drawer when no stack restored, but only when requesting + // non-visual content. However, if we last used an external app, + // drawer is always shown. + + boolean showDrawer = false; + if (!mRestoredStack) { + showDrawer = true; + } + if (MimePredicate.mimeMatches(MimePredicate.VISUAL_MIMES, mState.acceptMimes)) { + showDrawer = false; + } + if (mExternal && mState.action == ACTION_GET_CONTENT) { + showDrawer = true; + } + + if (showDrawer) { setRootsDrawerOpen(true); } @@ -340,6 +359,7 @@ public class DocumentsActivity extends Activity { mState.showSize = true; } else { mState.showSize = SettingsActivity.getDisplayFileSize(this); + invalidateOptionsMenu(); } } @@ -779,10 +799,10 @@ public class DocumentsActivity extends Activity { } else { DirectoryFragment.showRecentsOpen(fm, anim); - // Start recents in relevant mode - final boolean acceptImages = MimePredicate.mimeMatches( - "image/*", mState.acceptMimes); - mState.userMode = acceptImages ? MODE_GRID : MODE_LIST; + // Start recents in grid when requesting visual things + final boolean visualMimes = MimePredicate.mimeMatches( + MimePredicate.VISUAL_MIMES, mState.acceptMimes); + mState.userMode = visualMimes ? MODE_GRID : MODE_LIST; mState.derivedMode = mState.userMode; } } else { @@ -814,9 +834,17 @@ public class DocumentsActivity extends Activity { } public void onStackPicked(DocumentStack stack) { - mState.stack = stack; - mState.stackTouched = true; - onCurrentDirectoryChanged(ANIM_SIDE); + try { + // Update the restored stack to ensure we have freshest data + stack.updateDocuments(getContentResolver()); + + mState.stack = stack; + mState.stackTouched = true; + onCurrentDirectoryChanged(ANIM_SIDE); + + } catch (FileNotFoundException e) { + Log.w(TAG, "Failed to restore stack: " + e); + } } public void onRootPicked(RootInfo root, boolean closeDrawer) { @@ -858,6 +886,14 @@ public class DocumentsActivity extends Activity { // Only relay back results when not canceled; otherwise stick around to // let the user pick another app/backend. if (requestCode == CODE_FORWARD && resultCode != RESULT_CANCELED) { + + // Remember that we last picked via external app + final String packageName = getCallingPackage(); + final ContentValues values = new ContentValues(); + values.put(ResumeColumns.EXTERNAL, 1); + getContentResolver().insert(RecentsProvider.buildResume(packageName), values); + + // Pass back result to original caller setResult(resultCode, data); finish(); } else { @@ -945,6 +981,7 @@ public class DocumentsActivity extends Activity { final String packageName = getCallingPackage(); values.clear(); values.put(ResumeColumns.STACK, rawStack); + values.put(ResumeColumns.EXTERNAL, 0); resolver.insert(RecentsProvider.buildResume(packageName), values); final Intent intent = new Intent(); diff --git a/packages/DocumentsUI/src/com/android/documentsui/RecentsCreateFragment.java b/packages/DocumentsUI/src/com/android/documentsui/RecentsCreateFragment.java index a396f79f1691e..670d5c0a076e4 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/RecentsCreateFragment.java +++ b/packages/DocumentsUI/src/com/android/documentsui/RecentsCreateFragment.java @@ -45,8 +45,10 @@ import android.widget.ImageView; import android.widget.ListView; import android.widget.TextView; +import com.android.documentsui.DocumentsActivity.State; import com.android.documentsui.RecentsProvider.RecentColumns; import com.android.documentsui.model.DocumentStack; +import com.android.documentsui.model.RootInfo; import com.google.android.collect.Lists; import libcore.io.IoUtils; @@ -55,6 +57,7 @@ import java.io.ByteArrayInputStream; import java.io.DataInputStream; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.List; /** @@ -89,10 +92,13 @@ public class RecentsCreateFragment extends Fragment { mAdapter = new DocumentStackAdapter(); mListView.setAdapter(mAdapter); + final RootsCache roots = DocumentsApplication.getRootsCache(context); + final State state = ((DocumentsActivity) getActivity()).getDisplayState(); + mCallbacks = new LoaderCallbacks>() { @Override public Loader> onCreateLoader(int id, Bundle args) { - return new RecentsCreateLoader(context); + return new RecentsCreateLoader(context, roots, state); } @Override @@ -131,12 +137,18 @@ public class RecentsCreateFragment extends Fragment { }; public static class RecentsCreateLoader extends UriDerivativeLoader> { - public RecentsCreateLoader(Context context) { + private final RootsCache mRoots; + private final State mState; + + public RecentsCreateLoader(Context context, RootsCache roots, State state) { super(context, RecentsProvider.buildRecent()); + mRoots = roots; + mState = state; } @Override public List loadInBackground(Uri uri, CancellationSignal signal) { + final Collection matchingRoots = mRoots.getMatchingRootsBlocking(mState); final ArrayList result = Lists.newArrayList(); final ContentResolver resolver = getContext().getContentResolver(); @@ -149,6 +161,12 @@ public class RecentsCreateFragment extends Fragment { try { final DocumentStack stack = new DocumentStack(); stack.read(new DataInputStream(new ByteArrayInputStream(rawStack))); + + // Only update root here to avoid spinning up all + // providers; we update the stack during the actual + // restore. This also filters away roots that don't + // match current filter. + stack.updateRoot(matchingRoots); result.add(stack); } catch (IOException e) { Log.w(TAG, "Failed to resolve stack: " + e); diff --git a/packages/DocumentsUI/src/com/android/documentsui/RecentsProvider.java b/packages/DocumentsUI/src/com/android/documentsui/RecentsProvider.java index af79c93641229..7386cae9b84e7 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/RecentsProvider.java +++ b/packages/DocumentsUI/src/com/android/documentsui/RecentsProvider.java @@ -72,6 +72,7 @@ public class RecentsProvider extends ContentProvider { public static final String PACKAGE_NAME = "package_name"; public static final String STACK = "stack"; public static final String TIMESTAMP = "timestamp"; + public static final String EXTERNAL = "external"; } public static Uri buildRecent() { @@ -97,9 +98,10 @@ public class RecentsProvider extends ContentProvider { private static final int VERSION_INIT = 1; private static final int VERSION_AS_BLOB = 3; + private static final int VERSION_ADD_EXTERNAL = 4; public DatabaseHelper(Context context) { - super(context, DB_NAME, null, VERSION_AS_BLOB); + super(context, DB_NAME, null, VERSION_ADD_EXTERNAL); } @Override @@ -121,9 +123,10 @@ public class RecentsProvider extends ContentProvider { ")"); db.execSQL("CREATE TABLE " + TABLE_RESUME + " (" + - ResumeColumns.PACKAGE_NAME + " TEXT PRIMARY KEY ON CONFLICT REPLACE," + - ResumeColumns.STACK + " BLOB," + - ResumeColumns.TIMESTAMP + " INTEGER" + + ResumeColumns.PACKAGE_NAME + " TEXT NOT NULL PRIMARY KEY," + + ResumeColumns.STACK + " BLOB DEFAULT NULL," + + ResumeColumns.TIMESTAMP + " INTEGER," + + ResumeColumns.EXTERNAL + " INTEGER NOT NULL DEFAULT 0" + ")"); } @@ -176,6 +179,7 @@ public class RecentsProvider extends ContentProvider { @Override public Uri insert(Uri uri, ContentValues values) { final SQLiteDatabase db = mHelper.getWritableDatabase(); + final ContentValues key = new ContentValues(); switch (sMatcher.match(uri)) { case URI_RECENT: values.put(RecentColumns.TIMESTAMP, System.currentTimeMillis()); @@ -188,7 +192,6 @@ public class RecentsProvider extends ContentProvider { final String rootId = uri.getPathSegments().get(2); final String documentId = uri.getPathSegments().get(3); - final ContentValues key = new ContentValues(); key.put(StateColumns.AUTHORITY, authority); key.put(StateColumns.ROOT_ID, rootId); key.put(StateColumns.DOCUMENT_ID, documentId); @@ -201,10 +204,15 @@ public class RecentsProvider extends ContentProvider { return uri; case URI_RESUME: - final String packageName = uri.getPathSegments().get(1); - values.put(ResumeColumns.PACKAGE_NAME, packageName); values.put(ResumeColumns.TIMESTAMP, System.currentTimeMillis()); - db.insert(TABLE_RESUME, null, values); + + final String packageName = uri.getPathSegments().get(1); + key.put(ResumeColumns.PACKAGE_NAME, packageName); + + // Ensure that row exists, then update with changed values + db.insertWithOnConflict(TABLE_RESUME, null, key, SQLiteDatabase.CONFLICT_IGNORE); + db.update(TABLE_RESUME, values, ResumeColumns.PACKAGE_NAME + "=?", + new String[] { packageName }); return uri; default: throw new UnsupportedOperationException("Unsupported Uri " + uri); diff --git a/packages/DocumentsUI/src/com/android/documentsui/TestActivity.java b/packages/DocumentsUI/src/com/android/documentsui/TestActivity.java index 7b7c3d53a0756..1cc35a7d97de2 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/TestActivity.java +++ b/packages/DocumentsUI/src/com/android/documentsui/TestActivity.java @@ -27,6 +27,7 @@ import android.view.View.OnClickListener; import android.widget.Button; import android.widget.CheckBox; import android.widget.LinearLayout; +import android.widget.ScrollView; import android.widget.TextView; import libcore.io.IoUtils; @@ -52,6 +53,9 @@ public class TestActivity extends Activity { final LinearLayout view = new LinearLayout(context); view.setOrientation(LinearLayout.VERTICAL); + mResult = new TextView(context); + view.addView(mResult); + final CheckBox multiple = new CheckBox(context); multiple.setText("ALLOW_MULTIPLE"); view.addView(multiple); @@ -155,6 +159,23 @@ public class TestActivity extends Activity { }); view.addView(button); + button = new Button(context); + button.setText("CREATE_DOC image/png"); + button.setOnClickListener(new OnClickListener() { + @Override + public void onClick(View v) { + Intent intent = new Intent(Intent.ACTION_CREATE_DOCUMENT); + intent.addCategory(Intent.CATEGORY_OPENABLE); + intent.setType("image/png"); + intent.putExtra(Intent.EXTRA_TITLE, "mypicture.png"); + if (localOnly.isChecked()) { + intent.putExtra(Intent.EXTRA_LOCAL_ONLY, true); + } + startActivityForResult(intent, CODE_WRITE); + } + }); + view.addView(button); + button = new Button(context); button.setText("GET_CONTENT */*"); button.setOnClickListener(new OnClickListener() { @@ -174,10 +195,10 @@ public class TestActivity extends Activity { }); view.addView(button); - mResult = new TextView(context); - view.addView(mResult); + final ScrollView scroll = new ScrollView(context); + scroll.addView(view); - setContentView(view); + setContentView(scroll); } @Override diff --git a/packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java b/packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java index 08a8c1366f84c..5091a612365ac 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java +++ b/packages/DocumentsUI/src/com/android/documentsui/model/DocumentInfo.java @@ -23,6 +23,7 @@ import android.net.Uri; import android.os.Parcel; import android.os.Parcelable; import android.provider.DocumentsContract; +import android.provider.DocumentsProvider; import android.provider.DocumentsContract.Document; import com.android.documentsui.RootCursorWrapper; @@ -141,23 +142,42 @@ public class DocumentInfo implements Durable, Parcelable { } public static DocumentInfo fromCursor(Cursor cursor, String authority) { - final DocumentInfo doc = new DocumentInfo(); - doc.authority = authority; - doc.documentId = getCursorString(cursor, Document.COLUMN_DOCUMENT_ID); - doc.mimeType = getCursorString(cursor, Document.COLUMN_MIME_TYPE); - doc.documentId = getCursorString(cursor, Document.COLUMN_DOCUMENT_ID); - doc.mimeType = getCursorString(cursor, Document.COLUMN_MIME_TYPE); - doc.displayName = getCursorString(cursor, Document.COLUMN_DISPLAY_NAME); - doc.lastModified = getCursorLong(cursor, Document.COLUMN_LAST_MODIFIED); - doc.flags = getCursorInt(cursor, Document.COLUMN_FLAGS); - doc.summary = getCursorString(cursor, Document.COLUMN_SUMMARY); - doc.size = getCursorLong(cursor, Document.COLUMN_SIZE); - doc.icon = getCursorInt(cursor, Document.COLUMN_ICON); - doc.deriveFields(); - return doc; + final DocumentInfo info = new DocumentInfo(); + info.updateFromCursor(cursor, authority); + return info; } - public static DocumentInfo fromUri(ContentResolver resolver, Uri uri) throws FileNotFoundException { + public void updateFromCursor(Cursor cursor, String authority) { + this.authority = authority; + this.documentId = getCursorString(cursor, Document.COLUMN_DOCUMENT_ID); + this.mimeType = getCursorString(cursor, Document.COLUMN_MIME_TYPE); + this.documentId = getCursorString(cursor, Document.COLUMN_DOCUMENT_ID); + this.mimeType = getCursorString(cursor, Document.COLUMN_MIME_TYPE); + this.displayName = getCursorString(cursor, Document.COLUMN_DISPLAY_NAME); + this.lastModified = getCursorLong(cursor, Document.COLUMN_LAST_MODIFIED); + this.flags = getCursorInt(cursor, Document.COLUMN_FLAGS); + this.summary = getCursorString(cursor, Document.COLUMN_SUMMARY); + this.size = getCursorLong(cursor, Document.COLUMN_SIZE); + this.icon = getCursorInt(cursor, Document.COLUMN_ICON); + this.deriveFields(); + } + + public static DocumentInfo fromUri(ContentResolver resolver, Uri uri) + throws FileNotFoundException { + final DocumentInfo info = new DocumentInfo(); + info.updateFromUri(resolver, uri); + return info; + } + + /** + * Update a possibly stale restored document against a live + * {@link DocumentsProvider}. + */ + public void updateSelf(ContentResolver resolver) throws FileNotFoundException { + updateFromUri(resolver, derivedUri); + } + + public void updateFromUri(ContentResolver resolver, Uri uri) throws FileNotFoundException { final ContentProviderClient client = resolver.acquireUnstableContentProviderClient( uri.getAuthority()); Cursor cursor = null; @@ -166,7 +186,7 @@ public class DocumentInfo implements Durable, Parcelable { if (!cursor.moveToFirst()) { throw new FileNotFoundException("Missing details for " + uri); } - return fromCursor(cursor, uri.getAuthority()); + updateFromCursor(cursor, uri.getAuthority()); } catch (Throwable t) { throw asFileNotFoundException(t); } finally { diff --git a/packages/DocumentsUI/src/com/android/documentsui/model/DocumentStack.java b/packages/DocumentsUI/src/com/android/documentsui/model/DocumentStack.java index 254144065f8a2..0a378c07532a7 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/model/DocumentStack.java +++ b/packages/DocumentsUI/src/com/android/documentsui/model/DocumentStack.java @@ -16,10 +16,15 @@ package com.android.documentsui.model; +import android.content.ContentResolver; +import android.provider.DocumentsProvider; + import java.io.DataInputStream; import java.io.DataOutputStream; +import java.io.FileNotFoundException; import java.io.IOException; import java.net.ProtocolException; +import java.util.Collection; import java.util.LinkedList; /** @@ -46,6 +51,26 @@ public class DocumentStack extends LinkedList implements Durable { return size() == 0; } + public void updateRoot(Collection matchingRoots) throws FileNotFoundException { + for (RootInfo root : matchingRoots) { + if (root.equals(this.root)) { + this.root = root; + return; + } + } + throw new FileNotFoundException("Failed to find matching root for " + root); + } + + /** + * Update a possibly stale restored stack against a live + * {@link DocumentsProvider}. + */ + public void updateDocuments(ContentResolver resolver) throws FileNotFoundException { + for (DocumentInfo info : this) { + info.updateSelf(resolver); + } + } + @Override public void reset() { clear();