Keep ReadableStreamDefaultReader alive if it has pending requests.

If the only reference to a ReadableStreamDefaultReader is from
javascript code that is used as fulfillment handler for its "read"
promise the reader can get garbage collected, resulting in the promise
never resolving. This fixes that issue by making
ReadableStreamDefaultReader implement HasPendingActivity to return true
if there are any pending read promises.

Bug: 1092048
Change-Id: Iccf2d6db453c6a27c82542af7a1dc1a2d792c3ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2561043Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832599}

third_party/blink/renderer/core/streams/readable_stream_default_reader.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/streams/readable_stream_default_reader.h"
 
 #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/streams/readable_stream.h"
 #include "third_party/blink/renderer/core/streams/readable_stream_default_controller.h"
 #include "third_party/blink/renderer/core/streams/stream_promise_resolver.h"
@@ -28,7 +29,8 @@ ReadableStreamDefaultReader* ReadableStreamDefaultReader::Create(
 ReadableStreamDefaultReader::ReadableStreamDefaultReader(
     ScriptState* script_state,
     ReadableStream* stream,
-    ExceptionState& exception_state) {
+    ExceptionState& exception_state)
+    : ExecutionContextClient(ExecutionContext::From(script_state)) {
   // https://streams.spec.whatwg.org/#default-reader-constructor
   // 1. Perform ? SetUpReadableStreamDefaultReader(this, stream).
   SetUpDefaultReader(script_state, this, stream, exception_state);
@@ -140,6 +142,11 @@ void ReadableStreamDefaultReader::SetUpDefaultReader(
 void ReadableStreamDefaultReader::Trace(Visitor* visitor) const {
   visitor->Trace(read_requests_);
   ReadableStreamGenericReader::Trace(visitor);
+  ExecutionContextClient::Trace(visitor);
+}
+
+bool ReadableStreamDefaultReader::HasPendingActivity() const {
+  return !read_requests_.empty();
 }
 
 }  // namespace blink

third_party/blink/renderer/core/streams/readable_stream_default_reader.h

@@ -5,7 +5,9 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_READABLE_STREAM_DEFAULT_READER_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_READABLE_STREAM_DEFAULT_READER_H_
 
+#include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
 #include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
 #include "third_party/blink/renderer/core/streams/readable_stream_generic_reader.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
 
@@ -18,7 +20,9 @@ class ScriptState;
 class StreamPromiseResolver;
 
 class CORE_EXPORT ReadableStreamDefaultReader
-    : public ReadableStreamGenericReader {
+    : public ReadableStreamGenericReader,
+      public ActiveScriptWrappable<ReadableStreamDefaultReader>,
+      public ExecutionContextClient {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
@@ -53,6 +57,8 @@ class CORE_EXPORT ReadableStreamDefaultReader
 
   void Trace(Visitor*) const override;
 
+  bool HasPendingActivity() const final;
+
  private:
   friend class ReadableStreamDefaultController;
   friend class ReadableStream;

third_party/blink/renderer/core/streams/readable_stream_default_reader.idl

@@ -4,7 +4,8 @@
 
 // https://streams.spec.whatwg.org/#default-reader-class-definition
 [
-    Exposed=(Window,Worker,Worklet)
+    Exposed=(Window,Worker,Worklet),
+    ActiveScriptWrappable
 ] interface ReadableStreamDefaultReader {
     [CallWith=ScriptState, RaisesException] constructor(ReadableStream stream);
     [CallWith=ScriptState] readonly attribute Promise<void>

third_party/blink/web_tests/SlowTests

@@ -739,8 +739,6 @@ crbug.com/1105270 [ Mac10.15 ] fast/events/open-window-from-another-frame.html [
 crbug.com/1094436 http/tests/devtools/sources/debugger-ui/snippet-edit-breakpoint.js [ Slow ]
 crbug.com/1108423 external/wpt/referrer-policy/gen/worker-classic.http-rp/unset/worker-classic.http.html [ Slow ]
 crbug.com/1112304 external/wpt/html/cross-origin-opener-policy/popup-coop-by-sw-from-coop.https.html [ Slow ]
-crbug.com/1092048 external/wpt/FileAPI/blob/Blob-stream.any.html [ Slow ]
-crbug.com/1092048 external/wpt/FileAPI/blob/Blob-stream.any.worker.html [ Slow ]
 crbug.com/1115091 [ Linux ] external/wpt/secure-payment-confirmation/secure-payment-confirmation.tenative.https.html [ Slow ]
 crbug.com/1135978 [ Linux Release ] fast/dom/css-delete-doc.html [ Slow ]
 crbug.com/1145424 [ Linux ] http/tests/devtools/sources/debugger-ui/reveal-execution-line.js [ Slow ]

third_party/blink/web_tests/TestExpectations

@@ -5977,8 +5977,6 @@ crbug.com/1152496 [ Win ] virtual/oopr-canvas2d/fast/canvas/canvas-composite-sha
 crbug.com/1087242 http/tests/inspector-protocol/service-worker/network-extrainfo-main-request.js [ Pass Failure Timeout Crash ]
 crbug.com/1149771 [ Linux ] virtual/android/fullscreen/video-scrolled-iframe.html [ Pass Timeout ]
 crbug.com/1152532 http/tests/devtools/service-workers/user-agent-override.js [ Pass Timeout ]
-crbug.com/1092048 external/wpt/FileAPI/blob/Blob-stream.any.html [ Pass Timeout ]
-crbug.com/1092048 external/wpt/FileAPI/blob/Blob-stream.any.worker.html [ Pass Timeout ]
 crbug.com/1134459 accessibility/aom-click-action.html [ Pass Timeout ]
 
 #Sheriff 2020-11-26

third_party/blink/web_tests/external/wpt/FileAPI/blob/Blob-stream.any.js

@@ -3,15 +3,25 @@
 // META: script=../../streams/resources/test-utils.js
 'use strict';
 
+// Helper function that triggers garbage collection while reading a chunk
+// if perform_gc is true.
+async function read_and_gc(reader, perform_gc) {
+  const read_promise = reader.read();
+  if (perform_gc)
+    garbageCollect();
+  return read_promise;
+}
+
 // Takes in a ReadableStream and reads from it until it is done, returning
-// an array that contains the results of each read operation
-async function read_all_chunks(stream) {
+// an array that contains the results of each read operation. If perform_gc
+// is true, garbage collection is triggered while reading every chunk.
+async function read_all_chunks(stream, perform_gc = false) {
   assert_true(stream instanceof ReadableStream);
   assert_true('getReader' in stream);
   const reader = stream.getReader();
 
   assert_true('read' in reader);
-  let read_value = await reader.read();
+  let read_value = await read_and_gc(reader, perform_gc);
 
   let out = [];
   let i = 0;
@@ -19,7 +29,7 @@ async function read_all_chunks(stream) {
     for (let val of read_value.value) {
       out[i++] = val;
     }
-    read_value = await reader.read();
+    read_value = await read_and_gc(reader, perform_gc);
   }
   return out;
 }
@@ -56,7 +66,7 @@ promise_test(async() => {
   const stream = blob.stream();
   blob = null;
   garbageCollect();
-  const chunks = await read_all_chunks(stream);
+  const chunks = await read_all_chunks(stream, /*perform_gc=*/true);
   assert_array_equals(chunks, input_arr);
 }, "Blob.stream() garbage collection of blob shouldn't break stream" +
       "consumption")