From bed703ef28f013639db3cd7a6b8a6e94d61075da Mon Sep 17 00:00:00 2001 From: Chris Wren Date: Tue, 8 Jan 2013 15:25:23 -0500 Subject: defensive loop guards around cursors. I did not realize that an error in moveToNext() would result in a curor that isBeforeFirst() but not isAfterLast(). Bug: 7465164 Change-Id: I5502f18e863ad48ec88397fdf0b4ec457be723dc --- src/com/android/dreams/phototable/LocalSource.java | 29 ++++++++-------- src/com/android/dreams/phototable/PhotoSource.java | 11 ++++++ .../android/dreams/phototable/PicasaSource.java | 39 ++++++++++------------ 3 files changed, 41 insertions(+), 38 deletions(-) diff --git a/src/com/android/dreams/phototable/LocalSource.java b/src/com/android/dreams/phototable/LocalSource.java index 1faf589..4efc43b 100644 --- a/src/com/android/dreams/phototable/LocalSource.java +++ b/src/com/android/dreams/phototable/LocalSource.java @@ -36,14 +36,14 @@ public class LocalSource extends PhotoSource { private final String mUnknownAlbumName; private final String mLocalSourceName; private Set mFoundAlbumIds; - private int mNextPosition; + private int mLastPosition; public LocalSource(Context context, SharedPreferences settings) { super(context, settings); mLocalSourceName = mResources.getString(R.string.local_source_name, "Photos on Device"); mUnknownAlbumName = mResources.getString(R.string.unknown_album_name, "Unknown"); mSourceName = TAG; - mNextPosition = -1; + mLastPosition = INVALID; fillQueue(); } @@ -65,7 +65,7 @@ public class LocalSource extends PhotoSource { Cursor cursor = mResolver.query(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, projection, null, null, null); if (cursor != null) { - cursor.moveToFirst(); + cursor.moveToPosition(-1); int dataIndex = cursor.getColumnIndex(MediaStore.Images.Media.DATA); int bucketIndex = cursor.getColumnIndex(MediaStore.Images.Media.BUCKET_ID); @@ -75,7 +75,7 @@ public class LocalSource extends PhotoSource { if (bucketIndex < 0) { log(TAG, "can't find the ID column!"); } else { - while (!cursor.isAfterLast()) { + while (cursor.moveToNext()) { String id = TAG + ":" + cursor.getString(bucketIndex); AlbumData data = foundAlbums.get(id); if (foundAlbums.get(id) == null) { @@ -102,7 +102,6 @@ public class LocalSource extends PhotoSource { updated : Math.min(data.updated, updated)); } - cursor.moveToNext(); } } cursor.close(); @@ -139,13 +138,10 @@ public class LocalSource extends PhotoSource { Cursor cursor = mResolver.query(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, projection, selection, null, null); if (cursor != null) { - if (cursor.getCount() > howMany && mNextPosition == -1) { - mNextPosition = mRNG.nextInt() % (cursor.getCount() - howMany); + if (cursor.getCount() > howMany && mLastPosition == INVALID) { + mLastPosition = pickRandomStart(cursor.getCount(), howMany); } - if (mNextPosition == -1) { - mNextPosition = 0; - } - cursor.moveToPosition(mNextPosition); + cursor.moveToPosition(mLastPosition); int dataIndex = cursor.getColumnIndex(MediaStore.Images.Media.DATA); int orientationIndex = cursor.getColumnIndex(MediaStore.Images.Media.ORIENTATION); @@ -155,17 +151,18 @@ public class LocalSource extends PhotoSource { if (dataIndex < 0) { log(TAG, "can't find the DATA column!"); } else { - while (foundImages.size() < howMany && !cursor.isAfterLast()) { + while (foundImages.size() < howMany && cursor.moveToNext()) { ImageData data = new ImageData(); data.url = cursor.getString(dataIndex); data.orientation = cursor.getInt(orientationIndex); foundImages.offer(data); - if (cursor.moveToNext()) { - mNextPosition++; - } + mLastPosition = cursor.getPosition(); } if (cursor.isAfterLast()) { - mNextPosition = 0; + mLastPosition = -1; + } + if (cursor.isBeforeFirst()) { + mLastPosition = INVALID; } } diff --git a/src/com/android/dreams/phototable/PhotoSource.java b/src/com/android/dreams/phototable/PhotoSource.java index 670bd02..32d41c7 100644 --- a/src/com/android/dreams/phototable/PhotoSource.java +++ b/src/com/android/dreams/phototable/PhotoSource.java @@ -43,6 +43,9 @@ public abstract class PhotoSource { private static final String TAG = "PhotoTable.PhotoSource"; private static final boolean DEBUG = false; + // An invalid cursor position to represent the uninitialized state. + protected static final int INVALID = -2; + // This should be large enough for BitmapFactory to decode the header so // that we can mark and reset the input stream to avoid duplicate network i/o private static final int BUFFER_SIZE = 128 * 1024; @@ -248,6 +251,14 @@ public abstract class PhotoSource { } } + protected int pickRandomStart(int total, int max) { + if (max >= total) { + return -1; + } else { + return (mRNG.nextInt() % (total - max)) - 1; + } + } + protected abstract InputStream getStream(ImageData data, int longSide); protected abstract Collection findImages(int howMany); public abstract Collection findAlbums(); diff --git a/src/com/android/dreams/phototable/PicasaSource.java b/src/com/android/dreams/phototable/PicasaSource.java index 9513d3c..ef4a7c4 100644 --- a/src/com/android/dreams/phototable/PicasaSource.java +++ b/src/com/android/dreams/phototable/PicasaSource.java @@ -75,13 +75,13 @@ public class PicasaSource extends PhotoSource { private final int mMaxRecycleSize; private Set mFoundAlbumIds; - private int mNextPosition; + private int mLastPosition; private int mDisplayLongSide; public PicasaSource(Context context, SharedPreferences settings) { super(context, settings); mSourceName = TAG; - mNextPosition = -1; + mLastPosition = INVALID; mMaxPostAblums = mResources.getInteger(R.integer.max_post_albums); mPostsAlbumName = mResources.getString(R.string.posts_album_name, "Posts"); mUploadsAlbumName = mResources.getString(R.string.uploads_album_name, "Instant Uploads"); @@ -162,15 +162,12 @@ public class PicasaSource extends PhotoSource { Cursor cursor = mResolver.query(picasaUriBuilder.build(), projection, selection.toString(), null, null); if (cursor != null) { - if (cursor.getCount() > howMany && mNextPosition == -1) { - mNextPosition = - (int) Math.abs(mRNG.nextInt() % (cursor.getCount() - howMany)); + if (cursor.getCount() > howMany && mLastPosition == INVALID) { + mLastPosition = pickRandomStart(cursor.getCount(), howMany); } - if (mNextPosition == -1) { - mNextPosition = 0; - } - log(TAG, "moving to position: " + mNextPosition); - cursor.moveToPosition(mNextPosition); + + log(TAG, "moving to position: " + mLastPosition); + cursor.moveToPosition(mLastPosition); int idIndex = cursor.getColumnIndex(PICASA_ID); int urlIndex = cursor.getColumnIndex(PICASA_URL); @@ -180,7 +177,7 @@ public class PicasaSource extends PhotoSource { if (idIndex < 0) { log(TAG, "can't find the ID column!"); } else { - while (foundImages.size() < howMany && !cursor.isAfterLast()) { + while (cursor.moveToNext()) { if (idIndex >= 0) { ImageData data = new ImageData(); data.id = cursor.getString(idIndex); @@ -191,12 +188,13 @@ public class PicasaSource extends PhotoSource { foundImages.offer(data); } - if (cursor.moveToNext()) { - mNextPosition++; - } + mLastPosition = cursor.getPosition(); } if (cursor.isAfterLast()) { - mNextPosition = 0; + mLastPosition = -1; + } + if (cursor.isBeforeFirst()) { + mLastPosition = INVALID; } } @@ -254,7 +252,7 @@ public class PicasaSource extends PhotoSource { projection, selection, null, order); if (cursor != null) { log(TAG, " " + id + " resolved to " + cursor.getCount() + " albums"); - cursor.moveToFirst(); + cursor.moveToPosition(-1); int idIndex = cursor.getColumnIndex(PICASA_ID); int typeIndex = cursor.getColumnIndex(PICASA_ALBUM_TYPE); @@ -262,9 +260,8 @@ public class PicasaSource extends PhotoSource { if (idIndex < 0) { log(TAG, "can't find the ID column!"); } else { - while (!cursor.isAfterLast()) { + while (cursor.moveToNext()) { albumIds.add(cursor.getString(idIndex)); - cursor.moveToNext(); } } cursor.close(); @@ -296,7 +293,7 @@ public class PicasaSource extends PhotoSource { Cursor cursor = mResolver.query(picasaUriBuilder.build(), projection, null, null, null); if (cursor != null) { - cursor.moveToFirst(); + cursor.moveToPosition(-1); int idIndex = cursor.getColumnIndex(PICASA_ID); int thumbIndex = cursor.getColumnIndex(PICASA_THUMB); @@ -308,7 +305,7 @@ public class PicasaSource extends PhotoSource { if (idIndex < 0) { log(TAG, "can't find the ID column!"); } else { - while (!cursor.isAfterLast()) { + while (cursor.moveToNext()) { String id = TAG + ":" + cursor.getString(idIndex); String user = (userIndex >= 0 ? cursor.getString(userIndex) : "-1"); String type = (typeIndex >= 0 ? cursor.getString(typeIndex) : "none"); @@ -367,8 +364,6 @@ public class PicasaSource extends PhotoSource { if (data.thumbnailUrl == null || data.updated == updated) { data.thumbnailUrl = thumbnailUrl; } - - cursor.moveToNext(); } } cursor.close(); -- cgit v1.2.3