Bug 1595145 - Allow `GeckoInputStream#read()` to timeout. r=geckoview-reviewers,agi,esawin
authorJames Willcox <snorp@snorp.net>
Wed, 13 Nov 2019 15:38:51 +0000
changeset 501805 e7a759d68461596d4988c54e8c8e23b9a853e6ae
parent 501804 9a4d7144206139470df276d6d0a26a99068aa0ab
child 501806 da9476881d622b2d519f589f39d586f974ddb1e1
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, agi, esawin
bugs1595145
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1595145 - Allow `GeckoInputStream#read()` to timeout. r=geckoview-reviewers,agi,esawin Differential Revision: https://phabricator.services.mozilla.com/D52382
mobile/android/geckoview/api.txt
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExecutorTest.kt
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputStream.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebResponse.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
--- a/mobile/android/geckoview/api.txt
+++ b/mobile/android/geckoview/api.txt
@@ -1541,16 +1541,18 @@ package org.mozilla.geckoview {
     field public static final int ERROR_UNKNOWN_SOCKET_TYPE = 83;
     field public static final int ERROR_UNSAFE_CONTENT_TYPE = 36;
     field public final int category;
     field public final int code;
   }
 
   @AnyThread public class WebResponse extends WebMessage {
     ctor protected WebResponse(@NonNull WebResponse.Builder);
+    method public void setReadTimeoutMillis(long);
+    field public static final long DEFAULT_READ_TIMEOUT_MS = 30000L;
     field @Nullable public final InputStream body;
     field public final boolean redirected;
     field public final int statusCode;
   }
 
   @AnyThread public static class WebResponse.Builder extends WebMessage.Builder {
     ctor public Builder(@NonNull String);
     method @NonNull public WebResponse.Builder body(@NonNull InputStream);
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExecutorTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExecutorTest.kt
@@ -310,16 +310,31 @@ class WebExecutorTest {
 
         assertThat("Status code should match", response.statusCode, equalTo(200))
 
         val stream = response.body!!
         stream.close()
         stream.readBytes()
     }
 
+    @Test(expected = IOException::class)
+    fun readTimeout() {
+        val expectedCount = 1 * 1024 * 1024 // 1MB
+        val response = executor.fetch(WebRequest("$TEST_ENDPOINT/bytes/$expectedCount")).pollDefault()!!
+
+        assertThat("Status code should match", response.statusCode, equalTo(200))
+        assertThat("Content-Length should match", response.headers["Content-Length"]!!.toInt(), equalTo(expectedCount))
+
+        // Only allow 1ms of blocking. This should reliably timeout with 1MB of data.
+        response.setReadTimeoutMillis(1)
+
+        val stream = response.body!!
+        stream.readBytes()
+    }
+
     @Test
     fun testFetchStreamCancel() {
         val expectedCount = 1 * 1024 * 1024 // 1MB
         val response = executor.fetch(WebRequest("$TEST_ENDPOINT/bytes/$expectedCount")).pollDefault()!!
 
         assertThat("Status code should match", response.statusCode, equalTo(200))
         assertThat("Content-Length should match", response.headers["Content-Length"]!!.toInt(), equalTo(expectedCount))
 
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputStream.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputStream.java
@@ -19,29 +19,34 @@ import java.util.LinkedList;
 @WrapForJNI
 @AnyThread
 /* package */ class GeckoInputStream extends InputStream {
     private static final String LOGTAG = "GeckoInputStream";
 
     private LinkedList<ByteBuffer> mBuffers = new LinkedList<>();
     private boolean mEOF;
     private boolean mClosed;
+    private long mReadTimeout;
     private boolean mResumed;
     private Support mSupport;
 
     /**
      * This is only called via JNI. The support instance provides
      * callbacks for the native counterpart.
      *
      * @param support An instance of {@link Support}, used for native callbacks.
      */
     private GeckoInputStream(final @NonNull Support support) {
         mSupport = support;
     }
 
+    public void setReadTimeoutMillis(final long millis) {
+        mReadTimeout = millis;
+    }
+
     @Override
     public synchronized void close() throws IOException {
         super.close();
         sendEof();
         mClosed = true;
     }
 
     @Override
@@ -86,26 +91,31 @@ import java.util.LinkedList;
         return read(b, 0, b.length);
     }
 
     @Override
     public synchronized int read(final @NonNull byte[] dest, final int offset, final int length)
             throws IOException {
         ensureNotClosed();
 
+        long startTime = System.currentTimeMillis();
         while (!mEOF && mBuffers.size() == 0) {
+            if (mReadTimeout > 0 && (System.currentTimeMillis() - startTime) >= mReadTimeout) {
+                throw new IOException("Timed out");
+            }
+
             // The underlying channel is suspended, so resume that before
             // waiting for a buffer.
             if (!mResumed) {
                 mSupport.resume();
                 mResumed = true;
             }
 
             try {
-                wait();
+                wait(mReadTimeout);
             } catch (InterruptedException e) {
             }
         }
 
         if (mEOF && mBuffers.size() == 0) {
             // We have no data and we're not expecting more.
             return -1;
         }
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebResponse.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/WebResponse.java
@@ -17,16 +17,21 @@ import java.io.InputStream;
 /**
  * WebResponse represents an HTTP[S] response. It is normally created
  * by {@link GeckoWebExecutor#fetch(WebRequest)}.
  */
 @WrapForJNI
 @AnyThread
 public class WebResponse extends WebMessage {
     /**
+     * The default read timeout for the {@link #body} stream.
+     */
+    public static final long DEFAULT_READ_TIMEOUT_MS = 30000;
+
+    /**
      * The HTTP status code for the response, e.g. 200.
      */
     public final int statusCode;
 
     /**
      * A boolean indicating whether or not this response is
      * the result of a redirection.
      */
@@ -37,16 +42,32 @@ public class WebResponse extends WebMess
      */
     public final @Nullable InputStream body;
 
     protected WebResponse(final @NonNull Builder builder) {
         super(builder);
         this.statusCode = builder.mStatusCode;
         this.redirected = builder.mRedirected;
         this.body = builder.mBody;
+
+        this.setReadTimeoutMillis(DEFAULT_READ_TIMEOUT_MS);
+    }
+
+    /**
+     * Sets the maximum amount of time to wait for data in the {@link #body} read() method.
+     * By default, the read timeout is set to {@link #DEFAULT_READ_TIMEOUT_MS}.
+     *
+     * If 0, there will be no timeout and read() will block indefinitely.
+     *
+     * @param millis The duration in milliseconds for the timeout.
+     */
+    public void setReadTimeoutMillis(final long millis) {
+        if (this.body != null && this.body instanceof GeckoInputStream) {
+            ((GeckoInputStream)this.body).setReadTimeoutMillis(millis);
+        }
     }
 
     /**
      * Builder offers a convenient way to create WebResponse instances.
      */
     @WrapForJNI
     @AnyThread
     public static class Builder extends WebMessage.Builder {
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
@@ -15,19 +15,26 @@ exclude: true
 
 ## v72
 - Added [`GeckoSession.NavigationDelegate.LoadRequest#hasUserGesture`][72.1]. This indicates
   if a load was requested while a user gesture was active (e.g., a tap).
   ([bug 1555337]({{bugzilla}}1555337))
 - ⚠️  Refactored `AutofillElement` and `AutofillSupport` into the
   [`Autofill`][72.2] API.
   ([bug 1591462]({{bugzilla}}1591462))
+- Make `read()` in the `InputStream` returned from [`WebResponse#body`][72.3] timeout according
+  to [`WebResponse#setReadTimeoutMillis()`][72.4]. The default timeout value is reflected in
+  [`WebResponse#DEFAULT_READ_TIMEOUT_MS`][72.5], currently 30s.
+  ([bug 1595145]({{bugzilla}}1595145))
 
 [72.1]: {{javadoc_uri}}/GeckoSession.NavigationDelegate.LoadRequest#hasUserGesture-
 [72.2]: {{javadoc_uri}}/Autofill.html
+[72.3]: {{javadoc_uri}}/WebResponse.html#body
+[72.4]: {{javadoc_uri}}/WebResponse.html#setReadTimeoutMillis-long-
+[72.5]: {{javadoc_uri}}/WebResponse.html#DEFAULT_READ_TIMEOUT_MS
 
 ## v71
 - Added a content blocking flag for blocked social cookies to [`ContentBlocking`][70.17].
   ([bug 1584479]({{bugzilla}}1584479))
 - Added [`onBooleanScalar`][71.1], [`onLongScalar`][71.2],
   [`onStringScalar`][71.3] to [`RuntimeTelemetry.Delegate`][70.12] to support
   scalars in streaming telemetry. ⚠️  As part of this change,
   `onTelemetryReceived` has been renamed to [`onHistogram`][71.4], and
@@ -413,9 +420,9 @@ exclude: true
 [65.19]: {{javadoc_uri}}/GeckoSession.NavigationDelegate.LoadRequest.html#isRedirect
 [65.20]: {{javadoc_uri}}/GeckoSession.html#LOAD_FLAGS_BYPASS_CLASSIFIER    
 [65.21]: {{javadoc_uri}}/GeckoSession.ContentDelegate.ContextElement.html
 [65.22]: {{javadoc_uri}}/GeckoSession.ContentDelegate.html#onContextMenu-org.mozilla.geckoview.GeckoSession-int-int-org.mozilla.geckoview.GeckoSession.ContentDelegate.ContextElement-
 [65.23]: {{javadoc_uri}}/GeckoSession.FinderResult.html
 [65.24]: {{javadoc_uri}}/CrashReporter.html#sendCrashReport-android.content.Context-android.os.Bundle-java.lang.String-
 [65.25]: {{javadoc_uri}}/GeckoResult.html
 
-[api-version]: df2caa160412647457546a27c33e2dd6d016cc28
+[api-version]: f4f62b0476eb283fbaf4be55e91b78dede9f0099