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

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<String> 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<ImageData> findImages(int howMany);
     public abstract Collection<AlbumData> 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<String> 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();