Skip to content

Commit

Permalink
Make ImaAdsLoader robust to calls after it's released
Browse files Browse the repository at this point in the history
Issue: #3879

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202100576
  • Loading branch information
andrewlewis authored and ojw28 committed Jun 28, 2018
1 parent 57b7e18 commit 9a6a725
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 20 deletions.
2 changes: 2 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
be directly made with the `Player.EventListener` interface.
* Deprecate `DefaultAnalyticsListener` as selective listener overrides can be
directly made with the `AnalyticsListener` interface.
* IMA: Fix behavior when creating/releasing the player then releasing
`ImaAdsLoader` ((#3879)[https://github.com/google/ExoPlayer/issues/3879]).

### 2.8.2 ###

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,13 +281,9 @@ public ImaAdsLoader buildForAdsResponse(String adsResponse) {

/** The expected ad group index that IMA should load next. */
private int expectedAdGroupIndex;
/**
* The index of the current ad group that IMA is loading.
*/
/** The index of the current ad group that IMA is loading. */
private int adGroupIndex;
/**
* Whether IMA has sent an ad event to pause content since the last resume content event.
*/
/** Whether IMA has sent an ad event to pause content since the last resume content event. */
private boolean imaPausedContent;
/** The current ad playback state. */
private @ImaAdState int imaAdState;
Expand All @@ -299,9 +295,7 @@ public ImaAdsLoader buildForAdsResponse(String adsResponse) {

// Fields tracking the player/loader state.

/**
* Whether the player is playing an ad.
*/
/** Whether the player is playing an ad. */
private boolean playingAd;
/**
* If the player is playing an ad, stores the ad index in its ad group. {@link C#INDEX_UNSET}
Expand All @@ -324,13 +318,9 @@ public ImaAdsLoader buildForAdsResponse(String adsResponse) {
* content progress should increase. {@link C#TIME_UNSET} otherwise.
*/
private long fakeContentProgressOffsetMs;
/**
* Stores the pending content position when a seek operation was intercepted to play an ad.
*/
/** Stores the pending content position when a seek operation was intercepted to play an ad. */
private long pendingContentPositionMs;
/**
* Whether {@link #getContentProgress()} has sent {@link #pendingContentPositionMs} to IMA.
*/
/** Whether {@link #getContentProgress()} has sent {@link #pendingContentPositionMs} to IMA. */
private boolean sentPendingContentPositionMs;

/**
Expand Down Expand Up @@ -526,6 +516,11 @@ public void release() {
adsManager.destroy();
adsManager = null;
}
imaPausedContent = false;
imaAdState = IMA_AD_STATE_NONE;
pendingAdLoadError = null;
adPlaybackState = AdPlaybackState.NONE;
updateAdPlaybackState();
}

@Override
Expand Down Expand Up @@ -575,7 +570,7 @@ public void onAdEvent(AdEvent adEvent) {
Log.d(TAG, "onAdEvent: " + adEventType);
}
if (adsManager == null) {
Log.w(TAG, "Dropping ad event after release: " + adEvent);
Log.w(TAG, "Ignoring AdEvent after release: " + adEvent);
return;
}
try {
Expand Down Expand Up @@ -671,6 +666,13 @@ public VideoProgressUpdate getAdProgress() {
@Override
public void loadAd(String adUriString) {
try {
if (DEBUG) {
Log.d(TAG, "loadAd in ad group " + adGroupIndex);
}
if (adsManager == null) {
Log.w(TAG, "Ignoring loadAd after release");
return;
}
if (adGroupIndex == C.INDEX_UNSET) {
Log.w(
TAG,
Expand All @@ -679,9 +681,6 @@ public void loadAd(String adUriString) {
adGroupIndex = expectedAdGroupIndex;
adsManager.start();
}
if (DEBUG) {
Log.d(TAG, "loadAd in ad group " + adGroupIndex);
}
int adIndexInAdGroup = getAdIndexInAdGroupToLoad(adGroupIndex);
if (adIndexInAdGroup == C.INDEX_UNSET) {
Log.w(TAG, "Unexpected loadAd in an ad group with no remaining unavailable ads");
Expand Down Expand Up @@ -710,6 +709,10 @@ public void playAd() {
if (DEBUG) {
Log.d(TAG, "playAd");
}
if (adsManager == null) {
Log.w(TAG, "Ignoring playAd after release");
return;
}
switch (imaAdState) {
case IMA_AD_STATE_PLAYING:
// IMA does not always call stopAd before resuming content.
Expand Down Expand Up @@ -753,6 +756,10 @@ public void stopAd() {
if (DEBUG) {
Log.d(TAG, "stopAd");
}
if (adsManager == null) {
Log.w(TAG, "Ignoring stopAd after release");
return;
}
if (player == null) {
// Sometimes messages from IMA arrive after detaching the player. See [Internal: b/63801642].
Log.w(TAG, "Unexpected stopAd while detached");
Expand Down Expand Up @@ -1099,6 +1106,10 @@ private void handleAdPrepareError(int adGroupIndex, int adIndexInAdGroup, Except
Log.d(
TAG, "Prepare error for ad " + adIndexInAdGroup + " in group " + adGroupIndex, exception);
}
if (adsManager == null) {
Log.w(TAG, "Ignoring ad prepare error after release");
return;
}
if (imaAdState == IMA_AD_STATE_NONE) {
// Send IMA a content position at the ad group so that it will try to play it, at which point
// we can notify that it failed to load.
Expand Down Expand Up @@ -1181,7 +1192,7 @@ private void maybeNotifyInternalError(String name, Exception cause) {
Log.e(TAG, message, cause);
// We can't recover from an unexpected error in general, so skip all remaining ads.
if (adPlaybackState == null) {
adPlaybackState = new AdPlaybackState();
adPlaybackState = AdPlaybackState.NONE;
} else {
for (int i = 0; i < adPlaybackState.adGroupCount; i++) {
adPlaybackState = adPlaybackState.withSkippedAdGroup(i);
Expand Down

0 comments on commit 9a6a725

Please sign in to comment.