Fixes for a plethora of crashes

app/src/main/java/com/quran/labs/androidquran/QuranDataActivity.kt

@@ -631,9 +631,16 @@ class QuranDataActivity : Activity(), SimpleDownloadListener, OnRequestPermissio
     if (downloadReceiver != null && downloadReceiver!!.didReceiveBroadcast() && !force) {
       return
     }
+
     val dataStatus = quranDataStatus
+    if (dataStatus == null) {
+      // we lost the cached data status, so just check again
+      checkPages()
+      return
+    }
+
     var url: String
-    url = if (dataStatus!!.needPortrait() && !dataStatus.needLandscape()) {
+    url = if (dataStatus.needPortrait() && !dataStatus.needLandscape()) {
       // phone (and tablet when upgrading on some devices, ex n10)
       quranFileUtils.zipFileUrl
     } else if (dataStatus.needLandscape() && !dataStatus.needPortrait()) {

app/src/main/java/com/quran/labs/androidquran/pageselect/PageSelectPresenter.kt

@@ -101,14 +101,33 @@ constructor(
       val updatedBookmarks = bookmarksDao.bookmarks()
         .map {
           val page = it.page
-          val (pageSura, pageAyah) = suraAyahFromPage(page)
-          val sura = it.sura ?: pageSura
-          val ayah = it.ayah ?: pageAyah
+          if (page - 1 >= sourcePageSuraStart.size) {
+            if (it.isPageBookmark()) {
+              // this bookmark is on a page that doesn't exist in the old page type
+              if (destination.suraForPageArray.size > page) {
+                // but it does exist on the new type, so it's ok, let's not re-map
+                it
+              } else {
+                // we can't map it, so let's just put it as the max page number to avoid bad data
+                it.copy(page = sourcePageAyahStart.size - 1)
+              }
+            } else {
+              // ayah bookmark, so let's just map it
+              val sura = requireNotNull(it.sura)
+              val ayah = requireNotNull(it.ayah)
+              val mappedPage = destinationQuranInfo.getPageFromSuraAyah(sura, ayah)
+              it.copy(page = mappedPage)
+            }
+          } else {
+            val (pageSura, pageAyah) = suraAyahFromPage(page)
+            val sura = it.sura ?: pageSura
+            val ayah = it.ayah ?: pageAyah
 
-          val mappedPage = destinationQuranInfo.getPageFromSuraAyah(sura, ayah)
+            val mappedPage = destinationQuranInfo.getPageFromSuraAyah(sura, ayah)
 
-          // we only copy the page because sura and ayah are the same.
-          it.copy(page = mappedPage)
+            // we only copy the page because sura and ayah are the same.
+            it.copy(page = mappedPage)
+          }
         }
 
       if (updatedBookmarks.isNotEmpty()) {

app/src/main/java/com/quran/labs/androidquran/service/AudioService.kt

@@ -221,6 +221,13 @@ class AudioService : Service(), OnCompletionListener, OnPreparedListener,
       localPlayer.setOnCompletionListener(this)
       localPlayer.setOnErrorListener(this)
       localPlayer.setOnSeekCompleteListener(this)
+
+      val audioAttributes = AudioAttributes.Builder()
+        .setContentType(AudioAttributes.CONTENT_TYPE_MUSIC)
+        .setUsage(AudioAttributes.USAGE_MEDIA)
+        .build()
+      localPlayer.setAudioAttributes(audioAttributes)
+
       mediaSession.isActive = true
       localPlayer
     } else {
@@ -979,12 +986,6 @@ class AudioService : Service(), OnCompletionListener, OnPreparedListener,
       state = State.Preparing
       updateAudioPlaybackStatus()
 
-      val audioAttributes = AudioAttributes.Builder()
-        .setContentType(AudioAttributes.CONTENT_TYPE_MUSIC)
-        .setUsage(AudioAttributes.USAGE_MEDIA)
-        .build()
-      localPlayer.setAudioAttributes(audioAttributes)
-
       // starts preparing the media player in the background. When it's
       // done, it will call our OnPreparedListener (that is, the
       // onPrepared() method on this class, since we set the listener
@@ -1018,15 +1019,47 @@ class AudioService : Service(), OnCompletionListener, OnPreparedListener,
     }
     val builder = PlaybackStateCompat.Builder()
     builder.setState(state, position, 1.0f)
-    builder.setActions(
-      PlaybackStateCompat.ACTION_PLAY or
-          PlaybackStateCompat.ACTION_STOP or
-          PlaybackStateCompat.ACTION_REWIND or
-          PlaybackStateCompat.ACTION_FAST_FORWARD or
-          PlaybackStateCompat.ACTION_PAUSE or
-          PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or
-          PlaybackStateCompat.ACTION_SKIP_TO_NEXT
-    )
+
+    val actions = when (state) {
+      PlaybackStateCompat.STATE_PLAYING -> {
+        PlaybackStateCompat.ACTION_PAUSE or
+            PlaybackStateCompat.ACTION_STOP or
+            PlaybackStateCompat.ACTION_REWIND or
+            PlaybackStateCompat.ACTION_FAST_FORWARD or
+            PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or
+            PlaybackStateCompat.ACTION_SKIP_TO_NEXT
+      }
+      PlaybackStateCompat.STATE_PAUSED -> {
+        PlaybackStateCompat.ACTION_PLAY or
+            PlaybackStateCompat.ACTION_STOP or
+            PlaybackStateCompat.ACTION_REWIND or
+            PlaybackStateCompat.ACTION_FAST_FORWARD or
+            PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or
+            PlaybackStateCompat.ACTION_SKIP_TO_NEXT
+      }
+      PlaybackStateCompat.STATE_STOPPED -> {
+        PlaybackStateCompat.ACTION_PLAY
+      }
+      PlaybackStateCompat.STATE_CONNECTING -> {
+        PlaybackStateCompat.ACTION_STOP
+      }
+      PlaybackStateCompat.STATE_REWINDING -> {
+        PlaybackStateCompat.ACTION_STOP or
+            PlaybackStateCompat.ACTION_REWIND or
+            PlaybackStateCompat.ACTION_FAST_FORWARD or
+            PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or
+            PlaybackStateCompat.ACTION_SKIP_TO_NEXT
+      }
+      PlaybackStateCompat.STATE_SKIPPING_TO_NEXT -> {
+        PlaybackStateCompat.ACTION_STOP or
+            PlaybackStateCompat.ACTION_SKIP_TO_PREVIOUS or
+            PlaybackStateCompat.ACTION_SKIP_TO_NEXT or
+            PlaybackStateCompat.ACTION_FAST_FORWARD or
+            PlaybackStateCompat.ACTION_REWIND
+      }
+      else -> { PlaybackStateCompat.ACTION_STOP }
+    }
+    builder.setActions(actions)
     mediaSession.setPlaybackState(builder.build())
   }
 

app/src/main/java/com/quran/labs/androidquran/ui/PagerActivity.java

@@ -759,8 +759,12 @@ public void onResume() {
       foregroundDisposable.add(Completable.timer(500, TimeUnit.MILLISECONDS)
           .observeOn(AndroidSchedulers.mainThread())
           .subscribe(() -> {
-            startService(
-                audioUtils.getAudioIntent(PagerActivity.this, AudioService.ACTION_CONNECT));
+            try {
+              startService(
+                  audioUtils.getAudioIntent(PagerActivity.this, AudioService.ACTION_CONNECT));
+            } catch (IllegalStateException ise) {
+              // we're likely in the background, so ignore.
+            }
             shouldReconnect = false;
           }));
     }

app/src/main/java/com/quran/labs/androidquran/view/AudioStatusBar.java

@@ -261,8 +261,9 @@ private void showStoppedMode() {
 
   private void updateButton() {
     final TextView currentQariView = qariView;
-    if (currentQariView != null && currentQari != null) {
-      currentQariView.setText(currentQari.getNameResource());
+    final Qari qari = currentQari;
+    if (currentQariView != null && qari != null) {
+      currentQariView.setText(qari.getNameResource());
     }
   }
 

app/src/main/java/com/quran/labs/androidquran/view/CurrentQariBridge.kt

@@ -10,20 +10,20 @@ import kotlinx.coroutines.launch
 import javax.inject.Inject
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.flowOn
+import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.observeOn
+import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.withContext
 
 class CurrentQariBridge @Inject constructor(private val currentQariManager: CurrentQariManager) {
   private val scope: CoroutineScope = CoroutineScope(SupervisorJob())
 
   fun listenToQaris(lambda: ((Qari) -> Unit)) {
-    scope.launch {
-      withContext(Dispatchers.Main) {
-        currentQariManager
-          .flow()
-          .collect { lambda(it) }
-      }
-    }
+    currentQariManager
+      .flow()
+      .onEach { lambda(it) }
+      .flowOn(Dispatchers.Main)
+      .launchIn(scope)
   }
 
   fun unsubscribeAll() {

app/src/main/java/com/quran/labs/androidquran/view/HighlightingImageView.java

@@ -1,5 +1,11 @@
 package com.quran.labs.androidquran.view;
 
+import static com.quran.data.model.highlight.HighlightType.Mode.BACKGROUND;
+import static com.quran.data.model.highlight.HighlightType.Mode.COLOR;
+import static com.quran.data.model.highlight.HighlightType.Mode.HIDE;
+import static com.quran.data.model.highlight.HighlightType.Mode.HIGHLIGHT;
+import static com.quran.data.model.highlight.HighlightType.Mode.UNDERLINE;
+
 import android.animation.Animator;
 import android.animation.ValueAnimator;
 import android.content.Context;
@@ -15,6 +21,11 @@
 import android.graphics.drawable.Drawable;
 import android.util.AttributeSet;
 
+import androidx.annotation.NonNull;
+import androidx.appcompat.widget.AppCompatImageView;
+import androidx.core.content.ContextCompat;
+import androidx.core.view.DisplayCutoutCompat;
+
 import com.quran.data.model.highlight.HighlightType;
 import com.quran.labs.androidquran.R;
 import com.quran.labs.androidquran.data.Constants;
@@ -37,19 +48,8 @@
 import java.util.SortedMap;
 import java.util.TreeMap;
 
-import androidx.annotation.NonNull;
-import androidx.appcompat.widget.AppCompatImageView;
-import androidx.core.content.ContextCompat;
-import androidx.core.view.DisplayCutoutCompat;
 import dev.chrisbanes.insetter.Insetter;
 
-import static com.quran.data.model.highlight.HighlightType.Mode.BACKGROUND;
-import static com.quran.data.model.highlight.HighlightType.Mode.COLOR;
-import static com.quran.data.model.highlight.HighlightType.Mode.HIDE;
-import static com.quran.data.model.highlight.HighlightType.Mode.HIGHLIGHT;
-import static com.quran.data.model.highlight.HighlightType.Mode.UNDERLINE;
-import java.lang.Math;
-
 public class HighlightingImageView extends AppCompatImageView {
   // for debugging / visualizing glyph bounds:
   // when enabled, will draw bounds around each glyph to visualize the glyph bounds
@@ -276,6 +276,10 @@ private void highlightFloatableAyah(Set<AyahHighlight> highlights, AyahHighlight
 
     final TransitionAyahHighlight transitionHighlight = new TransitionAyahHighlight(sourceHighlight, destinationHighlight);
 
+    if (startingBounds == null) {
+      startingBounds = new ArrayList<>();
+    }
+
     // yes we make copies, because normalizing the bounds will change them
     List<AyahBounds> sourceBounds = new ArrayList<>(startingBounds);
 

common/bookmark/src/main/java/com/quran/mobile/bookmark/model/BookmarkModel.kt

@@ -2,7 +2,6 @@ package com.quran.mobile.bookmark.model
 
 import app.cash.sqldelight.coroutines.asFlow
 import app.cash.sqldelight.coroutines.mapToList
-import app.cash.sqldelight.coroutines.mapToOneOrNull
 import com.quran.data.model.SuraAyah
 import com.quran.data.model.bookmark.Bookmark
 import com.quran.labs.androidquran.BookmarksDatabase
@@ -21,10 +20,12 @@ class BookmarkModel @Inject constructor(bookmarksDatabase: BookmarksDatabase) {
       .mapToList(Dispatchers.IO)
 
   suspend fun isSuraAyahBookmarked(suraAyah: SuraAyah): Pair<SuraAyah, Boolean> {
-    val bookmarkId = bookmarkQueries.getBookmarkIdForSuraAyah(suraAyah.sura, suraAyah.ayah)
+    val bookmarkIds = bookmarkQueries.getBookmarkIdForSuraAyah(suraAyah.sura, suraAyah.ayah)
       .asFlow()
-      .mapToOneOrNull(Dispatchers.IO)
+      // was .mapToOneOrNull, but some people have multiple bookmarks for the same ayah
+      // should try to figure out why at some point or otherwise de-duplicate them
+      .mapToList(Dispatchers.IO)
       .first()
-    return suraAyah to (bookmarkId != null)
+    return suraAyah to bookmarkIds.isNotEmpty()
   }
 }