ConditionVariable: Improve documentation and allow clock injection

- Improve documentation explaining the benefits of ExoPlayer's ConditionVariable
  over the one that the platform provides
- Allow Clock injection
- Create TestUtil method for obtaining a ConditionVariable whose block(long)
  method times out correctly when used in a Robolectric test
- Add basic unit tests for ConditionVariable

PiperOrigin-RevId: 308812698

library/core/src/main/java/com/google/android/exoplayer2/util/ConditionVariable.java

@@ -16,13 +16,39 @@
 package com.google.android.exoplayer2.util;
 package com.google.android.exoplayer2.util;
 
 
 /**
 /**
- * An interruptible condition variable whose {@link #open()} and {@link #close()} methods return
- * whether they resulted in a change of state.
+ * An interruptible condition variable. This class provides a number of benefits over {@link
+ * android.os.ConditionVariable}:
+ *
+ * <ul>
+ *   <li>Consistent use of ({@link Clock#elapsedRealtime()} for timing {@link #block(long)} timeout
+ *       intervals. {@link android.os.ConditionVariable} used {@link System#currentTimeMillis()}
+ *       prior to Android 10, which is not a correct clock to use for interval timing because it's
+ *       not guaranteed to be monotonic.
+ *   <li>Support for injecting a custom {@link Clock}.
+ *   <li>The ability to query the variable's current state, by calling {@link #isOpen()}.
+ *   <li>{@link #open()} and {@link #close()} return whether they changed the variable's state.
+ * </ul>
  */
  */
 public class ConditionVariable {
 public class ConditionVariable {
 
 
+  private final Clock clock;
   private boolean isOpen;
   private boolean isOpen;
 
 
+  /** Creates an instance using {@link Clock#DEFAULT}. */
+  public ConditionVariable() {
+    this(Clock.DEFAULT);
+  }
+
+  /**
+   * Creates an instance.
+   *
+   * @param clock The {@link Clock} whose {@link Clock#elapsedRealtime()} method is used to
+   *     determine when {@link #block(long)} should time out.
+   */
+  public ConditionVariable(Clock clock) {
+    this.clock = clock;
+  }
+
   /**
   /**
    * Opens the condition and releases all threads that are blocked.
    * Opens the condition and releases all threads that are blocked.
    *
    *
@@ -67,11 +93,11 @@ public class ConditionVariable {
    * @throws InterruptedException If the thread is interrupted.
    * @throws InterruptedException If the thread is interrupted.
    */
    */
   public synchronized boolean block(long timeout) throws InterruptedException {
   public synchronized boolean block(long timeout) throws InterruptedException {
-    long now = android.os.SystemClock.elapsedRealtime();
+    long now = clock.elapsedRealtime();
     long end = now + timeout;
     long end = now + timeout;
     while (!isOpen && now < end) {
     while (!isOpen && now < end) {
       wait(end - now);
       wait(end - now);
-      now = android.os.SystemClock.elapsedRealtime();
+      now = clock.elapsedRealtime();
     }
     }
     return isOpen;
     return isOpen;
   }
   }

library/core/src/main/java/com/google/android/exoplayer2/util/SystemClock.java

@@ -21,9 +21,12 @@ import android.os.Looper;
 import androidx.annotation.Nullable;
 import androidx.annotation.Nullable;
 
 
 /**
 /**
- * The standard implementation of {@link Clock}.
+ * The standard implementation of {@link Clock}, an instance of which is available via {@link
+ * SystemClock#DEFAULT}.
  */
  */
-/* package */ final class SystemClock implements Clock {
+public class SystemClock implements Clock {
+
+  protected SystemClock() {}
 
 
   @Override
   @Override
   public long currentTimeMillis() {
   public long currentTimeMillis() {

library/core/src/test/java/com/google/android/exoplayer2/util/ConditionVariableTest.java

+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.android.exoplayer2.util;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/** Unit test for {@link ConditionVariableTest}. */
+@RunWith(AndroidJUnit4.class)
+public class ConditionVariableTest {
+
+  @Test
+  public void initialState_isClosed() {
+    ConditionVariable conditionVariable = buildTestConditionVariable();
+    assertThat(conditionVariable.isOpen()).isFalse();
+  }
+
+  @Test
+  public void blockWithTimeout_timesOut() throws InterruptedException {
+    ConditionVariable conditionVariable = buildTestConditionVariable();
+    assertThat(conditionVariable.block(1)).isFalse();
+    assertThat(conditionVariable.isOpen()).isFalse();
+  }
+
+  @Test
+  public void blockWithTimeout_blocksForAtLeastTimeout() throws InterruptedException {
+    ConditionVariable conditionVariable = buildTestConditionVariable();
+    long startTimeMs = System.currentTimeMillis();
+    assertThat(conditionVariable.block(/* timeout= */ 500)).isFalse();
+    long endTimeMs = System.currentTimeMillis();
+    assertThat(endTimeMs - startTimeMs).isAtLeast(500);
+  }
+
+  @Test
+  public void blockWithoutTimeout_blocks() throws InterruptedException {
+    ConditionVariable conditionVariable = buildTestConditionVariable();
+
+    AtomicBoolean blockReturned = new AtomicBoolean();
+    AtomicBoolean blockWasInterrupted = new AtomicBoolean();
+    Thread blockingThread =
+        new Thread(
+            () -> {
+              try {
+                conditionVariable.block();
+                blockReturned.set(true);
+              } catch (InterruptedException e) {
+                blockWasInterrupted.set(true);
+              }
+            });
+
+    blockingThread.start();
+    Thread.sleep(500);
+    assertThat(blockReturned.get()).isFalse();
+
+    blockingThread.interrupt();
+    blockingThread.join();
+    assertThat(blockWasInterrupted.get()).isTrue();
+    assertThat(conditionVariable.isOpen()).isFalse();
+  }
+
+  @Test
+  public void open_unblocksBlock() throws InterruptedException {
+    ConditionVariable conditionVariable = buildTestConditionVariable();
+
+    AtomicBoolean blockReturned = new AtomicBoolean();
+    AtomicBoolean blockWasInterrupted = new AtomicBoolean();
+    Thread blockingThread =
+        new Thread(
+            () -> {
+              try {
+                conditionVariable.block();
+                blockReturned.set(true);
+              } catch (InterruptedException e) {
+                blockWasInterrupted.set(true);
+              }
+            });
+
+    blockingThread.start();
+    Thread.sleep(500);
+    assertThat(blockReturned.get()).isFalse();
+
+    conditionVariable.open();
+    blockingThread.join();
+    assertThat(blockReturned.get()).isTrue();
+    assertThat(conditionVariable.isOpen()).isTrue();
+  }
+
+  private static ConditionVariable buildTestConditionVariable() {
+    return new ConditionVariable(
+        new SystemClock() {
+          @Override
+          public long elapsedRealtime() {
+            // elapsedRealtime() does not advance during Robolectric test execution, so use
+            // currentTimeMillis() instead. This is technically unsafe because this clock is not
+            // guaranteed to be monotonic, but in practice it will work provided the clock of the
+            // host machine does not change during test execution.
+            return Clock.DEFAULT.currentTimeMillis();
+          }
+        });
+  }
+}

testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java

@@ -38,6 +38,9 @@ import com.google.android.exoplayer2.metadata.MetadataInputBuffer;
 import com.google.android.exoplayer2.upstream.DataSource;
 import com.google.android.exoplayer2.upstream.DataSource;
 import com.google.android.exoplayer2.upstream.DataSpec;
 import com.google.android.exoplayer2.upstream.DataSpec;
 import com.google.android.exoplayer2.util.Assertions;
 import com.google.android.exoplayer2.util.Assertions;
+import com.google.android.exoplayer2.util.Clock;
+import com.google.android.exoplayer2.util.ConditionVariable;
+import com.google.android.exoplayer2.util.SystemClock;
 import com.google.android.exoplayer2.util.Util;
 import com.google.android.exoplayer2.util.Util;
 import java.io.File;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileOutputStream;
@@ -483,4 +486,22 @@ public class TestUtil {
     buffer.data.flip();
     buffer.data.flip();
     return buffer;
     return buffer;
   }
   }
+
+  /**
+   * Creates a {@link ConditionVariable} whose {@link ConditionVariable#block(long)} method times
+   * out according to wallclock time when used in Robolectric tests.
+   */
+  public static ConditionVariable createRobolectricConditionVariable() {
+    return new ConditionVariable(
+        new SystemClock() {
+          @Override
+          public long elapsedRealtime() {
+            // elapsedRealtime() does not advance during Robolectric test execution, so use
+            // currentTimeMillis() instead. This is technically unsafe because this clock is not
+            // guaranteed to be monotonic, but in practice it will work provided the clock of the
+            // host machine does not change during test execution.
+            return Clock.DEFAULT.currentTimeMillis();
+          }
+        });
+  }
 }
 }

testutils/src/test/java/com/google/android/exoplayer2/testutil/TestUtilTest.java

+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.android.exoplayer2.testutil;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import com.google.android.exoplayer2.util.ConditionVariable;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/** Unit test for {@link TestUtil}. */
+@RunWith(AndroidJUnit4.class)
+public class TestUtilTest {
+
+  @Test
+  public void createRobolectricConditionVariable_blockWithTimeout_timesOut()
+      throws InterruptedException {
+    ConditionVariable conditionVariable = TestUtil.createRobolectricConditionVariable();
+    assertThat(conditionVariable.block(/* timeout= */ 1)).isFalse();
+    assertThat(conditionVariable.isOpen()).isFalse();
+  }
+
+  @Test
+  public void createRobolectricConditionVariable_blockWithTimeout_blocksForAtLeastTimeout()
+      throws InterruptedException {
+    ConditionVariable conditionVariable = TestUtil.createRobolectricConditionVariable();
+    long startTimeMs = System.currentTimeMillis();
+    assertThat(conditionVariable.block(/* timeout= */ 500)).isFalse();
+    long endTimeMs = System.currentTimeMillis();
+    assertThat(endTimeMs - startTimeMs).isAtLeast(500);
+  }
+}