XMLHttpRequest: return null upon failing responseArrayBuffer allocation.

XMLHttpRequest: return null upon failing responseArrayBuffer allocation.

The allocation of a response ArrayBuffer may fail, a large enough
contiguous chunk of memory simply not being available from the
underlying allocator. The spec [1] now admits allocation failure as a
possibility, allowing the return of a null buffer if so.

Update our implementation accordingly, returning null rather than
failing hard with an OOM.

1 - https://xhr.spec.whatwg.org/#arraybuffer-response

R=haraken,yhirano
BUG=673211
Review-Url: https://codereview.chromium.org/2730943002
Cr-Commit-Position: refs/heads/master@{#455398}
Committed: https://chromium.googlesource.com/chromium/src/+/7c838d986a3d95e81971ef40050fecce0ea2be2c

[sof, 2017-03-03 15:14:03]

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

[commit-bot: I haz the power, 2017-03-03 15:14:16]

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

[sof, 2017-03-03 16:13:07]

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

[sof, 2017-03-03 16:13:07]

please take a look. (haraken@ for wtf/ )

the condition to test is straightforward, but a reliable test that exercises OOM
isn't, hence none included. If anyone has suggestions, happy to hear & act upon.

[commit-bot: I haz the power, 2017-03-03 16:50:06]

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

[commit-bot: I haz the power, 2017-03-03 16:50:07]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-03-03 19:23:27]

LGTM on my side with a comment.

https://codereview.chromium.org/2730943002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/2730943002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:399: // fails.

I guess it would be nicer to set an exception but the spec is not saying to do
so...

[sof, 2017-03-03 21:23:53]

https://codereview.chromium.org/2730943002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/2730943002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:399: // fails.
On 2017/03/03 19:23:26, haraken wrote:
> 
> I guess it would be nicer to set an exception but the spec is not saying to do
> so...

https://github.com/whatwg/xhr/issues/26 has some discussion. That the getter
consistently returns null, makes for a more regular story.

[sof, 2017-03-06 07:35:28]

Description was changed from

==========
XMLHttpRequest: return null upon failing responseArrayBuffer allocation.

The allocation of a response ArrayBuffer may fail, a large enough
contiguous chunk of memory simply not being available from the
underlying allocator. The spec [1] now admits allocation failure as a
possibility, allowing the return of a null buffer if so.

Update our implementation accordingly, returning null rather than
failing hard with an OOM.

1 - https://xhr.spec.whatwg.org/#arraybuffer-response

R=
BUG=673211
==========

to

==========
XMLHttpRequest: return null upon failing responseArrayBuffer allocation.

The allocation of a response ArrayBuffer may fail, a large enough
contiguous chunk of memory simply not being available from the
underlying allocator. The spec [1] now admits allocation failure as a
possibility, allowing the return of a null buffer if so.

Update our implementation accordingly, returning null rather than
failing hard with an OOM.

1 - https://xhr.spec.whatwg.org/#arraybuffer-response

R=haraken
BUG=673211
==========

[sof, 2017-03-06 08:54:07]

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

[commit-bot: I haz the power, 2017-03-06 08:54:21]

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

[yhirano, 2017-03-06 09:19:53]

https://codereview.chromium.org/2730943002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/2730943002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:403:
m_responseArrayBuffer = DOMArrayBuffer::create(nullptr, 0);
This getter returns null for the first time, and will return an empty
DOMArrayBuffer from the second time, right?

[sof, 2017-03-06 09:29:16]

https://codereview.chromium.org/2730943002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):

https://codereview.chromium.org/2730943002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:403:
m_responseArrayBuffer = DOMArrayBuffer::create(nullptr, 0);
On 2017/03/06 09:19:53, yhirano (slow) wrote:
> This getter returns null for the first time, and will return an empty
> DOMArrayBuffer from the second time, right?

Hmm, yes. It implements the spec by doing so, I'd say.

[commit-bot: I haz the power, 2017-03-06 10:30:28]

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

[commit-bot: I haz the power, 2017-03-06 10:30:29]

Dry run: This issue passed the CQ dry run.

[sof, 2017-03-06 16:12:50]

On 2017/03/06 09:29:16, sof wrote:
>
https://codereview.chromium.org/2730943002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):
> 
>
https://codereview.chromium.org/2730943002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:403:
> m_responseArrayBuffer = DOMArrayBuffer::create(nullptr, 0);
> On 2017/03/06 09:19:53, yhirano (slow) wrote:
> > This getter returns null for the first time, and will return an empty
> > DOMArrayBuffer from the second time, right?
> 
> Hmm, yes. It implements the spec by doing so, I'd say.

