From b5a1efdbce3220ef6a5733bd67ceb7dbaca3a5f3 Mon Sep 17 00:00:00 2001
From: ibaker <ibaker@google.com>
Date: Thu, 5 Dec 2024 02:21:38 -0800
Subject: [PATCH] `ForwardingTimeline`: Implement & `final`ize some methods

`equals`, `hashCode`, and `getPeriodByUid` are correctly implemented on
`Timeline`. Overriding these in a way that maintains correctness is
fiddly, so this CL prevents that for the 'simple' case of subclasses of
`ForwardingTimeline`. Implementations of `Timeline` that need to
override these methods for performance should extend `Timeline` or
`AbstractConcatenatedTimeline` instead of `ForwardingTimeline`.

PiperOrigin-RevId: 703035721
---
 .../exoplayer/source/ForwardingTimeline.java  | 28 ++++++++++++++++++-
 .../source/ForwardingTimelineTest.java        | 10 +++----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ForwardingTimeline.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ForwardingTimeline.java
index 8182c017f4..becf42395e 100644
--- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ForwardingTimeline.java
+++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ForwardingTimeline.java
@@ -15,11 +15,22 @@
  */
 package androidx.media3.exoplayer.source;
 
+import androidx.annotation.Nullable;
 import androidx.media3.common.Player;
 import androidx.media3.common.Timeline;
 import androidx.media3.common.util.UnstableApi;
 
-/** An overridable {@link Timeline} implementation forwarding all methods to another timeline. */
+/**
+ * An overridable {@link Timeline} implementation forwarding methods to another timeline.
+ *
+ * <p>The following methods call through to {@code super} instead of the {@code timeline} delegate:
+ *
+ * <ul>
+ *   <li>{@link #getPeriodByUid(Object, Period)}
+ *   <li>{@link #equals(Object)}
+ *   <li>{@link #hashCode()}
+ * </ul>
+ */
 @UnstableApi
 public abstract class ForwardingTimeline extends Timeline {
 
@@ -66,6 +77,11 @@ public abstract class ForwardingTimeline extends Timeline {
     return timeline.getPeriodCount();
   }
 
+  @Override
+  public final Period getPeriodByUid(Object periodUid, Period period) {
+    return super.getPeriodByUid(periodUid, period);
+  }
+
   @Override
   public Period getPeriod(int periodIndex, Period period, boolean setIds) {
     return timeline.getPeriod(periodIndex, period, setIds);
@@ -80,4 +96,14 @@ public abstract class ForwardingTimeline extends Timeline {
   public Object getUidOfPeriod(int periodIndex) {
     return timeline.getUidOfPeriod(periodIndex);
   }
+
+  @Override
+  public final boolean equals(@Nullable Object obj) {
+    return super.equals(obj);
+  }
+
+  @Override
+  public final int hashCode() {
+    return super.hashCode();
+  }
 }
diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ForwardingTimelineTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ForwardingTimelineTest.java
index 3105d49cf9..e52fb52180 100644
--- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ForwardingTimelineTest.java
+++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ForwardingTimelineTest.java
@@ -16,7 +16,7 @@
 package androidx.media3.exoplayer.source;
 
 import static androidx.media3.test.utils.TestUtil.assertForwardingClassForwardsAllMethodsExcept;
-import static androidx.media3.test.utils.TestUtil.assertForwardingClassOverridesAllMethodsExcept;
+import static androidx.media3.test.utils.TestUtil.assertForwardingClassOverridesAllMethods;
 
 import androidx.media3.common.Timeline;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
@@ -30,14 +30,14 @@ public class ForwardingTimelineTest {
 
   @Test
   public void overridesAllMethods() throws Exception {
-    assertForwardingClassOverridesAllMethodsExcept(
-        Timeline.class,
-        ForwardingTimeline.class,
-        ImmutableSet.of("equals", "hashCode", "getPeriodByUid"));
+    assertForwardingClassOverridesAllMethods(Timeline.class, ForwardingTimeline.class);
   }
 
   @Test
   public void forwardsAllMethods() throws Exception {
+    // ForwardingTimeline equals, hashCode, and getPeriodByUid implementations deliberately call
+    // through to super rather than the delegate instance. This is because these methods are already
+    // correctly implemented on Timeline in terms of the publicly visible parts of Timeline.
     assertForwardingClassForwardsAllMethodsExcept(
         Timeline.class,
         delegate -> new ForwardingTimeline(delegate) {},