Merge pull request #1481 from bemusementpark/s1889

[SES-1889] Fix IndexOutOfBounds in MediaPreview

app/src/main/java/org/thoughtcrime/securesms/MediaPreviewActivity.java

@@ -21,6 +21,7 @@ import android.annotation.TargetApi;
 import android.content.Context;
 import android.content.Intent;
 import android.database.Cursor;
+import android.database.CursorIndexOutOfBoundsException;
 import android.net.Uri;
 import android.os.AsyncTask;
 import android.os.Build;
@@ -145,6 +146,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
                       View.SYSTEM_UI_FLAG_HIDE_NAVIGATION);
     }
   };
+  private MediaItemAdapter adapter;
 
   public static Intent getPreviewIntent(Context context, MediaPreviewArgs args) {
     return getPreviewIntent(context, args.getSlide(), args.getMmsRecord(), args.getThread());
@@ -217,13 +219,6 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
     Permissions.onRequestPermissionsResult(this, requestCode, permissions, grantResults);
   }
 
-  @TargetApi(VERSION_CODES.JELLY_BEAN)
-  private void setFullscreenIfPossible() {
-    if (VERSION.SDK_INT >= VERSION_CODES.JELLY_BEAN) {
-      getWindow().getDecorView().setSystemUiVisibility(View.SYSTEM_UI_FLAG_FULLSCREEN);
-    }
-  }
-
   @Override
   public void onModified(Recipient recipient) {
     Util.runOnMain(this::updateActionBar);
@@ -285,9 +280,6 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
     mediaPager = findViewById(R.id.media_pager);
     mediaPager.setOffscreenPageLimit(1);
 
-    viewPagerListener = new ViewPagerListener();
-    mediaPager.addOnPageChangeListener(viewPagerListener);
-
     albumRail        = findViewById(R.id.media_preview_album_rail);
     albumRailAdapter = new MediaRailAdapter(GlideApp.with(this), this, false);
 
@@ -378,7 +370,8 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
     if (conversationRecipient != null) {
       getSupportLoaderManager().restartLoader(0, null, this);
     } else {
-      mediaPager.setAdapter(new SingleItemPagerAdapter(this, GlideApp.with(this), getWindow(), initialMediaUri, initialMediaType, initialMediaSize));
+      adapter = new SingleItemPagerAdapter(this, GlideApp.with(this), getWindow(), initialMediaUri, initialMediaType, initialMediaSize);
+      mediaPager.setAdapter(adapter);
 
       if (initialCaption != null) {
         detailsContainer.setVisibility(View.VISIBLE);
@@ -506,13 +499,8 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
   }
 
   private @Nullable MediaItem getCurrentMediaItem() {
-    MediaItemAdapter adapter = (MediaItemAdapter)mediaPager.getAdapter();
+    if (adapter == null) return null;
-
+    return adapter.getMediaItemFor(mediaPager.getCurrentItem());
-    if (adapter != null) {
-      return adapter.getMediaItemFor(mediaPager.getCurrentItem());
-    } else {
-      return null;
-    }
   }
 
   public static boolean isContentTypeSupported(final String contentType) {
@@ -526,23 +514,28 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
 
   @Override
   public void onLoadFinished(@NonNull Loader<Pair<Cursor, Integer>> loader, @Nullable Pair<Cursor, Integer> data) {
-    if (data != null) {
+    if (data == null) return;
-      CursorPagerAdapter adapter = new CursorPagerAdapter(this, GlideApp.with(this), getWindow(), data.first, data.second, leftIsRecent);
-      mediaPager.setAdapter(adapter);
-      adapter.setActive(true);
 
-      viewModel.setCursor(this, data.first, leftIsRecent);
+    mediaPager.removeOnPageChangeListener(viewPagerListener);
 
-      if (restartItem >= 0 || data.second >= 0) {
+    adapter = new CursorPagerAdapter(this, GlideApp.with(this), getWindow(), data.first, data.second, leftIsRecent);
-        int item = restartItem >= 0 ? restartItem : data.second;
+    mediaPager.setAdapter(adapter);
-        mediaPager.setCurrentItem(item);
 
-        if (item == 0) {
+    viewModel.setCursor(this, data.first, leftIsRecent);
-          viewPagerListener.onPageSelected(0);
+
-        }
+    int item = restartItem >= 0  && restartItem < adapter.getCount() ? restartItem : Math.max(Math.min(data.second, adapter.getCount() - 1), 0);
-      } else {
+
-        Log.w(TAG, "one of restartItem "+restartItem+" and data.second "+data.second+" would cause OOB exception");
+    viewPagerListener = new ViewPagerListener();
-      }
+    mediaPager.addOnPageChangeListener(viewPagerListener);
+
+    try {
+      mediaPager.setCurrentItem(item);
+    } catch (CursorIndexOutOfBoundsException e) {
+      throw new RuntimeException("restartItem = " + restartItem + ", data.second = " + data.second + " leftIsRecent = " + leftIsRecent, e);
+    }
+
+    if (item == 0) {
+      viewPagerListener.onPageSelected(0);
     }
   }
 
@@ -560,26 +553,26 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
       if (currentPage != -1 && currentPage != position) onPageUnselected(currentPage);
       currentPage = position;
 
-      MediaItemAdapter adapter = (MediaItemAdapter)mediaPager.getAdapter();
+      if (adapter == null) return;
 
-      if (adapter != null) {
+      MediaItem item = adapter.getMediaItemFor(position);
-        MediaItem item = adapter.getMediaItemFor(position);
+      if (item.recipient != null) item.recipient.addListener(MediaPreviewActivity.this);
-        if (item.recipient != null) item.recipient.addListener(MediaPreviewActivity.this);
+      viewModel.setActiveAlbumRailItem(MediaPreviewActivity.this, position);
-        viewModel.setActiveAlbumRailItem(MediaPreviewActivity.this, position);
+      updateActionBar();
-        updateActionBar();
-      }
     }
 
 
     public void onPageUnselected(int position) {
-      MediaItemAdapter adapter = (MediaItemAdapter)mediaPager.getAdapter();
+      if (adapter == null) return;
 
-      if (adapter != null) {
+      try {
         MediaItem item = adapter.getMediaItemFor(position);
         if (item.recipient != null) item.recipient.removeListener(MediaPreviewActivity.this);
-
+      } catch (CursorIndexOutOfBoundsException e) {
-        adapter.pause(position);
+        throw new RuntimeException("position = " + position + " leftIsRecent = " + leftIsRecent, e);
       }
+
+      adapter.pause(position);
     }
 
     @Override
@@ -593,7 +586,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
     }
   }
 
-  private static class SingleItemPagerAdapter extends PagerAdapter implements MediaItemAdapter {
+  private static class SingleItemPagerAdapter extends MediaItemAdapter {
 
     private final GlideRequests glideRequests;
     private final Window        window;
@@ -665,7 +658,7 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
     }
   }
 
-  private static class CursorPagerAdapter extends PagerAdapter implements MediaItemAdapter {
+  private static class CursorPagerAdapter extends MediaItemAdapter {
 
     private final WeakHashMap<Integer, MediaView> mediaViews = new WeakHashMap<>();
 
@@ -675,7 +668,6 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
     private final Cursor        cursor;
     private final boolean       leftIsRecent;
 
-    private boolean active;
     private int     autoPlayPosition;
 
     CursorPagerAdapter(@NonNull Context context, @NonNull GlideRequests glideRequests,
@@ -690,15 +682,9 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
       this.leftIsRecent     = leftIsRecent;
     }
 
-    public void setActive(boolean active) {
-      this.active = active;
-      notifyDataSetChanged();
-    }
-
     @Override
     public int getCount() {
-      if (!active) return 0;
+      return cursor.getCount();
-      else         return cursor.getCount();
     }
 
     @Override
@@ -771,8 +757,8 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
     }
 
     private int getCursorPosition(int position) {
-      if (leftIsRecent) return position;
+        int unclamped = leftIsRecent ? position : cursor.getCount() - 1 - position;
-      else              return cursor.getCount() - 1 - position;
+        return Math.max(Math.min(unclamped, cursor.getCount() - 1), 0);
     }
   }
 
@@ -800,9 +786,9 @@ public class MediaPreviewActivity extends PassphraseRequiredActionBarActivity im
     }
   }
 
-  interface MediaItemAdapter {
+  abstract static class MediaItemAdapter extends PagerAdapter {
-    MediaItem getMediaItemFor(int position);
+    abstract MediaItem getMediaItemFor(int position);
-    void pause(int position);
+    abstract void pause(int position);
-    @Nullable View getPlaybackControls(int position);
+    @Nullable abstract View getPlaybackControls(int position);
   }
 }