Skip to content

Commit

Permalink
Ensure ForwardingPlayer users do listener registration correctly
Browse files Browse the repository at this point in the history
The `@CallSuper` annotation should help catch cases where subclasses are
calling `delegate.addListener` instead of `super.addListener` but it
will also (unintentionally) prevent subclasses from either completely
no-opping the listener registration, or implementing it themselves in a
very custom way. I think that's probably OK, since these cases are
probably unusual, and they should be able to suppress the warning/error.

Issue: #258

#minor-release

PiperOrigin-RevId: 513848402
(cherry picked from commit 5d23a92)
  • Loading branch information
icbaker authored and rohitjoins committed Apr 18, 2023
1 parent 2ca9050 commit 4666d57
Showing 1 changed file with 18 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import android.view.SurfaceHolder;
import android.view.SurfaceView;
import android.view.TextureView;
import androidx.annotation.CallSuper;
import androidx.annotation.Nullable;
import androidx.media3.common.text.Cue;
import androidx.media3.common.text.CueGroup;
Expand Down Expand Up @@ -47,14 +48,29 @@ public Looper getApplicationLooper() {
return player.getApplicationLooper();
}

/** Calls {@link Player#addListener(Listener)} on the delegate. */
/**
* Calls {@link Player#addListener(Listener)} on the delegate.
*
* <p>Overrides of this method must delegate to {@code super.addListener} and not {@code
* delegate.addListener}, in order to ensure the correct {@link Player} instance is passed to
* {@link Player.Listener#onEvents(Player, Events)} (i.e. this forwarding instance, and not the
* underlying {@code delegate} instance).
*/
@Override
@CallSuper
public void addListener(Listener listener) {
player.addListener(new ForwardingListener(this, listener));
}

/** Calls {@link Player#removeListener(Listener)} on the delegate. */
/**
* Calls {@link Player#removeListener(Listener)} on the delegate.
*
* <p>Overrides of this method must delegate to {@code super.removeListener} and not {@code
* delegate.removeListener}, in order to ensure the listener 'matches' the listener added via
* {@link #addListener} (otherwise the listener registered on the delegate won't be removed).
*/
@Override
@CallSuper
public void removeListener(Listener listener) {
player.removeListener(new ForwardingListener(this, listener));
}
Expand Down

0 comments on commit 4666d57

Please sign in to comment.