Handle errors in all VideoAdPlayer callbacks

Some but not all VideoAdPlayer callbacks from the IMA SDK included defensive handling of unexpected cases. Add the remaining ones. Issue: #7492 PiperOrigin-RevId: 316082651
google · Jun 12, 2020 · 5a88e0b · 5a88e0b
1 parent c5144dc
commit 5a88e0b
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 29 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -208,7 +208,10 @@
     ([#6922](https://github.com/google/ExoPlayer/pull/6922)).
 *   Cast extension: Implement playlist API and deprecate the old queue
     manipulation API.
-*   IMA extension: Add option to skip ads before the start position.
+*   IMA extension:
+    *   Add option to skip ads before the start position.
+    *   Catch unexpected errors in `stopAd` to avoid a crash
+        ([#7492](https://github.com/google/ExoPlayer/issues/7492)).
 *   Demo app: Retain previous position in list of samples.
 *   Add Guava dependency.
 

diff --git a/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java b/extensions/ima/src/main/java/com/google/android/exoplayer2/ext/ima/ImaAdsLoader.java
@@ -916,32 +916,36 @@ public void playAd(AdMediaInfo adMediaInfo) {
       Log.w(TAG, "Unexpected playAd without stopAd");
     }
 
-    if (imaAdState == IMA_AD_STATE_NONE) {
-      // IMA is requesting to play the ad, so stop faking the content position.
-      fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET;
-      fakeContentProgressOffsetMs = C.TIME_UNSET;
-      imaAdState = IMA_AD_STATE_PLAYING;
-      imaAdMediaInfo = adMediaInfo;
-      imaAdInfo = Assertions.checkNotNull(adInfoByAdMediaInfo.get(adMediaInfo));
-      for (int i = 0; i < adCallbacks.size(); i++) {
-        adCallbacks.get(i).onPlay(adMediaInfo);
-      }
-      if (pendingAdPrepareErrorAdInfo != null && pendingAdPrepareErrorAdInfo.equals(imaAdInfo)) {
-        pendingAdPrepareErrorAdInfo = null;
+    try {
+      if (imaAdState == IMA_AD_STATE_NONE) {
+        // IMA is requesting to play the ad, so stop faking the content position.
+        fakeContentProgressElapsedRealtimeMs = C.TIME_UNSET;
+        fakeContentProgressOffsetMs = C.TIME_UNSET;
+        imaAdState = IMA_AD_STATE_PLAYING;
+        imaAdMediaInfo = adMediaInfo;
+        imaAdInfo = Assertions.checkNotNull(adInfoByAdMediaInfo.get(adMediaInfo));
         for (int i = 0; i < adCallbacks.size(); i++) {
-          adCallbacks.get(i).onError(adMediaInfo);
+          adCallbacks.get(i).onPlay(adMediaInfo);
+        }
+        if (pendingAdPrepareErrorAdInfo != null && pendingAdPrepareErrorAdInfo.equals(imaAdInfo)) {
+          pendingAdPrepareErrorAdInfo = null;
+          for (int i = 0; i < adCallbacks.size(); i++) {
+            adCallbacks.get(i).onError(adMediaInfo);
+          }
+        }
+        updateAdProgress();
+      } else {
+        imaAdState = IMA_AD_STATE_PLAYING;
+        Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo));
+        for (int i = 0; i < adCallbacks.size(); i++) {
+          adCallbacks.get(i).onResume(adMediaInfo);
         }
       }
-      updateAdProgress();
-    } else {
-      imaAdState = IMA_AD_STATE_PLAYING;
-      Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo));
-      for (int i = 0; i < adCallbacks.size(); i++) {
-        adCallbacks.get(i).onResume(adMediaInfo);
+      if (!Assertions.checkNotNull(player).getPlayWhenReady()) {
+        Assertions.checkNotNull(adsManager).pause();
       }
-    }
-    if (!Assertions.checkNotNull(player).getPlayWhenReady()) {
-      Assertions.checkNotNull(adsManager).pause();
+    } catch (RuntimeException e) {
+      maybeNotifyInternalError("playAd", e);
     }
   }
 
@@ -955,9 +959,9 @@ public void stopAd(AdMediaInfo adMediaInfo) {
       return;
     }
 
-    Assertions.checkNotNull(player);
-    Assertions.checkState(imaAdState != IMA_AD_STATE_NONE);
     try {
+      Assertions.checkNotNull(player);
+      Assertions.checkState(imaAdState != IMA_AD_STATE_NONE);
       stopAdInternal();
     } catch (RuntimeException e) {
       maybeNotifyInternalError("stopAd", e);
@@ -973,10 +977,15 @@ public void pauseAd(AdMediaInfo adMediaInfo) {
       // This method is called after content is resumed.
       return;
     }
-    Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo));
-    imaAdState = IMA_AD_STATE_PAUSED;
-    for (int i = 0; i < adCallbacks.size(); i++) {
-      adCallbacks.get(i).onPause(adMediaInfo);
+
+    try {
+      Assertions.checkState(adMediaInfo.equals(imaAdMediaInfo));
+      imaAdState = IMA_AD_STATE_PAUSED;
+      for (int i = 0; i < adCallbacks.size(); i++) {
+        adCallbacks.get(i).onPause(adMediaInfo);
+      }
+    } catch (RuntimeException e) {
+      maybeNotifyInternalError("pauseAd", e);
     }
   }