Disallow sequences with lengths exceeding max allocation supported.

Disallow sequences with lengths exceeding max allocation supported.

Vector backing stores are limited in size by the maximum allowed by
their allocator. When converting incoming IDL sequence types into
native arrays, check if the requested size exceed that max limit and
throw a TypeError(), if needed.

Only pathological inputs will run up against this limit and exception. 

R=
BUG=682910
Review-Url: https://codereview.chromium.org/2657173002
Cr-Commit-Position: refs/heads/master@{#447466}
Committed: https://chromium.googlesource.com/chromium/src/+/4da5a6bc55b8e3909b98f3e0f23d7c5d0cb9ecb8

[sof, 2017-01-27 08:52:46]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 08:52:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2017-01-27 08:58:11]

Description was changed from

==========
Disallow vectors with lengths exceeding max allocation supported.

Vector backing stores are limited in size by the maximum allowed by
their allocator. When converting incoming IDL sequence types into
native arrays, check if the requested size exceed that max limit and
throw a TypeError(), if needed.

R=
BUG=682910
==========

to

==========
Disallow vectors with lengths exceeding max allocation supported.

Vector backing stores are limited in size by the maximum allowed by
their allocator. When converting incoming IDL sequence types into
native arrays, check if the requested size exceed that max limit and
throw a TypeError(), if needed.

Only pathological inputs will run up against this limit and exception. 

R=
BUG=682910
==========

[sof, 2017-01-27 10:06:09]

sigbjornf@opera.com changed reviewers:
+ haraken@chromium.org, yutak@chromium.org

[sof, 2017-01-27 10:06:09]

please take a look.

[haraken, 2017-01-27 10:46:05]

haraken@chromium.org changed reviewers:
+ junov@chromium.org

[haraken, 2017-01-27 10:46:06]

+junov, who had been working on implementing a similar issue:

https://codereview.chromium.org/1414553002/

I'm actually not sure what we should do here. What is the spec saying about it?
What are other browsers doing?

[sof, 2017-01-27 11:25:09]

On 2017/01/27 10:46:06, haraken wrote:
> +junov, who had been working on implementing a similar issue:
> 
> https://codereview.chromium.org/1414553002/
> 
> I'm actually not sure what we should do here. What is the spec saying about
it?
> What are other browsers doing?

WebIDL is silent about such operational matters,
https://heycam.github.io/webidl/#create-sequence-from-iterable , but invariably
there will be practical limits on the supported length of sequences that are
convertable.

We could "hide" some of these failures by not reserving capacity, which would
let input sequences with holes to fail with another exception when those are
encountered (Gecko appears to do this.) But that's just moving failure reporting
around for a subset of the input range.

[commit-bot: I haz the power, 2017-01-27 12:01:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-27 12:01:57]

Dry run: This issue passed the CQ dry run.

[sof, 2017-01-27 14:05:33]

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.h (right):

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.h:666:
exceptionState.throwTypeError("Array length exceeds supported limit.");
Throwing RangeError is an option, and would be consistent with what's done
elsewhere.

[sof, 2017-01-28 08:00:33]

For reference/context, the limit being imposed here on sequence lengths is 2^24
(64-bit target) when passing them into Blink.

[sof, 2017-01-28 08:01:26]

Description was changed from

==========
Disallow vectors with lengths exceeding max allocation supported.

Vector backing stores are limited in size by the maximum allowed by
their allocator. When converting incoming IDL sequence types into
native arrays, check if the requested size exceed that max limit and
throw a TypeError(), if needed.

Only pathological inputs will run up against this limit and exception. 

R=
BUG=682910
==========

to

==========
Disallow sequences with lengths exceeding max allocation supported.

Vector backing stores are limited in size by the maximum allowed by
their allocator. When converting incoming IDL sequence types into
native arrays, check if the requested size exceed that max limit and
throw a TypeError(), if needed.

Only pathological inputs will run up against this limit and exception. 

R=
BUG=682910
==========

[Yuta Kitamura, 2017-01-30 07:19:42]

lgtm

