Make implementation of adLoadTimeoutRunnable safer

We currently start the timeout as soon as the player enters
the target ad, but that ad hasn't loaded yet (= at the point
where we actively start waiting for it). And then we try to
clear the timeout at various places that mean we are no longer
waiting for this ad. This is very error-prone though because
there are a lot of places in the AdTagLoader that change the
AdPlaybackState and could mark the ad as skipped, failed or
loaded.

A much less error-prone way of clearing the timeout is not
even try to find all places that may revert the wait condition
and instead just check if we are still waiting at the point
of executing the timeout message.

PiperOrigin-RevId: 748291720

RELEASENOTES.md

@@ -47,6 +47,8 @@
     *   `writeSampleData()` API now uses muxer specific `BufferInfo` class
         instead of `MediaCodec.BufferInfo`.
 *   IMA extension:
+    *   Fix a bug where a load error in one ad may accidentally invalidate
+        another ad group.
 *   Session:
 *   UI:
 *   Downloads:

libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java

@@ -801,9 +801,9 @@ import java.util.Objects;
 
   private void resumeContentInternal() {
     if (imaAdInfo != null) {
-      // Remove any pending timeout tasks as CONTENT_RESUME_REQUESTED may occur instead of loadAd.
+      // Mark current ad group as skipped if it hasn't finished yet. This could for example happen
+      // after a load timeout where we receive CONTENT_RESUME_REQUESTED instead of loadAd.
       // See [Internal: b/330750756].
-      handler.removeCallbacks(adLoadTimeoutRunnable);
       adPlaybackState = adPlaybackState.withSkippedAdGroup(checkNotNull(imaAdInfo).adGroupIndex);
       updateAdPlaybackState();
     }
@@ -844,6 +844,9 @@ import java.util.Objects;
     if (adGroupIndex == C.INDEX_UNSET) {
       return false;
     }
+    if (adGroupIndex >= adPlaybackState.adGroupCount) {
+      return true;
+    }
     AdPlaybackState.AdGroup adGroup = adPlaybackState.getAdGroup(adGroupIndex);
     int adIndexInAdGroup = player.getCurrentAdIndexInAdGroup();
     if (adGroup.count == C.LENGTH_UNSET || adGroup.count <= adIndexInAdGroup) {
@@ -972,12 +975,6 @@ import java.util.Objects;
       // timeout after its media load timeout.
       return;
     }
-    if (player != null
-        && player.getCurrentAdGroupIndex() == adGroupIndex
-        && player.getCurrentAdIndexInAdGroup() == adIndexInAdGroup) {
-      // Loaded ad info the player is currently waiting for.
-      handler.removeCallbacks(adLoadTimeoutRunnable);
-    }
 
     // The ad count may increase on successive loads of ads in the same ad pod, for example, due to
     // separate requests for ad tags with multiple ads within the ad pod completing after an earlier
@@ -1129,6 +1126,11 @@ import java.util.Objects;
   }
 
   private void handleAdLoadTimeout() {
+    // We started the timeout when we were first waiting for the current ad to load. Check if we are
+    // still waiting after the timeout before triggering the error event.
+    if (!isWaitingForCurrentAdToLoad()) {
+      return;
+    }
     // IMA got stuck and didn't load an ad in time, so skip the entire group.
     handleAdGroupLoadError(new IOException("Ad loading timed out"));
     maybeNotifyPendingAdLoadError();

libraries/exoplayer_ima/src/test/java/androidx/media3/exoplayer/ima/ImaAdsLoaderTest.java

@@ -27,6 +27,7 @@ import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.robolectric.Shadows.shadowOf;
@@ -95,6 +96,7 @@ import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnit;
 import org.mockito.junit.MockitoRule;
 import org.mockito.stubbing.Answer;
+import org.robolectric.shadows.ShadowLooper;
 import org.robolectric.shadows.ShadowSystemClock;
 
 /** Tests for {@link ImaAdsLoader}. */
@@ -450,6 +452,40 @@ public final class ImaAdsLoaderTest {
                 .withContentDurationUs(CONTENT_PERIOD_DURATION_US));
   }
 
+  @Test
+  public void playback_withAdErrorWhileWaitingForPreload_doesNotReportAdLoadErrorAfterTimeout() {
+    // Simulate an ad group at 0 seconds and another at 10 seconds.
+    when(mockAdsManager.getAdCuePoints()).thenReturn(ImmutableList.of(0f, 10.0f));
+    AdEvent mockMidrollFetchErrorAdEvent = mock(AdEvent.class);
+    when(mockMidrollFetchErrorAdEvent.getType()).thenReturn(AdEventType.AD_BREAK_FETCH_ERROR);
+    when(mockMidrollFetchErrorAdEvent.getAdData())
+        .thenReturn(ImmutableMap.of("adBreakTime", "0.0"));
+    AdEvent mockContentResumeAdEvent = mock(AdEvent.class);
+    when(mockContentResumeAdEvent.getType()).thenReturn(AdEventType.CONTENT_RESUME_REQUESTED);
+    AdsLoader.EventListener mockEventListener =
+        spy(new TestAdsLoaderListener(/* periodIndex= */ 0));
+    // Advance playback to the start of the ad and simulate buffering.
+    imaAdsLoader.start(
+        adsMediaSource, TEST_DATA_SPEC, TEST_ADS_ID, adViewProvider, mockEventListener);
+    videoAdPlayer.loadAd(TEST_AD_MEDIA_INFO, mockAdPodInfo);
+    fakePlayer.setState(Player.STATE_BUFFERING, /* playWhenReady= */ true);
+    fakePlayer.setPlayingAdPosition(
+        /* periodIndex= */ 0,
+        /* adGroupIndex= */ 0,
+        /* adIndexInAdGroup= */ 0,
+        /* positionMs= */ 0,
+        /* contentPositionMs= */ 0);
+
+    // Simulate an ad load error and request to resume content from the Ads SDK.
+    adEventListener.onAdEvent(mockMidrollFetchErrorAdEvent);
+    adEventListener.onAdEvent(mockContentResumeAdEvent);
+    // Wait beyond the timeout to see if any error events from the timeout arrive after this point.
+    ShadowSystemClock.advanceBy(Duration.ofSeconds(10));
+    ShadowLooper.idleMainLooper();
+
+    verify(mockEventListener, never()).onAdLoadError(any(), any());
+  }
+
   @Test
   public void playback_withAdNotPreloadingAfterTimeout_hasErrorAdGroup() {
     // Simulate an ad at 2 seconds.
@@ -607,7 +643,7 @@ public final class ImaAdsLoaderTest {
         /* contentPositionMs= */ 0);
     shadowOf(Looper.getMainLooper()).runToEndOfTasks();
 
-    verify(mockVideoAdPlayerCallback)
+    verify(mockVideoAdPlayerCallback, atLeastOnce())
         .onAdProgress(
             TEST_AD_MEDIA_INFO,
             new VideoProgressUpdate(newPlayerPositionMs, Util.usToMs(TEST_AD_DURATION_US)));
@@ -1438,7 +1474,7 @@ public final class ImaAdsLoaderTest {
   }
 
   /** Ad loader event listener that forwards ad playback state to a fake player. */
-  private final class TestAdsLoaderListener implements AdsLoader.EventListener {
+  private class TestAdsLoaderListener implements AdsLoader.EventListener {
 
     private final int periodIndex;