Bug 1594905 - Propagate Gecko channel errors to GeckoInputStream r=geckoview-reviewers,esawin

Differential Revision: https://phabricator.services.mozilla.com/D54615

--HG--
extra : moz-landing-system : lando

mobile/android/geckoview/api.txt

@@ -1178,6 +1178,7 @@ package org.mozilla.geckoview {
     field public static final int FETCH_FLAGS_ANONYMOUS = 1;
     field public static final int FETCH_FLAGS_NONE = 0;
     field public static final int FETCH_FLAGS_NO_REDIRECTS = 2;
+    field public static final int FETCH_FLAGS_STREAM_FAILURE_TEST = 1024;
   }
 
   @AnyThread public class MediaElement {

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/WebExecutorTest.kt

@@ -304,6 +304,21 @@ class WebExecutorTest {
                 equalTo(String.format("%064x", BigInteger(1, digest))))
     }
 
+    @Test(expected = IOException::class)
+    fun testFetchStreamError() {
+
+        val expectedCount = 1 * 1024 * 1024 // 1MB
+        val response = executor.fetch(WebRequest("$TEST_ENDPOINT/bytes/$expectedCount"),
+                GeckoWebExecutor.FETCH_FLAGS_STREAM_FAILURE_TEST).pollDefault()!!
+
+        assertThat("Status code should match", response.statusCode, equalTo(200))
+        assertThat("Content-Length should match",response.headers["Content-Length"]!!.toInt(), equalTo(expectedCount))
+
+        val stream = response.body!!
+        val bytes = ByteArray(1)
+        stream.read(bytes)
+    }
+
     @Test(expected = IOException::class)
     fun readClosedStream() {
         val response = executor.fetch(WebRequest("$TEST_ENDPOINT/anything")).pollDefault()!!

mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoInputStream.java

@@ -24,6 +24,7 @@ import java.util.LinkedList;
     private LinkedList<ByteBuffer> mBuffers = new LinkedList<>();
     private boolean mEOF;
     private boolean mClosed;
+    private boolean mHaveError;
     private long mReadTimeout;
     private boolean mResumed;
     private Support mSupport;
@@ -45,7 +46,6 @@ import java.util.LinkedList;
     @Override
     public synchronized void close() throws IOException {
         super.close();
-        sendEof();
         mClosed = true;
     }
 
@@ -116,6 +116,10 @@ import java.util.LinkedList;
         }
 
         if (mEOF && mBuffers.size() == 0) {
+            if (mHaveError) {
+                throw new IOException("Unknown error");
+            }
+
             // We have no data and we're not expecting more.
             return -1;
         }
@@ -138,10 +142,29 @@ import java.util.LinkedList;
      */
     @WrapForJNI(calledFrom = "gecko")
     public synchronized void sendEof() {
+        if (mEOF) {
+            throw new IllegalStateException("Already have EOF");
+        }
+
         mEOF = true;
         notifyAll();
     }
 
