Extend TransferListener with onTransferInitializing and additional parameters.

This allows more fine-grained analysis of transfers.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=203950327

RELEASENOTES.md

@@ -29,6 +29,7 @@
     directly reading data should implement `BaseDataSource` to handle the
     registration correctly. Custom `DataSource`'s forwarding to other sources
     should forward all calls to `addTransferListener`.
+  * Extend `TransferListener` with additional callback parameters.
 * Error handling:
   * Allow configuration of the Loader retry delay
     ([#3370](https://github.com/google/ExoPlayer/issues/3370)).

extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java

@@ -213,7 +213,8 @@ public final class CronetDataSourceTest {
   public void testRequestOpen() throws HttpDataSourceException {
     mockResponseStartSuccess();
     assertThat(dataSourceUnderTest.open(testDataSpec)).isEqualTo(TEST_CONTENT_LENGTH);
-    verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec);
+    verify(mockTransferListener)
+        .onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
   }
 
   @Test
@@ -225,7 +226,8 @@ public final class CronetDataSourceTest {
     mockResponseStartSuccess();
 
     assertThat(dataSourceUnderTest.open(testDataSpec)).isEqualTo(5000 /* contentLength */);
-    verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testDataSpec);
+    verify(mockTransferListener)
+        .onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
   }
 
   @Test
@@ -239,7 +241,8 @@ public final class CronetDataSourceTest {
       // Check for connection not automatically closed.
       assertThat(e.getCause() instanceof UnknownHostException).isFalse();
       verify(mockUrlRequest, never()).cancel();
-      verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
+      verify(mockTransferListener, never())
+          .onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
     }
   }
 
@@ -256,7 +259,8 @@ public final class CronetDataSourceTest {
       // Check for connection not automatically closed.
       assertThat(e.getCause() instanceof UnknownHostException).isTrue();
       verify(mockUrlRequest, never()).cancel();
-      verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
+      verify(mockTransferListener, never())
+          .onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
     }
   }
 
@@ -272,7 +276,8 @@ public final class CronetDataSourceTest {
       assertThat(e instanceof HttpDataSource.InvalidResponseCodeException).isTrue();
       // Check for connection not automatically closed.
       verify(mockUrlRequest, never()).cancel();
-      verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
+      verify(mockTransferListener, never())
+          .onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
     }
   }
 
@@ -298,7 +303,8 @@ public final class CronetDataSourceTest {
 
     dataSourceUnderTest.setRequestProperty("Content-Type", TEST_CONTENT_TYPE);
     assertThat(dataSourceUnderTest.open(testPostDataSpec)).isEqualTo(TEST_CONTENT_LENGTH);
-    verify(mockTransferListener).onTransferStart(dataSourceUnderTest, testPostDataSpec);
+    verify(mockTransferListener)
+        .onTransferStart(dataSourceUnderTest, testPostDataSpec, /* isNetwork= */ true);
   }
 
   @Test
@@ -346,7 +352,8 @@ public final class CronetDataSourceTest {
 
     // Should have only called read on cronet once.
     verify(mockUrlRequest, times(1)).read(any(ByteBuffer.class));
-    verify(mockTransferListener, times(2)).onBytesTransferred(dataSourceUnderTest, 8);
+    verify(mockTransferListener, times(2))
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 8);
   }
 
   @Test
@@ -386,7 +393,8 @@ public final class CronetDataSourceTest {
     int bytesRead = dataSourceUnderTest.read(returnedBuffer, 8, 8);
     assertThat(bytesRead).isEqualTo(8);
     assertThat(returnedBuffer).isEqualTo(prefixZeros(buildTestDataArray(0, 8), 16));
-    verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 8);
+    verify(mockTransferListener)
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 8);
   }
 
   @Test
@@ -402,7 +410,8 @@ public final class CronetDataSourceTest {
     int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 16);
     assertThat(bytesRead).isEqualTo(16);
     assertThat(returnedBuffer).isEqualTo(buildTestDataArray(1000, 16));
-    verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 16);
+    verify(mockTransferListener)
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 16);
   }
 
   @Test
@@ -418,7 +427,8 @@ public final class CronetDataSourceTest {
     int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 16);
     assertThat(bytesRead).isEqualTo(16);
     assertThat(returnedBuffer).isEqualTo(buildTestDataArray(1000, 16));
-    verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 16);
+    verify(mockTransferListener)
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 16);
   }
 
   @Test
