From af45bedfcd7b60391afac5e82c962b7b2958f888 Mon Sep 17 00:00:00 2001
From: ibaker <ibaker@google.com>
Date: Fri, 3 Mar 2023 17:32:36 +0000
Subject: [PATCH] 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: androidx/media#258

#minor-release

PiperOrigin-RevId: 513848402
---
 .../android/exoplayer2/ForwardingPlayer.java  | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/library/common/src/main/java/com/google/android/exoplayer2/ForwardingPlayer.java b/library/common/src/main/java/com/google/android/exoplayer2/ForwardingPlayer.java
index 014ce116e7..4b835d32d9 100644
--- a/library/common/src/main/java/com/google/android/exoplayer2/ForwardingPlayer.java
+++ b/library/common/src/main/java/com/google/android/exoplayer2/ForwardingPlayer.java
@@ -20,6 +20,7 @@ import android.view.Surface;
 import android.view.SurfaceHolder;
 import android.view.SurfaceView;
 import android.view.TextureView;
+import androidx.annotation.CallSuper;
 import androidx.annotation.Nullable;
 import com.google.android.exoplayer2.audio.AudioAttributes;
 import com.google.android.exoplayer2.metadata.Metadata;
@@ -49,14 +50,29 @@ public class ForwardingPlayer implements Player {
     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));
   }