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
(cherry picked from commit e1fde5d530a43b64f6e0603843ccafe7692e8453)
This commit is contained in:
ibaker 2022-07-19 11:05:40 +00:00 committed by microkatz
parent c37c6812d6
commit d5e0c7597a
3 changed files with 30 additions and 15 deletions

View File

@ -21,7 +21,7 @@ import static java.lang.Math.min;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.UnstableApi;
import androidx.media3.common.util.Util; import androidx.media3.common.util.Util;
import java.util.Collections; import com.google.common.collect.ImmutableList;
import java.util.List; import java.util.List;
/** Abstract base {@link Player} which implements common implementation independent methods. */ /** Abstract base {@link Player} which implements common implementation independent methods. */
@ -36,17 +36,17 @@ public abstract class BasePlayer implements Player {
@Override @Override
public final void setMediaItem(MediaItem mediaItem) { public final void setMediaItem(MediaItem mediaItem) {
setMediaItems(Collections.singletonList(mediaItem)); setMediaItems(ImmutableList.of(mediaItem));
} }
@Override @Override
public final void setMediaItem(MediaItem mediaItem, long startPositionMs) { public final void setMediaItem(MediaItem mediaItem, long startPositionMs) {
setMediaItems(Collections.singletonList(mediaItem), /* startWindowIndex= */ 0, startPositionMs); setMediaItems(ImmutableList.of(mediaItem), /* startIndex= */ 0, startPositionMs);
} }
@Override @Override
public final void setMediaItem(MediaItem mediaItem, boolean resetPosition) { public final void setMediaItem(MediaItem mediaItem, boolean resetPosition) {
setMediaItems(Collections.singletonList(mediaItem), resetPosition); setMediaItems(ImmutableList.of(mediaItem), resetPosition);
} }
@Override @Override
@ -56,12 +56,12 @@ public abstract class BasePlayer implements Player {
@Override @Override
public final void addMediaItem(int index, MediaItem mediaItem) { public final void addMediaItem(int index, MediaItem mediaItem) {
addMediaItems(index, Collections.singletonList(mediaItem)); addMediaItems(index, ImmutableList.of(mediaItem));
} }
@Override @Override
public final void addMediaItem(MediaItem mediaItem) { public final void addMediaItem(MediaItem mediaItem) {
addMediaItems(Collections.singletonList(mediaItem)); addMediaItems(ImmutableList.of(mediaItem));
} }
@Override @Override

View File

@ -16,6 +16,7 @@
package androidx.media3.exoplayer; package androidx.media3.exoplayer;
import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkArgument;
import static androidx.media3.common.util.Assertions.checkNotNull;
import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Assertions.checkState;
import android.content.Context; import android.content.Context;
@ -542,6 +543,7 @@ public interface ExoPlayer extends Player {
context, context,
() -> renderersFactory, () -> renderersFactory,
() -> new DefaultMediaSourceFactory(context, new DefaultExtractorsFactory())); () -> new DefaultMediaSourceFactory(context, new DefaultExtractorsFactory()));
checkNotNull(renderersFactory);
} }
/** /**
@ -560,6 +562,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder(Context context, MediaSource.Factory mediaSourceFactory) { public Builder(Context context, MediaSource.Factory mediaSourceFactory) {
this(context, () -> new DefaultRenderersFactory(context), () -> mediaSourceFactory); this(context, () -> new DefaultRenderersFactory(context), () -> mediaSourceFactory);
checkNotNull(mediaSourceFactory);
} }
/** /**
@ -583,6 +586,8 @@ public interface ExoPlayer extends Player {
RenderersFactory renderersFactory, RenderersFactory renderersFactory,
MediaSource.Factory mediaSourceFactory) { MediaSource.Factory mediaSourceFactory) {
this(context, () -> renderersFactory, () -> mediaSourceFactory); this(context, () -> renderersFactory, () -> mediaSourceFactory);
checkNotNull(renderersFactory);
checkNotNull(mediaSourceFactory);
} }
/** /**
@ -617,6 +622,11 @@ public interface ExoPlayer extends Player {
() -> loadControl, () -> loadControl,
() -> bandwidthMeter, () -> bandwidthMeter,
(clock) -> analyticsCollector); (clock) -> analyticsCollector);
checkNotNull(renderersFactory);
checkNotNull(mediaSourceFactory);
checkNotNull(trackSelector);
checkNotNull(bandwidthMeter);
checkNotNull(analyticsCollector);
} }
private Builder( private Builder(
@ -641,7 +651,7 @@ public interface ExoPlayer extends Player {
Supplier<LoadControl> loadControlSupplier, Supplier<LoadControl> loadControlSupplier,
Supplier<BandwidthMeter> bandwidthMeterSupplier, Supplier<BandwidthMeter> bandwidthMeterSupplier,
Function<Clock, AnalyticsCollector> analyticsCollectorFunction) { Function<Clock, AnalyticsCollector> analyticsCollectorFunction) {
this.context = context; this.context = checkNotNull(context);
this.renderersFactorySupplier = renderersFactorySupplier; this.renderersFactorySupplier = renderersFactorySupplier;
this.mediaSourceFactorySupplier = mediaSourceFactorySupplier; this.mediaSourceFactorySupplier = mediaSourceFactorySupplier;
this.trackSelectorSupplier = trackSelectorSupplier; this.trackSelectorSupplier = trackSelectorSupplier;
@ -690,6 +700,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder setRenderersFactory(RenderersFactory renderersFactory) { public Builder setRenderersFactory(RenderersFactory renderersFactory) {
checkState(!buildCalled); checkState(!buildCalled);
checkNotNull(renderersFactory);
this.renderersFactorySupplier = () -> renderersFactory; this.renderersFactorySupplier = () -> renderersFactory;
return this; return this;
} }
@ -703,6 +714,7 @@ public interface ExoPlayer extends Player {
*/ */
public Builder setMediaSourceFactory(MediaSource.Factory mediaSourceFactory) { public Builder setMediaSourceFactory(MediaSource.Factory mediaSourceFactory) {
checkState(!buildCalled); checkState(!buildCalled);
checkNotNull(mediaSourceFactory);
this.mediaSourceFactorySupplier = () -> mediaSourceFactory; this.mediaSourceFactorySupplier = () -> mediaSourceFactory;
return this; return this;
} }
@ -717,6 +729,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder setTrackSelector(TrackSelector trackSelector) { public Builder setTrackSelector(TrackSelector trackSelector) {
checkState(!buildCalled); checkState(!buildCalled);
checkNotNull(trackSelector);
this.trackSelectorSupplier = () -> trackSelector; this.trackSelectorSupplier = () -> trackSelector;
return this; return this;
} }
@ -731,6 +744,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder setLoadControl(LoadControl loadControl) { public Builder setLoadControl(LoadControl loadControl) {
checkState(!buildCalled); checkState(!buildCalled);
checkNotNull(loadControl);
this.loadControlSupplier = () -> loadControl; this.loadControlSupplier = () -> loadControl;
return this; return this;
} }
@ -745,6 +759,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder setBandwidthMeter(BandwidthMeter bandwidthMeter) { public Builder setBandwidthMeter(BandwidthMeter bandwidthMeter) {
checkState(!buildCalled); checkState(!buildCalled);
checkNotNull(bandwidthMeter);
this.bandwidthMeterSupplier = () -> bandwidthMeter; this.bandwidthMeterSupplier = () -> bandwidthMeter;
return this; return this;
} }
@ -760,6 +775,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder setLooper(Looper looper) { public Builder setLooper(Looper looper) {
checkState(!buildCalled); checkState(!buildCalled);
checkNotNull(looper);
this.looper = looper; this.looper = looper;
return this; return this;
} }
@ -774,6 +790,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder setAnalyticsCollector(AnalyticsCollector analyticsCollector) { public Builder setAnalyticsCollector(AnalyticsCollector analyticsCollector) {
checkState(!buildCalled); checkState(!buildCalled);
checkNotNull(analyticsCollector);
this.analyticsCollectorFunction = (clock) -> analyticsCollector; this.analyticsCollectorFunction = (clock) -> analyticsCollector;
return this; return this;
} }
@ -809,7 +826,7 @@ public interface ExoPlayer extends Player {
*/ */
public Builder setAudioAttributes(AudioAttributes audioAttributes, boolean handleAudioFocus) { public Builder setAudioAttributes(AudioAttributes audioAttributes, boolean handleAudioFocus) {
checkState(!buildCalled); checkState(!buildCalled);
this.audioAttributes = audioAttributes; this.audioAttributes = checkNotNull(audioAttributes);
this.handleAudioFocus = handleAudioFocus; this.handleAudioFocus = handleAudioFocus;
return this; return this;
} }
@ -935,7 +952,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder setSeekParameters(SeekParameters seekParameters) { public Builder setSeekParameters(SeekParameters seekParameters) {
checkState(!buildCalled); checkState(!buildCalled);
this.seekParameters = seekParameters; this.seekParameters = checkNotNull(seekParameters);
return this; return this;
} }
@ -1037,7 +1054,7 @@ public interface ExoPlayer extends Player {
@UnstableApi @UnstableApi
public Builder setLivePlaybackSpeedControl(LivePlaybackSpeedControl livePlaybackSpeedControl) { public Builder setLivePlaybackSpeedControl(LivePlaybackSpeedControl livePlaybackSpeedControl) {
checkState(!buildCalled); checkState(!buildCalled);
this.livePlaybackSpeedControl = livePlaybackSpeedControl; this.livePlaybackSpeedControl = checkNotNull(livePlaybackSpeedControl);
return this; return this;
} }

View File

@ -1484,14 +1484,13 @@ import java.util.concurrent.TimeoutException;
@Override @Override
public void addAnalyticsListener(AnalyticsListener listener) { public void addAnalyticsListener(AnalyticsListener listener) {
// Don't verify application thread. We allow calls to this method from any thread. // Don't verify application thread. We allow calls to this method from any thread.
checkNotNull(listener); analyticsCollector.addListener(checkNotNull(listener));
analyticsCollector.addListener(listener);
} }
@Override @Override
public void removeAnalyticsListener(AnalyticsListener listener) { public void removeAnalyticsListener(AnalyticsListener listener) {
// Don't verify application thread. We allow calls to this method from any thread. // Don't verify application thread. We allow calls to this method from any thread.
analyticsCollector.removeListener(listener); analyticsCollector.removeListener(checkNotNull(listener));
} }
@Override @Override
@ -1602,8 +1601,7 @@ import java.util.concurrent.TimeoutException;
@Override @Override
public void addListener(Listener listener) { public void addListener(Listener listener) {
// Don't verify application thread. We allow calls to this method from any thread. // Don't verify application thread. We allow calls to this method from any thread.
checkNotNull(listener); listeners.add(checkNotNull(listener));
listeners.add(listener);
} }
@Override @Override