+    /**
+     * Called by native code to indicate that there was an error
+     * while reading the stream.
+     */
+    @WrapForJNI(calledFrom = "gecko")
+    public synchronized void sendError() {
+        if (mEOF) {
+            throw new IllegalStateException("Already have EOF");
+        }
+
+        mEOF = true;
+        mHaveError = true;
+        notifyAll();
+    }
+
     /**
      * Called by native code to provide data for this stream.
      *
@@ -152,8 +175,12 @@ import java.util.LinkedList;
     private synchronized void appendBuffer(final byte[] buf) throws IOException {
         ThreadUtils.assertOnGeckoThread();
 
+        if (mClosed) {
+            throw new IllegalStateException("Stream is closed");
+        }
+
         if (mEOF) {
-            throw new IllegalStateException();
+            throw new IllegalStateException("EOF, no more data expected");
         }
 
         mBuffers.add(ByteBuffer.wrap(buf));

mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoWebExecutor.java

@@ -54,7 +54,7 @@ public class GeckoWebExecutor {
     }
 
     @Retention(RetentionPolicy.SOURCE)
-    @IntDef({FETCH_FLAGS_NONE, FETCH_FLAGS_ANONYMOUS, FETCH_FLAGS_NO_REDIRECTS})
+    @IntDef({FETCH_FLAGS_NONE, FETCH_FLAGS_ANONYMOUS, FETCH_FLAGS_NO_REDIRECTS, FETCH_FLAGS_STREAM_FAILURE_TEST})
     /* package */ @interface FetchFlags {};
 
     /**
@@ -74,6 +74,12 @@ public class GeckoWebExecutor {
     @WrapForJNI
     public static final int FETCH_FLAGS_NO_REDIRECTS = 1 << 1;
 
+    /**
+     * This flag causes a read error in the {@link WebResponse} body. Useful for testing.
+     */
+    @WrapForJNI
+    public static final int FETCH_FLAGS_STREAM_FAILURE_TEST = 1 << 10;
+
     /**
      * Create a new GeckoWebExecutor instance.
      *

mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md

@@ -48,6 +48,9 @@ exclude: true
   [`ContentBlockingController.Event.LOADED_LEVEL_2_TRACKING_CONTENT`][72.17].
 - Replaced `subscription` argument in [`WebPushDelegate.onPushEvent`][72.18] from a [`WebPushSubscription`][72.19] to the [`String`][72.20] `scope`.
 - ⚠️ Renamed `WebExtension.ActionIcon` to [`Icon`][72.21].
+- Added ['GeckoWebExecutor#FETCH_FLAGS_STREAM_FAILURE_TEST'][72.22], which is a new
+  flag used to immediately fail when reading a `WebResponse` body.
+  ([bug 1594905]({{bugzilla}}1594905))
 
 [72.1]: {{javadoc_uri}}/GeckoSession.NavigationDelegate.LoadRequest#hasUserGesture-
 [72.2]: {{javadoc_uri}}/Autofill.html
@@ -70,6 +73,7 @@ exclude: true
 [72.19]: {{javadoc_uri}}/WebPushSubscription.html
 [72.20]: https://developer.android.com/reference/java/lang/String
 [72.21]: {{javadoc_uri}}/WebExtension.Icon.html
+[72.22]: {{javadoc_uri}}/GeckoWebExecutor.html#FETCH_FLAGS_STREAM_FAILURE_TEST
 
 ## v71
 - Added a content blocking flag for blocked social cookies to [`ContentBlocking`][70.17].
@@ -467,4 +471,4 @@ exclude: true
 [65.24]: {{javadoc_uri}}/CrashReporter.html#sendCrashReport-android.content.Context-android.os.Bundle-java.lang.String-
 [65.25]: {{javadoc_uri}}/GeckoResult.html
 
-[api-version]: 5cdf99a7eb0c22b4d5dd0058d8f9bf3b326b3655
+[api-version]: 40acc9e29d33b23b72a345047ed1f6d4695b8c65

widget/android/WebExecutorSupport.cpp

@@ -23,6 +23,7 @@
 
 #include "mozilla/net/DNS.h"  // for NetAddr
 #include "mozilla/net/CookieSettings.h"
+#include "mozilla/Preferences.h"
 
 #include "nsNetUtil.h"  // for NS_NewURI, NS_NewChannel, NS_NewStreamLoader
 
@@ -161,8 +162,10 @@ class LoaderListener final : public nsIStreamListener,
   NS_DECL_THREADSAFE_ISUPPORTS
 
   explicit LoaderListener(java::GeckoResult::Param aResult,
-                          bool aAllowRedirects)
-      : mResult(aResult), mAllowRedirects(aAllowRedirects) {
+                          bool aAllowRedirects, bool testStreamFailure)
+      : mResult(aResult),
+        mTestStreamFailure(testStreamFailure),
+        mAllowRedirects(aAllowRedirects) {
     MOZ_ASSERT(mResult);
   }
 
@@ -202,7 +205,11 @@ class LoaderListener final : public nsIStreamListener,
   NS_IMETHOD
   OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) override {
     if (mStream) {
-      mStream->SendEof();
+      if (NS_FAILED(aStatusCode)) {
+        mStream->SendError();
+      } else {
+        mStream->SendEof();
+      }
     }
     return NS_OK;
   }
@@ -212,6 +219,10 @@ class LoaderListener final : public nsIStreamListener,
                   uint64_t aOffset, uint32_t aCount) override {
     MOZ_ASSERT(mStream);
 
+    if (mTestStreamFailure) {
+      aRequest->Cancel(NS_ERROR_ABORT);
+    }
+
     // We only need this for the ReadSegments call, the value is unused.
     uint32_t countRead;
     return aInputStream->ReadSegments(WriteSegment, this, aCount, &countRead);
@@ -311,6 +322,7 @@ class LoaderListener final : public nsIStreamListener,
   const java::GeckoResult::GlobalRef mResult;
   java::GeckoInputStream::GlobalRef mStream;
   java::GeckoInputStream::Support::GlobalRef mSupport;
+  const bool mTestStreamFailure;
 
   bool mAllowRedirects;
 };
@@ -517,8 +529,12 @@ nsresult WebExecutorSupport::CreateStreamLoader(
   const bool allowRedirects =
       !(aFlags & java::GeckoWebExecutor::FETCH_FLAGS_NO_REDIRECTS);
 
+  const bool testStreamFailure =
+      (aFlags & java::GeckoWebExecutor::FETCH_FLAGS_STREAM_FAILURE_TEST);
+
   // All done, set up the listener
-  RefPtr<LoaderListener> listener = new LoaderListener(aResult, allowRedirects);
+  RefPtr<LoaderListener> listener =
+      new LoaderListener(aResult, allowRedirects, testStreamFailure);
 
   rv = channel->SetNotificationCallbacks(listener);
   NS_ENSURE_SUCCESS(rv, rv);