Bindings: Handle length changes when converting arrays to sequences

A fast-path for Array to sequence<> conversion assumed that the
length would remain constant. This failed some WPTs that mutate
the array during conversion itself (via toString() overrides.

Modify the fast-path to observe length changes, and add tests.

Bug: 977305
Change-Id: I1206c9475813868f5e26c690ea3a1d210bbeb7df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670055Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671342}

third_party/blink/renderer/bindings/core/v8/native_value_traits_impl.h

@@ -452,13 +452,14 @@ struct NativeValueTraits<IDLSequence<T>>
     }
     result.ReserveInitialCapacity(length);
     v8::TryCatch block(isolate);
-    for (uint32_t i = 0; i < length; ++i) {
+    // Array length may change if array is mutated during iteration.
+    for (uint32_t i = 0; i < v8_array->Length(); ++i) {
       v8::Local<v8::Value> element;
       if (!v8_array->Get(isolate->GetCurrentContext(), i).ToLocal(&element)) {
         exception_state.RethrowV8Exception(block.Exception());
         return;
       }
-      result.UncheckedAppend(
+      result.push_back(
           NativeValueTraits<T>::NativeValue(isolate, element, exception_state));
       if (exception_state.HadException())
         return;

third_party/blink/web_tests/bindings/sequence-type.html

@@ -204,4 +204,59 @@
                        "Changing the original object does not change the sequence value");
    }
  }, "Converting DOM object sequences");
+
+ test(() => {
+   const sequenceTest = internals.sequenceTest();
+
+   const array = [
+     1,
+     {valueOf: () => { array.pop(); return 2; }},
+     3
+   ];
+   assert_array_equals(
+     sequenceTest.identityLongSequence(array),
+     [1, 2],
+     "Array iteration should respect new length");
+ }, "Array to sequence<> conversions with pop() during iteration");
+
+ test(() => {
+   const sequenceTest = internals.sequenceTest();
+
+   const array = [
+     1,
+     {valueOf: () => { array.push(3); return 2; }},
+   ];
+   assert_array_equals(
+     sequenceTest.identityLongSequence(array),
+     [1, 2, 3],
+     "Array iteration should respect new length");
+ }, "Array to sequence<> conversions with push() during iteration");
+
+ test(() => {
+   const sequenceTest = internals.sequenceTest();
+
+   const array = [
+     1,
+     {valueOf: () => { array.length = 3; return 2; }}
+   ];
+   assert_array_equals(
+     sequenceTest.identityLongSequence(array),
+     [1, 2, 0],
+     "Array iteration should respect new length");
+ }, "Array to sequence<> conversions with length increase during iteration");
+
+ test(() => {
+   const sequenceTest = internals.sequenceTest();
+
+   const array = [
+     1,
+     {valueOf: () => { array.length = 2; return 2; }},
+     3
+   ];
+   assert_array_equals(
+     sequenceTest.identityLongSequence(array),
+     [1, 2],
+     "Array iteration should respect new length");
+ }, "Array to sequence<> conversions with length decrease during iteration");
+
 </script>

third_party/blink/web_tests/external/wpt/FileAPI/blob/Blob-constructor-expected.txt

-This is a testharness.js-based test.
-Found 64 tests; 62 PASS, 2 FAIL, 0 TIMEOUT, 0 NOTRUN.
-PASS Blob interface object
-PASS Blob constructor with no arguments
-PASS Blob constructor with no arguments, without 'new'
-PASS Blob constructor without brackets
-PASS Blob constructor with undefined as first argument
-PASS Passing non-objects, Dates and RegExps for blobParts should throw a TypeError.
-PASS A plain object with @@iterator should be treated as a sequence for the blobParts argument.
-PASS A plain object with custom @@iterator should be treated as a sequence for the blobParts argument.
-PASS A plain object with @@iterator and a length property should be treated as a sequence for the blobParts argument.
-PASS A String object should be treated as a sequence for the blobParts argument.
-PASS A Uint8Array object should be treated as a sequence for the blobParts argument.
-PASS The length getter should be invoked and any exceptions should be propagated.
-PASS A platform object that supports indexed properties should be treated as a sequence for the blobParts argument (overwritten 'length'.)
-PASS ToUint32 should be applied to the length and any exceptions should be propagated.
-PASS Getters and value conversions should happen in order until an exception is thrown.
-PASS ToString should be called on elements of the blobParts array and any exceptions should be propagated.
-FAIL Changes to the blobParts array should be reflected in the returned Blob (pop). assert_equals: expected 4 but got 13
-FAIL Changes to the blobParts array should be reflected in the returned Blob (unshift). assert_equals: expected 4 but got 2
-PASS ToString should be called on elements of the blobParts array.
-PASS ArrayBuffer elements of the blobParts array should be supported.
-PASS Passing typed arrays as elements of the blobParts array should work.
-PASS Passing a Float64Array as element of the blobParts array should work.
-PASS Passing an platform object that supports indexed properties as the blobParts array should work (select).
-PASS Passing an platform object that supports indexed properties as the blobParts array should work (attributes).
-PASS Passing a FrozenArray as the blobParts array should work (FrozenArray<MessagePort>).
-PASS Array with two blobs
-PASS Array with two buffers
-PASS Array with two bufferviews
-PASS Array with mixed types
-PASS options properties should be accessed in lexicographic order.
-PASS Arguments should be evaluated from left to right.
-PASS Passing null (index 0) for options should use the defaults.
-PASS Passing null (index 0) for options should use the defaults (with newlines).
-PASS Passing undefined (index 1) for options should use the defaults.
-PASS Passing undefined (index 1) for options should use the defaults (with newlines).
-PASS Passing object "[object Object]" (index 2) for options should use the defaults.
-PASS Passing object "[object Object]" (index 2) for options should use the defaults (with newlines).
-PASS Passing object "[object Object]" (index 3) for options should use the defaults.
-PASS Passing object "[object Object]" (index 3) for options should use the defaults (with newlines).
-PASS Passing object "/regex/" (index 4) for options should use the defaults.
-PASS Passing object "/regex/" (index 4) for options should use the defaults (with newlines).
-PASS Passing function "function() {}" (index 5) for options should use the defaults.
-PASS Passing function "function() {}" (index 5) for options should use the defaults (with newlines).
-PASS Passing 123 for options should throw
-PASS Passing 123.4 for options should throw
-PASS Passing true for options should throw
-PASS Passing "abc" for options should throw
-PASS Blob with type ""
-PASS Blob with type "a"
-PASS Blob with type "A"
-PASS Blob with type "text/html"
-PASS Blob with type "TEXT/HTML"
-PASS Blob with type "text/plain;charset=utf-8"
-PASS Blob with type "å"
-PASS Blob with type "𐑾"
-PASS Blob with type " image/gif "
-PASS Blob with type "\timage/gif\t"
-PASS Blob with type "image/gif;"
-PASS Blob with type "İmage/gif"
-PASS Blob with type "ımage/gif"
-PASS Blob with type "image/gif\0"
-PASS Blob with type "unknown/unknown"
-PASS Blob with type "text/plain"
-PASS Blob with type "image/png"
-Harness: the test ran to completion.
-