@@ -433,7 +443,8 @@ public final class CronetDataSourceTest {
     int bytesRead = dataSourceUnderTest.read(returnedBuffer, 8, 8);
     assertThat(returnedBuffer).isEqualTo(prefixZeros(buildTestDataArray(0, 8), 16));
     assertThat(bytesRead).isEqualTo(8);
-    verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 8);
+    verify(mockTransferListener)
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 8);
   }
 
   @Test
@@ -447,7 +458,8 @@ public final class CronetDataSourceTest {
     int bytesRead = dataSourceUnderTest.read(returnedBuffer, 0, 24);
     assertThat(returnedBuffer).isEqualTo(suffixZeros(buildTestDataArray(0, 16), 24));
     assertThat(bytesRead).isEqualTo(16);
-    verify(mockTransferListener).onBytesTransferred(dataSourceUnderTest, 16);
+    verify(mockTransferListener)
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 16);
   }
 
   @Test
@@ -464,7 +476,8 @@ public final class CronetDataSourceTest {
     assertThat(bytesRead).isEqualTo(8);
 
     dataSourceUnderTest.close();
-    verify(mockTransferListener).onTransferEnd(dataSourceUnderTest);
+    verify(mockTransferListener)
+        .onTransferEnd(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
 
     try {
       bytesRead += dataSourceUnderTest.read(returnedBuffer, 0, 8);
@@ -505,9 +518,12 @@ public final class CronetDataSourceTest {
 
     // Should have only called read on cronet once.
     verify(mockUrlRequest, times(1)).read(any(ByteBuffer.class));
-    verify(mockTransferListener, times(1)).onBytesTransferred(dataSourceUnderTest, 8);
+    verify(mockTransferListener, times(1))
-    verify(mockTransferListener, times(1)).onBytesTransferred(dataSourceUnderTest, 6);
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 8);
-    verify(mockTransferListener, times(1)).onBytesTransferred(dataSourceUnderTest, 2);
+    verify(mockTransferListener, times(1))
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 6);
+    verify(mockTransferListener, times(1))
+        .onBytesTransferred(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, 2);
 
     // Now we already returned the 16 bytes initially asked.
     // Try to read again even though all requested 16 bytes are already returned.
@@ -518,7 +534,8 @@ public final class CronetDataSourceTest {
     assertThat(returnedBuffer).isEqualTo(new byte[16]);
     // C.RESULT_END_OF_INPUT should not be reported though the TransferListener.
     verify(mockTransferListener, never())
-        .onBytesTransferred(dataSourceUnderTest, C.RESULT_END_OF_INPUT);
+        .onBytesTransferred(
+            dataSourceUnderTest, testDataSpec, /* isNetwork= */ true, C.RESULT_END_OF_INPUT);
     // There should still be only one call to read on cronet.
     verify(mockUrlRequest, times(1)).read(any(ByteBuffer.class));
     // Check for connection not automatically closed.
@@ -559,7 +576,8 @@ public final class CronetDataSourceTest {
     ShadowSystemClock.setCurrentTimeMillis(startTimeMs + TEST_CONNECT_TIMEOUT_MS + 10);
     timedOutLatch.await();
 
-    verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
+    verify(mockTransferListener, never())
+        .onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
   }
 
   @Test
@@ -597,7 +615,8 @@ public final class CronetDataSourceTest {
     thread.interrupt();
     timedOutLatch.await();
 
-    verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
+    verify(mockTransferListener, never())
+        .onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
   }
 
   @Test
@@ -678,7 +697,8 @@ public final class CronetDataSourceTest {
     ShadowSystemClock.setCurrentTimeMillis(startTimeMs + newTimeoutMs + 10);
     timedOutLatch.await();
 
-    verify(mockTransferListener, never()).onTransferStart(dataSourceUnderTest, testDataSpec);
+    verify(mockTransferListener, never())
+        .onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
     assertThat(openExceptions.get()).isEqualTo(1);
   }
 
@@ -797,7 +817,7 @@ public final class CronetDataSourceTest {
     // the subsequent open() call succeeds.
     doThrow(new NullPointerException())
         .when(mockTransferListener)
-        .onTransferEnd(dataSourceUnderTest);
+        .onTransferEnd(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
     dataSourceUnderTest.open(testDataSpec);
     try {
       dataSourceUnderTest.close();

library/core/src/main/java/com/google/android/exoplayer2/upstream/BaseDataSource.java

@@ -15,6 +15,8 @@
  */
 package com.google.android.exoplayer2.upstream;
 
+import android.support.annotation.Nullable;
+import com.google.android.exoplayer2.util.Assertions;
 import java.util.ArrayList;
 
 /**
@@ -29,6 +31,8 @@ public abstract class BaseDataSource implements DataSource {
   private final boolean isNetwork;
   private final ArrayList<TransferListener<? super DataSource>> listeners;
 
+  private @Nullable DataSpec dataSpec;
+
   /**
    * Creates base data source.
    *
@@ -50,7 +54,9 @@ public abstract class BaseDataSource implements DataSource {
    * @param dataSpec {@link DataSpec} describing the data for initializing transfer.
    */
   protected final void transferInitializing(DataSpec dataSpec) {
-    // TODO: notify listeners.
+    for (int i = 0; i < listeners.size(); i++) {
+      listeners.get(i).onTransferInitializing(/* source= */ this, dataSpec, isNetwork);
+    }
   }
 
   /**
@@ -59,8 +65,9 @@ public abstract class BaseDataSource implements DataSource {
    * @param dataSpec {@link DataSpec} describing the data being transferred.
    */
   protected final void transferStarted(DataSpec dataSpec) {
+    this.dataSpec = dataSpec;
     for (int i = 0; i < listeners.size(); i++) {
-      listeners.get(i).onTransferStart(/* source= */ this, dataSpec);
+      listeners.get(i).onTransferStart(/* source= */ this, dataSpec, isNetwork);
     }
   }
 
@@ -71,15 +78,20 @@ public abstract class BaseDataSource implements DataSource {
    *     (or if the first call, since the transfer was started).
    */
   protected final void bytesTransferred(int bytesTransferred) {
+    DataSpec dataSpec = Assertions.checkNotNull(this.dataSpec);
     for (int i = 0; i < listeners.size(); i++) {
-      listeners.get(i).onBytesTransferred(/* source= */ this, bytesTransferred);
+      listeners
+          .get(i)
+          .onBytesTransferred(/* source= */ this, dataSpec, isNetwork, bytesTransferred);
     }
   }
 
   /** Notifies listeners that a transfer ended. */
   protected final void transferEnded() {
+    DataSpec dataSpec = Assertions.checkNotNull(this.dataSpec);
     for (int i = 0; i < listeners.size(); i++) {
-      listeners.get(i).onTransferEnd(/* source= */ this);
+      listeners.get(i).onTransferEnd(/* source= */ this, dataSpec, isNetwork);
     }
+    this.dataSpec = null;
   }
 }

library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultBandwidthMeter.java

@@ -22,8 +22,8 @@ import com.google.android.exoplayer2.util.Clock;
 import com.google.android.exoplayer2.util.SlidingPercentile;
 
 /**
- * Estimates bandwidth by listening to data transfers. The bandwidth estimate is calculated using
+ * Estimates bandwidth by listening to data transfers. The bandwidth estimate is calculated using a
- * a {@link SlidingPercentile} and is updated each time a transfer ends.
+ * {@link SlidingPercentile} and is updated each time a transfer ends.
  */
 public final class DefaultBandwidthMeter implements BandwidthMeter, TransferListener<Object> {
 
@@ -177,7 +177,15 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList
   }
 
   @Override
-  public synchronized void onTransferStart(Object source, DataSpec dataSpec) {
+  public void onTransferInitializing(Object source, DataSpec dataSpec, boolean isNetwork) {
+    // Do nothing.
+  }
+
+  @Override
+  public synchronized void onTransferStart(Object source, DataSpec dataSpec, boolean isNetwork) {
+    if (!isNetwork) {
+      return;
+    }
     if (streamCount == 0) {
       sampleStartTimeMs = clock.elapsedRealtime();
     }
@@ -185,12 +193,19 @@ public final class DefaultBandwidthMeter implements BandwidthMeter, TransferList
   }
 
   @Override
-  public synchronized void onBytesTransferred(Object source, int bytes) {
+  public synchronized void onBytesTransferred(
+      Object source, DataSpec dataSpec, boolean isNetwork, int bytes) {
+    if (!isNetwork) {
+      return;
+    }
     sampleBytesTransferred += bytes;
   }
 
   @Override
-  public synchronized void onTransferEnd(Object source) {
+  public synchronized void onTransferEnd(Object source, DataSpec dataSpec, boolean isNetwork) {
+    if (!isNetwork) {
+      return;
+    }
     Assertions.checkState(streamCount > 0);
     long nowMs = clock.elapsedRealtime();
     int sampleElapsedTimeMs = (int) (nowMs - sampleStartTimeMs);

library/core/src/main/java/com/google/android/exoplayer2/upstream/TransferListener.java

@@ -17,31 +17,60 @@ package com.google.android.exoplayer2.upstream;
 
 /**
  * A listener of data transfer events.
+ *
+ * <p>A transfer usually progresses through multiple steps:
+ *
+ * <ol>
+ *   <li>Initializing the underlying resource (e.g. opening a HTTP connection). {@link
+ *       #onTransferInitializing(Object, DataSpec, boolean)} is called before the initialization
+ *       starts.
+ *   <li>Starting the transfer after successfully initializing the resource. {@link
+ *       #onTransferStart(Object, DataSpec, boolean)} is called. Note that this only happens if the
+ *       initialization was successful.
+ *   <li>Transferring data. {@link #onBytesTransferred(Object, DataSpec, boolean, int)} is called
+ *       frequently during the transfer to indicate progress.
+ *   <li>Closing the transfer and the underlying resource. {@link #onTransferEnd(Object, DataSpec,
+ *       boolean)} is called. Note that each {@link #onTransferStart(Object, DataSpec, boolean)}
+ *       will have exactly one corresponding call to {@link #onTransferEnd(Object, DataSpec,
+ *       boolean)}.
+ * </ol>
  */
 public interface TransferListener<S> {
 
+  /**
+   * Called when a transfer is being initialized.
+   *
+   * @param source The source performing the transfer.
+   * @param dataSpec Describes the data for which the transfer is initialized.
+   * @param isNetwork Whether the data is transferred through a network.
+   */
+  void onTransferInitializing(S source, DataSpec dataSpec, boolean isNetwork);
+
   /**
    * Called when a transfer starts.
    *
    * @param source The source performing the transfer.
    * @param dataSpec Describes the data being transferred.
+   * @param isNetwork Whether the data is transferred through a network.
    */
-  void onTransferStart(S source, DataSpec dataSpec);
+  void onTransferStart(S source, DataSpec dataSpec, boolean isNetwork);
 
   /**
    * Called incrementally during a transfer.
    *
    * @param source The source performing the transfer.
-   * @param bytesTransferred The number of bytes transferred since the previous call to this
+   * @param dataSpec Describes the data being transferred.
-   *     method (or if the first call, since the transfer was started).
+   * @param isNetwork Whether the data is transferred through a network.
+   * @param bytesTransferred The number of bytes transferred since the previous call to this method
    */
-  void onBytesTransferred(S source, int bytesTransferred);
+  void onBytesTransferred(S source, DataSpec dataSpec, boolean isNetwork, int bytesTransferred);
 
   /**
    * Called when a transfer ends.
    *
    * @param source The source performing the transfer.
+   * @param dataSpec Describes the data being transferred.
+   * @param isNetwork Whether the data is transferred through a network.
    */
-  void onTransferEnd(S source);
+  void onTransferEnd(S source, DataSpec dataSpec, boolean isNetwork);
-
 }

library/core/src/test/java/com/google/android/exoplayer2/upstream/BaseDataSourceTest.java

@@ -40,10 +40,21 @@ public class BaseDataSourceTest {
     testSource.read(/* buffer= */ null, /* offset= */ 0, /* readLength= */ 100);
     testSource.close();
 
+    assertThat(transferListener.lastTransferInitializingSource).isSameAs(testSource);
     assertThat(transferListener.lastTransferStartSource).isSameAs(testSource);
     assertThat(transferListener.lastBytesTransferredSource).isSameAs(testSource);
     assertThat(transferListener.lastTransferEndSource).isSameAs(testSource);
+
+    assertThat(transferListener.lastTransferInitializingDataSpec).isEqualTo(dataSpec);
     assertThat(transferListener.lastTransferStartDataSpec).isEqualTo(dataSpec);
+    assertThat(transferListener.lastBytesTransferredDataSpec).isEqualTo(dataSpec);
+    assertThat(transferListener.lastTransferEndDataSpec).isEqualTo(dataSpec);
+
+    assertThat(transferListener.lastTransferInitializingIsNetwork).isEqualTo(false);
+    assertThat(transferListener.lastTransferStartIsNetwork).isEqualTo(false);
+    assertThat(transferListener.lastBytesTransferredIsNetwork).isEqualTo(false);
+    assertThat(transferListener.lastTransferEndIsNetwork).isEqualTo(false);
+
     assertThat(transferListener.lastBytesTransferred).isEqualTo(100);
   }
 
@@ -58,10 +69,21 @@ public class BaseDataSourceTest {
     testSource.read(/* buffer= */ null, /* offset= */ 0, /* readLength= */ 100);
     testSource.close();
 
+    assertThat(transferListener.lastTransferInitializingSource).isSameAs(testSource);
     assertThat(transferListener.lastTransferStartSource).isSameAs(testSource);
     assertThat(transferListener.lastBytesTransferredSource).isSameAs(testSource);
     assertThat(transferListener.lastTransferEndSource).isSameAs(testSource);
+
+    assertThat(transferListener.lastTransferInitializingDataSpec).isEqualTo(dataSpec);
     assertThat(transferListener.lastTransferStartDataSpec).isEqualTo(dataSpec);
+    assertThat(transferListener.lastBytesTransferredDataSpec).isEqualTo(dataSpec);
+    assertThat(transferListener.lastTransferEndDataSpec).isEqualTo(dataSpec);
+
+    assertThat(transferListener.lastTransferInitializingIsNetwork).isEqualTo(true);
+    assertThat(transferListener.lastTransferStartIsNetwork).isEqualTo(true);
+    assertThat(transferListener.lastBytesTransferredIsNetwork).isEqualTo(true);
+    assertThat(transferListener.lastTransferEndIsNetwork).isEqualTo(true);
+
     assertThat(transferListener.lastBytesTransferred).isEqualTo(100);
   }
 
@@ -73,6 +95,7 @@ public class BaseDataSourceTest {
 
     @Override
     public long open(DataSpec dataSpec) throws IOException {
+      transferInitializing(dataSpec);
       transferStarted(dataSpec);
       return C.LENGTH_UNSET;
     }
@@ -96,27 +119,51 @@ public class BaseDataSourceTest {
 
   private static final class TestTransferListener implements TransferListener<DataSource> {
 
+    public Object lastTransferInitializingSource;
+    public DataSpec lastTransferInitializingDataSpec;
+    public boolean lastTransferInitializingIsNetwork;
+
     public Object lastTransferStartSource;
     public DataSpec lastTransferStartDataSpec;
+    public boolean lastTransferStartIsNetwork;
+
     public Object lastBytesTransferredSource;
+    public DataSpec lastBytesTransferredDataSpec;
+    public boolean lastBytesTransferredIsNetwork;
     public int lastBytesTransferred;
+
     public Object lastTransferEndSource;
+    public DataSpec lastTransferEndDataSpec;
+    public boolean lastTransferEndIsNetwork;
 
     @Override
-    public void onTransferStart(DataSource source, DataSpec dataSpec) {
+    public void onTransferInitializing(DataSource source, DataSpec dataSpec, boolean isNetwork) {
-      lastTransferStartSource = source;
+      lastTransferInitializingSource = source;
-      lastTransferStartDataSpec = dataSpec;
+      lastTransferInitializingDataSpec = dataSpec;
+      lastTransferInitializingIsNetwork = isNetwork;
     }
 
     @Override
-    public void onBytesTransferred(DataSource source, int bytesTransferred) {
+    public void onTransferStart(DataSource source, DataSpec dataSpec, boolean isNetwork) {
+      lastTransferStartSource = source;
+      lastTransferStartDataSpec = dataSpec;
+      lastTransferStartIsNetwork = isNetwork;
+    }
+
+    @Override
+    public void onBytesTransferred(
+        DataSource source, DataSpec dataSpec, boolean isNetwork, int bytesTransferred) {
       lastBytesTransferredSource = source;
+      lastBytesTransferredDataSpec = dataSpec;
+      lastBytesTransferredIsNetwork = isNetwork;
       lastBytesTransferred = bytesTransferred;
     }
 
     @Override
-    public void onTransferEnd(DataSource source) {
+    public void onTransferEnd(DataSource source, DataSpec dataSpec, boolean isNetwork) {
       lastTransferEndSource = source;
+      lastTransferEndDataSpec = dataSpec;
+      lastTransferEndIsNetwork = isNetwork;
     }
   }
 }