From 7cf2fd9486b44301200c4d96908a6ba3aaaa3a6e Mon Sep 17 00:00:00 2001
From: tonihei <tonihei@google.com>
Date: Thu, 2 May 2024 08:15:58 -0700
Subject: [PATCH] Postpone AdTagLoader listener deregistration to receive final
 error

When the AdTagLoader is deactivated because of a player error, the
error callback is already pending on the app's main thread, but not
yet executed. This means the VideoAdPlayerCallback instances
registered in AdTagLoader won't receive this error event if the
Player.Listener is immediately removed from AdTagLoader.

This can be fixed by postponing the deregistration until after
already pending messages have been handled. As this means other
callbacks can be triggered now with player==null, this check needs
to be added to other callbacks to avoid handling stale events.

Issue: androidx/media#1334
PiperOrigin-RevId: 630068222
---
 RELEASENOTES.md                                            | 3 +++
 .../java/androidx/media3/exoplayer/ima/AdTagLoader.java    | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/RELEASENOTES.md b/RELEASENOTES.md
index 0a3ee15d7b..9e30d2dbbb 100644
--- a/RELEASENOTES.md
+++ b/RELEASENOTES.md
@@ -63,6 +63,9 @@
     *   Add `replaceAdTagParameters(Map <String, String>)` to
         `ImaServerSideAdInsertionMediaSource.AdLoader` that allows replacing ad
         tag parameters at runtime.
+    *   Fix bug where `VideoAdPlayer.VideoAdPlayerCallback.onError` was not
+        called when a player error happened during ad playback
+        ([#1334](https://github.com/androidx/media/issues/1334)).
 *   Session:
     *   Hide seekbar in the media notification for live streams by not setting
         the duration into the platform session metadata
diff --git a/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java b/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java
index a0409dddde..3c52782f12 100644
--- a/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java
+++ b/libraries/exoplayer_ima/src/main/java/androidx/media3/exoplayer/ima/AdTagLoader.java
@@ -406,7 +406,8 @@ import java.util.Map;
     lastAdProgress = getAdVideoProgressUpdate();
     lastContentProgress = getContentVideoProgressUpdate();
 
-    player.removeListener(this);
+    // Post release of listener so that we can report any already pending errors via onPlayerError.
+    handler.post(() -> player.removeListener(this));
     this.player = null;
   }
 
@@ -477,12 +478,12 @@ import java.util.Map;
 
   @Override
   public void onTimelineChanged(Timeline timeline, @Player.TimelineChangeReason int reason) {
-    if (timeline.isEmpty()) {
+    if (timeline.isEmpty() || player == null) {
       // The player is being reset or contains no media.
       return;
     }
+    Player player = this.player;
     this.timeline = timeline;
-    Player player = checkNotNull(this.player);
     long contentDurationUs = timeline.getPeriod(player.getCurrentPeriodIndex(), period).durationUs;
     contentDurationMs = Util.usToMs(contentDurationUs);
     if (contentDurationUs != adPlaybackState.contentDurationUs) {