wtf/ LGTM.

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/Vector.h (right):

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/Vector.h:338: DCHECK(newCapacity <=
nit+optional: Use DCHECK_LE?

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/allocator/PartitionAllocator.h (right):

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/allocator/PartitionAllocator.h:37: CHECK(count <=
maxElementCountInBackingStore<T>());
nit+optional: Use CHECK_LE?

[Justin Novosad, 2017-01-30 23:10:48]

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.h (right):

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.h:666:
exceptionState.throwTypeError("Array length exceeds supported limit.");
On 2017/01/27 14:05:33, sof wrote:
> Throwing RangeError is an option, and would be consistent with what's done
> elsewhere.

+1
The ES6 considers 2^32-1 to be the supported limit for array lengths:
http://www.ecma-international.org/ecma-262/6.0/#sec-array-len
I think implementations that have a supported limit that is different from that
should do that same thing as if the requested length were above 2^32-1, which is
to throw a RangeError exception.

[sof, 2017-01-31 18:33:53]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 18:34:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-31 20:26:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 20:26:46]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sof, 2017-01-31 20:58:13]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 20:58:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2017-01-31 21:01:43]

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.h (right):

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.h:666:
exceptionState.throwTypeError("Array length exceeds supported limit.");
On 2017/01/30 23:10:48, Justin Novosad wrote:
> On 2017/01/27 14:05:33, sof wrote:
> > Throwing RangeError is an option, and would be consistent with what's done
> > elsewhere.
> 
> +1
> The ES6 considers 2^32-1 to be the supported limit for array lengths:
> http://www.ecma-international.org/ecma-262/6.0/#sec-array-len
> I think implementations that have a supported limit that is different from
that
> should do that same thing as if the requested length were above 2^32-1, which
is
> to throw a RangeError exception.

Thanks, better use RangeError, so let's. I don't it represents a limitation for
real use in these particular cases.

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/Vector.h (right):

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/Vector.h:338: DCHECK(newCapacity <=
On 2017/01/30 07:19:42, Yuta Kitamura wrote:
> nit+optional: Use DCHECK_LE?

yes, done.

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/allocator/PartitionAllocator.h (right):

https://codereview.chromium.org/2657173002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/allocator/PartitionAllocator.h:37: CHECK(count <=
maxElementCountInBackingStore<T>());
On 2017/01/30 07:19:42, Yuta Kitamura wrote:
> nit+optional: Use CHECK_LE?

Done.

[commit-bot: I haz the power, 2017-02-01 02:21:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-01 02:21:25]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-02-01 04:00:23]

LGTM

[sof, 2017-02-01 07:10:00]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2017-02-01 07:10:00]

The patchset sent to the CQ was uploaded after l-g-t-m from yutak@chromium.org
Link to the patchset: https://codereview.chromium.org/2657173002/#ps60001
(title: "re-add expected output")

[commit-bot: I haz the power, 2017-02-01 07:10:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 07:14:36]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485933000362850,
"parent_rev": "2d184d931166e89d8163c78f945fa168afd080d2", "commit_rev":
"4da5a6bc55b8e3909b98f3e0f23d7c5d0cb9ecb8"}

[commit-bot: I haz the power, 2017-02-01 07:15:12]

Description was changed from

==========
Disallow sequences with lengths exceeding max allocation supported.

Vector backing stores are limited in size by the maximum allowed by
their allocator. When converting incoming IDL sequence types into
native arrays, check if the requested size exceed that max limit and
throw a TypeError(), if needed.

Only pathological inputs will run up against this limit and exception. 

R=
BUG=682910
==========

to

==========
Disallow sequences with lengths exceeding max allocation supported.

Vector backing stores are limited in size by the maximum allowed by
their allocator. When converting incoming IDL sequence types into
native arrays, check if the requested size exceed that max limit and
throw a TypeError(), if needed.

Only pathological inputs will run up against this limit and exception. 

R=
BUG=682910

Review-Url: https://codereview.chromium.org/2657173002
Cr-Commit-Position: refs/heads/master@{#447466}
Committed:
https://chromium.googlesource.com/chromium/src/+/4da5a6bc55b8e3909b98f3e0f23d...
==========

[commit-bot: I haz the power, 2017-02-01 07:15:14]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/4da5a6bc55b8e3909b98f3e0f23d...