From 080db2a09b9ebc31fb346e0beb0ea7440df795a5 Mon Sep 17 00:00:00 2001 From: ibaker Date: Tue, 19 Jul 2022 11:05:40 +0000 Subject: [PATCH] Add fail-fast null checks to the stable Player API This will help developers self-diagnose issues like Issue: google/ExoPlayer#10392 where the NPE occurs far from the original null value because a field gets assigned to null. This change aims to ensure that every stable method on Player, ExoPlayer and ExoPlayer.Builder that takes a non-null type will fail with an NPE before returning. #minor-release PiperOrigin-RevId: 461846580 --- .../google/android/exoplayer2/BasePlayer.java | 12 ++++----- .../google/android/exoplayer2/ExoPlayer.java | 25 ++++++++++++++++--- .../android/exoplayer2/ExoPlayerImpl.java | 8 +++--- 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/library/common/src/main/java/com/google/android/exoplayer2/BasePlayer.java b/library/common/src/main/java/com/google/android/exoplayer2/BasePlayer.java index 3e50201539..a3105192e5 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/BasePlayer.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/BasePlayer.java @@ -20,7 +20,7 @@ import static java.lang.Math.min; import androidx.annotation.Nullable; import com.google.android.exoplayer2.util.Util; -import java.util.Collections; +import com.google.common.collect.ImmutableList; import java.util.List; /** Abstract base {@link Player} which implements common implementation independent methods. */ @@ -34,17 +34,17 @@ public abstract class BasePlayer implements Player { @Override public final void setMediaItem(MediaItem mediaItem) { - setMediaItems(Collections.singletonList(mediaItem)); + setMediaItems(ImmutableList.of(mediaItem)); } @Override public final void setMediaItem(MediaItem mediaItem, long startPositionMs) { - setMediaItems(Collections.singletonList(mediaItem), /* startWindowIndex= */ 0, startPositionMs); + setMediaItems(ImmutableList.of(mediaItem), /* startIndex= */ 0, startPositionMs); } @Override public final void setMediaItem(MediaItem mediaItem, boolean resetPosition) { - setMediaItems(Collections.singletonList(mediaItem), resetPosition); + setMediaItems(ImmutableList.of(mediaItem), resetPosition); } @Override @@ -54,12 +54,12 @@ public abstract class BasePlayer implements Player { @Override public final void addMediaItem(int index, MediaItem mediaItem) { - addMediaItems(index, Collections.singletonList(mediaItem)); + addMediaItems(index, ImmutableList.of(mediaItem)); } @Override public final void addMediaItem(MediaItem mediaItem) { - addMediaItems(Collections.singletonList(mediaItem)); + addMediaItems(ImmutableList.of(mediaItem)); } @Override diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayer.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayer.java index b2b1512655..dc6e7a7bfb 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayer.java @@ -16,6 +16,7 @@ package com.google.android.exoplayer2; import static com.google.android.exoplayer2.util.Assertions.checkArgument; +import static com.google.android.exoplayer2.util.Assertions.checkNotNull; import static com.google.android.exoplayer2.util.Assertions.checkState; import android.content.Context; @@ -529,6 +530,7 @@ public interface ExoPlayer extends Player { context, () -> renderersFactory, () -> new DefaultMediaSourceFactory(context, new DefaultExtractorsFactory())); + checkNotNull(renderersFactory); } /** @@ -546,6 +548,7 @@ public interface ExoPlayer extends Player { */ public Builder(Context context, MediaSource.Factory mediaSourceFactory) { this(context, () -> new DefaultRenderersFactory(context), () -> mediaSourceFactory); + checkNotNull(mediaSourceFactory); } /** @@ -568,6 +571,8 @@ public interface ExoPlayer extends Player { RenderersFactory renderersFactory, MediaSource.Factory mediaSourceFactory) { this(context, () -> renderersFactory, () -> mediaSourceFactory); + checkNotNull(renderersFactory); + checkNotNull(mediaSourceFactory); } /** @@ -601,6 +606,11 @@ public interface ExoPlayer extends Player { () -> loadControl, () -> bandwidthMeter, (clock) -> analyticsCollector); + checkNotNull(renderersFactory); + checkNotNull(mediaSourceFactory); + checkNotNull(trackSelector); + checkNotNull(bandwidthMeter); + checkNotNull(analyticsCollector); } private Builder( @@ -625,7 +635,7 @@ public interface ExoPlayer extends Player { Supplier loadControlSupplier, Supplier bandwidthMeterSupplier, Function analyticsCollectorFunction) { - this.context = context; + this.context = checkNotNull(context); this.renderersFactorySupplier = renderersFactorySupplier; this.mediaSourceFactorySupplier = mediaSourceFactorySupplier; this.trackSelectorSupplier = trackSelectorSupplier; @@ -672,6 +682,7 @@ public interface ExoPlayer extends Player { */ public Builder setRenderersFactory(RenderersFactory renderersFactory) { checkState(!buildCalled); + checkNotNull(renderersFactory); this.renderersFactorySupplier = () -> renderersFactory; return this; } @@ -685,6 +696,7 @@ public interface ExoPlayer extends Player { */ public Builder setMediaSourceFactory(MediaSource.Factory mediaSourceFactory) { checkState(!buildCalled); + checkNotNull(mediaSourceFactory); this.mediaSourceFactorySupplier = () -> mediaSourceFactory; return this; } @@ -698,6 +710,7 @@ public interface ExoPlayer extends Player { */ public Builder setTrackSelector(TrackSelector trackSelector) { checkState(!buildCalled); + checkNotNull(trackSelector); this.trackSelectorSupplier = () -> trackSelector; return this; } @@ -711,6 +724,7 @@ public interface ExoPlayer extends Player { */ public Builder setLoadControl(LoadControl loadControl) { checkState(!buildCalled); + checkNotNull(loadControl); this.loadControlSupplier = () -> loadControl; return this; } @@ -724,6 +738,7 @@ public interface ExoPlayer extends Player { */ public Builder setBandwidthMeter(BandwidthMeter bandwidthMeter) { checkState(!buildCalled); + checkNotNull(bandwidthMeter); this.bandwidthMeterSupplier = () -> bandwidthMeter; return this; } @@ -738,6 +753,7 @@ public interface ExoPlayer extends Player { */ public Builder setLooper(Looper looper) { checkState(!buildCalled); + checkNotNull(looper); this.looper = looper; return this; } @@ -751,6 +767,7 @@ public interface ExoPlayer extends Player { */ public Builder setAnalyticsCollector(AnalyticsCollector analyticsCollector) { checkState(!buildCalled); + checkNotNull(analyticsCollector); this.analyticsCollectorFunction = (clock) -> analyticsCollector; return this; } @@ -785,7 +802,7 @@ public interface ExoPlayer extends Player { */ public Builder setAudioAttributes(AudioAttributes audioAttributes, boolean handleAudioFocus) { checkState(!buildCalled); - this.audioAttributes = audioAttributes; + this.audioAttributes = checkNotNull(audioAttributes); this.handleAudioFocus = handleAudioFocus; return this; } @@ -906,7 +923,7 @@ public interface ExoPlayer extends Player { */ public Builder setSeekParameters(SeekParameters seekParameters) { checkState(!buildCalled); - this.seekParameters = seekParameters; + this.seekParameters = checkNotNull(seekParameters); return this; } @@ -1002,7 +1019,7 @@ public interface ExoPlayer extends Player { */ public Builder setLivePlaybackSpeedControl(LivePlaybackSpeedControl livePlaybackSpeedControl) { checkState(!buildCalled); - this.livePlaybackSpeedControl = livePlaybackSpeedControl; + this.livePlaybackSpeedControl = checkNotNull(livePlaybackSpeedControl); return this; } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index d7358ec04e..7e6f99a5ad 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -1473,14 +1473,13 @@ import java.util.concurrent.TimeoutException; @Override public void addAnalyticsListener(AnalyticsListener listener) { // Don't verify application thread. We allow calls to this method from any thread. - checkNotNull(listener); - analyticsCollector.addListener(listener); + analyticsCollector.addListener(checkNotNull(listener)); } @Override public void removeAnalyticsListener(AnalyticsListener listener) { // Don't verify application thread. We allow calls to this method from any thread. - analyticsCollector.removeListener(listener); + analyticsCollector.removeListener(checkNotNull(listener)); } @Override @@ -1591,8 +1590,7 @@ import java.util.concurrent.TimeoutException; @Override public void addListener(Listener listener) { // Don't verify application thread. We allow calls to this method from any thread. - checkNotNull(listener); - listeners.add(listener); + listeners.add(checkNotNull(listener)); } @Override