Attempting to clarify wanted behavior via
https://github.com/whatwg/xhr/issues/26

[sof, 2017-03-07 15:13:42]

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

[commit-bot: I haz the power, 2017-03-07 15:14:12]

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-03-07 16:40:04]

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

[commit-bot: I haz the power, 2017-03-07 16:40:05]

Dry run: This issue passed the CQ dry run.

[sof, 2017-03-07 16:40:43]

On 2017/03/06 16:12:50, sof wrote:
> On 2017/03/06 09:29:16, sof wrote:
> >
>
https://codereview.chromium.org/2730943002/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
(right):
> > 
> >
>
https://codereview.chromium.org/2730943002/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:403:
> > m_responseArrayBuffer = DOMArrayBuffer::create(nullptr, 0);
> > On 2017/03/06 09:19:53, yhirano (slow) wrote:
> > > This getter returns null for the first time, and will return an empty
> > > DOMArrayBuffer from the second time, right?
> > 
> > Hmm, yes. It implements the spec by doing so, I'd say.
> 
> Attempting to clarify wanted behavior via
> https://github.com/whatwg/xhr/issues/26

The spec has now been updated, along with sync'ing this implementation to
conform. Let's move forwards?

[yhirano, 2017-03-08 08:00:04]

lgtm

[sof, 2017-03-08 08:15:52]

Description was changed from

==========
XMLHttpRequest: return null upon failing responseArrayBuffer allocation.

The allocation of a response ArrayBuffer may fail, a large enough
contiguous chunk of memory simply not being available from the
underlying allocator. The spec [1] now admits allocation failure as a
possibility, allowing the return of a null buffer if so.

Update our implementation accordingly, returning null rather than
failing hard with an OOM.

1 - https://xhr.spec.whatwg.org/#arraybuffer-response

R=haraken
BUG=673211
==========

to

==========
XMLHttpRequest: return null upon failing responseArrayBuffer allocation.

The allocation of a response ArrayBuffer may fail, a large enough
contiguous chunk of memory simply not being available from the
underlying allocator. The spec [1] now admits allocation failure as a
possibility, allowing the return of a null buffer if so.

Update our implementation accordingly, returning null rather than
failing hard with an OOM.

1 - https://xhr.spec.whatwg.org/#arraybuffer-response

R=haraken,yhirano
BUG=673211
==========

[sof, 2017-03-08 08:15:59]

The CQ bit was checked by sigbjornf@opera.com

[sof, 2017-03-08 08:15:59]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2730943002/#ps40001
(title: "mirror updated spec, and return |null| for failed allocs")

[commit-bot: I haz the power, 2017-03-08 08:16:27]

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-03-08 08:20:50]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488960959459140,
"parent_rev": "0e1f7f7cc98ccc64094ebcbb0614264c1f2d14cf", "commit_rev":
"7c838d986a3d95e81971ef40050fecce0ea2be2c"}

[commit-bot: I haz the power, 2017-03-08 08:21:30]

Description was changed from

==========
XMLHttpRequest: return null upon failing responseArrayBuffer allocation.

The allocation of a response ArrayBuffer may fail, a large enough
contiguous chunk of memory simply not being available from the
underlying allocator. The spec [1] now admits allocation failure as a
possibility, allowing the return of a null buffer if so.

Update our implementation accordingly, returning null rather than
failing hard with an OOM.

1 - https://xhr.spec.whatwg.org/#arraybuffer-response

R=haraken,yhirano
BUG=673211
==========

to

==========
XMLHttpRequest: return null upon failing responseArrayBuffer allocation.

The allocation of a response ArrayBuffer may fail, a large enough
contiguous chunk of memory simply not being available from the
underlying allocator. The spec [1] now admits allocation failure as a
possibility, allowing the return of a null buffer if so.

Update our implementation accordingly, returning null rather than
failing hard with an OOM.

1 - https://xhr.spec.whatwg.org/#arraybuffer-response

R=haraken,yhirano
BUG=673211

Review-Url: https://codereview.chromium.org/2730943002
Cr-Commit-Position: refs/heads/master@{#455398}
Committed:
https://chromium.googlesource.com/chromium/src/+/7c838d986a3d95e81971ef40050f...
==========

[commit-bot: I haz the power, 2017-03-08 08:21:32]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/7c838d986a3d95e81971ef40050f...