Skip to content

Commit 4666d57

Browse files
icbakerrohitjoins
authored andcommitted
Ensure ForwardingPlayer users do listener registration correctly
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)
1 parent 2ca9050 commit 4666d57

File tree

1 file changed

+18
-2
lines changed

1 file changed

+18
-2
lines changed

libraries/common/src/main/java/androidx/media3/common/ForwardingPlayer.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import android.view.SurfaceHolder;
2121
import android.view.SurfaceView;
2222
import android.view.TextureView;
23+
import androidx.annotation.CallSuper;
2324
import androidx.annotation.Nullable;
2425
import androidx.media3.common.text.Cue;
2526
import androidx.media3.common.text.CueGroup;
@@ -47,14 +48,29 @@ public Looper getApplicationLooper() {
4748
return player.getApplicationLooper();
4849
}
4950

50-
/** Calls {@link Player#addListener(Listener)} on the delegate. */
51+
/**
52+
* Calls {@link Player#addListener(Listener)} on the delegate.
53+
*
54+
* <p>Overrides of this method must delegate to {@code super.addListener} and not {@code
55+
* delegate.addListener}, in order to ensure the correct {@link Player} instance is passed to
56+
* {@link Player.Listener#onEvents(Player, Events)} (i.e. this forwarding instance, and not the
57+
* underlying {@code delegate} instance).
58+
*/
5159
@Override
60+
@CallSuper
5261
public void addListener(Listener listener) {
5362
player.addListener(new ForwardingListener(this, listener));
5463
}
5564

56-
/** Calls {@link Player#removeListener(Listener)} on the delegate. */
65+
/**
66+
* Calls {@link Player#removeListener(Listener)} on the delegate.
67+
*
68+
* <p>Overrides of this method must delegate to {@code super.removeListener} and not {@code
69+
* delegate.removeListener}, in order to ensure the listener 'matches' the listener added via
70+
* {@link #addListener} (otherwise the listener registered on the delegate won't be removed).
71+
*/
5772
@Override
73+
@CallSuper
5874
public void removeListener(Listener listener) {
5975
player.removeListener(new ForwardingListener(this, listener));
6076
}

0 commit comments

Comments
 (0)