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
This commit is contained in:
ibaker 2022-07-19 11:05:40 +00:00 committed by Rohit Singh
parent f67c1a73f4
commit 080db2a09b
3 changed files with 30 additions and 15 deletions

View File

@ -20,7 +20,7 @@ import static java.lang.Math.min;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.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. */
@ -34,17 +34,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
@ -54,12 +54,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 com.google.android.exoplayer2; package com.google.android.exoplayer2;
import static com.google.android.exoplayer2.util.Assertions.checkArgument; 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 static com.google.android.exoplayer2.util.Assertions.checkState;
import android.content.Context; import android.content.Context;
@ -529,6 +530,7 @@ public interface ExoPlayer extends Player {
context, context,
() -> renderersFactory, () -> renderersFactory,
() -> new DefaultMediaSourceFactory(context, new DefaultExtractorsFactory())); () -> new DefaultMediaSourceFactory(context, new DefaultExtractorsFactory()));
checkNotNull(renderersFactory);
} }
/** /**
@ -546,6 +548,7 @@ public interface ExoPlayer extends Player {
*/ */
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);
} }
/** /**
@ -568,6 +571,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);
} }
/** /**
@ -601,6 +606,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(
@ -625,7 +635,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;
@ -672,6 +682,7 @@ public interface ExoPlayer extends Player {
*/ */
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;
} }
@ -685,6 +696,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;
} }
@ -698,6 +710,7 @@ public interface ExoPlayer extends Player {
*/ */
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;
} }
@ -711,6 +724,7 @@ public interface ExoPlayer extends Player {
*/ */
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;
} }
@ -724,6 +738,7 @@ public interface ExoPlayer extends Player {
*/ */
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;
} }
@ -738,6 +753,7 @@ public interface ExoPlayer extends Player {
*/ */
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;
} }
@ -751,6 +767,7 @@ public interface ExoPlayer extends Player {
*/ */
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;
} }
@ -785,7 +802,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;
} }
@ -906,7 +923,7 @@ public interface ExoPlayer extends Player {
*/ */
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;
} }
@ -1002,7 +1019,7 @@ public interface ExoPlayer extends Player {
*/ */
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

@ -1473,14 +1473,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
@ -1591,8 +1590,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