LibWeb: Use alternative workaround for null strategy algorithm on abort

This unfortunately caused a regression for the included WPT test. Instead of reordering the spec step, fall back to the default size strategy of 1.
Author: https://github.com/shannonbooth Commit: https://github.com/LadybirdBrowser/ladybird/commit/c04b14d0cb4 Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/2311
2024-11-21 23:20:20 +00:00 · 2024-11-14 01:00:24 +13:00 · 2024-11-14 01:00:24 +13:00 · c04b14d0cb · 2024-11-13 15:45:13 +00:00
commit c04b14d0cb
parent c64521aa4f
4 changed files with 133 additions and 6 deletions
--- a/Libraries/LibWeb/Streams/AbstractOperations.cpp
+++ b/Libraries/LibWeb/Streams/AbstractOperations.cpp
@ -4255,7 +4255,7 @@ JS::NonnullGCPtr<WebIDL::Promise> writable_stream_default_writer_write(WritableS
    auto controller = stream->controller();

    // 4. Let chunkSize be ! WritableStreamDefaultControllerGetChunkSize(controller, chunk).
-    // NOTE: See FIXME below
+    auto chunk_size = writable_stream_default_controller_get_chunk_size(*controller, chunk);

    // 5. If stream is not equal to writer.[[stream]], return a promise rejected with a TypeError exception.
    if (stream.ptr() != writer.stream().ptr()) {
@ -4280,11 +4280,6 @@ JS::NonnullGCPtr<WebIDL::Promise> writable_stream_default_writer_write(WritableS
    if (state == WritableStream::State::Erroring)
        return WebIDL::create_rejected_promise(realm, stream->stored_error());

-    // 4. Let chunkSize be ! WritableStreamDefaultControllerGetChunkSize(controller, chunk).
-    // FIXME: This is out of order due to a spec bug: https://github.com/whatwg/streams/issues/1331
-    //        An abort clears the strategySizeAlgorithm, so we need to move this past the "erroring" check
-    auto chunk_size = writable_stream_default_controller_get_chunk_size(*controller, chunk);
-
    // 10. Assert: state is "writable".
    VERIFY(state == WritableStream::State::Writable);

@ -4559,6 +4554,11 @@ bool writable_stream_default_controller_get_backpressure(WritableStreamDefaultCo
 // https://streams.spec.whatwg.org/#writable-stream-default-controller-get-chunk-size
 JS::Value writable_stream_default_controller_get_chunk_size(WritableStreamDefaultController& controller, JS::Value chunk)
 {
+    // FIXME: This null check is due to a spec bug: https://github.com/whatwg/streams/issues/1331
+    //        An abort clears the strategySizeAlgorithm, so fall back to default value if we don't have any algorithm.
+    if (!controller.strategy_size_algorithm())
+        return JS::Value { 1.0 };
+
    // 1. Let returnValue be the result of performing controller.[[strategySizeAlgorithm]], passing in chunk, and interpreting the result as a completion record.
    auto return_value = controller.strategy_size_algorithm()->function()(chunk);

--- a/Tests/LibWeb/Text/expected/wpt-import/streams/writable-streams/bad-strategies.any.txt
+++ b/Tests/LibWeb/Text/expected/wpt-import/streams/writable-streams/bad-strategies.any.txt
@ -0,0 +1,17 @@
+Summary
+
+Harness status: OK
+
+Rerun
+
+Found 7 tests
+
+7 Pass
+Details
+Result	Test Name	MessagePass	Writable stream: throwing strategy.size getter	
+Pass	reject any non-function value for strategy.size	
+Pass	Writable stream: throwing strategy.highWaterMark getter	
+Pass	Writable stream: invalid strategy.highWaterMark	
+Pass	Writable stream: throwing strategy.size method	
+Pass	Writable stream: invalid strategy.size return value	
+Pass	Writable stream: invalid size beats invalid highWaterMark	
--- a/Tests/LibWeb/Text/input/wpt-import/streams/writable-streams/bad-strategies.any.html
+++ b/Tests/LibWeb/Text/input/wpt-import/streams/writable-streams/bad-strategies.any.html
@ -0,0 +1,15 @@
+<!doctype html>
+<meta charset=utf-8>
+
+<script>
+self.GLOBAL = {
+  isWindow: function() { return true; },
+  isWorker: function() { return false; },
+  isShadowRealm: function() { return false; },
+};
+</script>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+
+<div id=log></div>
+<script src="../../streams/writable-streams/bad-strategies.any.js"></script>
--- a/Tests/LibWeb/Text/input/wpt-import/streams/writable-streams/bad-strategies.any.js
+++ b/Tests/LibWeb/Text/input/wpt-import/streams/writable-streams/bad-strategies.any.js
@ -0,0 +1,95 @@
+// META: global=window,worker,shadowrealm
+'use strict';
+
+const error1 = new Error('a unique string');
+error1.name = 'error1';
+
+test(() => {
+  assert_throws_exactly(error1, () => {
+    new WritableStream({}, {
+      get size() {
+        throw error1;
+      },
+      highWaterMark: 5
+    });
+  }, 'construction should re-throw the error');
+}, 'Writable stream: throwing strategy.size getter');
+
+test(() => {
+  assert_throws_js(TypeError, () => {
+    new WritableStream({}, { size: 'a string' });
+  });
+}, 'reject any non-function value for strategy.size');
+
+test(() => {
+  assert_throws_exactly(error1, () => {
+    new WritableStream({}, {
+      size() {
+        return 1;
+      },
+      get highWaterMark() {
+        throw error1;
+      }
+    });
+  }, 'construction should re-throw the error');
+}, 'Writable stream: throwing strategy.highWaterMark getter');
+
+test(() => {
+
+  for (const highWaterMark of [-1, -Infinity, NaN, 'foo', {}]) {
+    assert_throws_js(RangeError, () => {
+      new WritableStream({}, {
+        size() {
+          return 1;
+        },
+        highWaterMark
+      });
+    }, `construction should throw a RangeError for ${highWaterMark}`);
+  }
+}, 'Writable stream: invalid strategy.highWaterMark');
+
+promise_test(t => {
+  const ws = new WritableStream({}, {
+    size() {
+      throw error1;
+    },
+    highWaterMark: 5
+  });
+
+  const writer = ws.getWriter();
+
+  const p1 = promise_rejects_exactly(t, error1, writer.write('a'), 'write should reject with the thrown error');
+
+  const p2 = promise_rejects_exactly(t, error1, writer.closed, 'closed should reject with the thrown error');
+
+  return Promise.all([p1, p2]);
+}, 'Writable stream: throwing strategy.size method');
+
+promise_test(() => {
+  const sizes = [NaN, -Infinity, Infinity, -1];
+  return Promise.all(sizes.map(size => {
+    const ws = new WritableStream({}, {
+      size() {
+        return size;
+      },
+      highWaterMark: 5
+    });
+
+    const writer = ws.getWriter();
+
+    return writer.write('a').then(() => assert_unreached('write must reject'), writeE => {
+      assert_equals(writeE.name, 'RangeError', `write must reject with a RangeError for ${size}`);
+
+      return writer.closed.then(() => assert_unreached('write must reject'), closedE => {
+        assert_equals(closedE, writeE, `closed should reject with the same error as write`);
+      });
+    });
+  }));
+}, 'Writable stream: invalid strategy.size return value');
+
+test(() => {
+  assert_throws_js(TypeError, () => new WritableStream(undefined, {
+    size: 'not a function',
+    highWaterMark: NaN
+  }), 'WritableStream constructor should throw a TypeError');
+}, 'Writable stream: invalid size beats invalid highWaterMark');