Replace boolean return value SharedBuffer::getAsBytes with DCHECK

Replace boolean return value SharedBuffer::getAsBytes with DCHECK

SharedBuffer::getAsBytes returns false on failure, but XHR, the only caller,
calls it in a situation where it never fails.

BUG=None
R=tyoshino@chromium.org, hiroshige@chromium.org

Committed: https://crrev.com/32fb9a2770ad1d1b427b3d56351187b5c1d7f25d
Cr-Commit-Position: refs/heads/master@{#434977}

[yhirano, 2016-11-28 02:32:24]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 02:32:33]

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

[yhirano, 2016-11-28 03:34:15]

See
https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=f....

[commit-bot: I haz the power, 2016-11-28 03:51:20]

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

[commit-bot: I haz the power, 2016-11-28 03:51:21]

Dry run: This issue passed the CQ dry run.

[tyoshino (SeeGerritForStatus), 2016-11-28 05:29:33]

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

https://codereview.chromium.org/2530323002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:394:
OOM_CRASH();
This comment was correct when it was written. But now, the condition in the
if-clause is not returning success/fail of buffer allocation. Allocation happens
on L387.

https://chromium.googlesource.com/chromium/src/+/b2da98fba6daaf142757f3aa1b94...
When this comment was written, SharedBuffer::getAsArrayBuffer() contained a
buffer allocation call which is invocation of
ArrayBuffer::createUninitialized(). ArrayBuffer::createUninitialized() returned
nullptr on allocation failure.

https://chromium.googlesource.com/chromium/src/+/3dfbd2ef9d62505887e29f276625...
This change moved the DOMArrayBuffer creation out of getAsArrayBuffer(), but
https://chromium.googlesource.com/chromium/src/+/3dfbd2ef9d62505887e29f276625...
create() was returning nullptr when the given buffer is nullptr. So, it was
still correct.

https://chromium.googlesource.com/chromium/src/+/612660c50e4ae437e379b299fa75...
This change removed the nullptr check code from DOMArrayBuffer. The commit log
is suggesting that we should use createOrNull() when we want to get nullptr on
allocation failure, but it wasn't changed to do so.

https://chromium.googlesource.com/chromium/src/+/4a7f1339c93c0b19ec75779d93a0...
And, on the same day, this change refactored allocation and serialization into
two separate lines.

So, we should:
- call ArrayBuffer::createOrNull(). If it returns nullptr, call OOM_CRASH().
- call DOMArrayBuffer* create(PassRefPtr<WTF::ArrayBuffer> buffer) with the
created ArrayBuffer.
- when getAsBytes() fails, CRASH() as it's bad but not allocation failure.

[yhirano, 2016-11-28 08:01:36]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 08:01:54]

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

[yhirano, 2016-11-28 08:09:18]

Description was changed from

==========
[XHR] Use OOM_CRASH instead of CRASH when failing to allocated ArrayBuffer

BUG=None
R=tyoshino@chromium.org
==========

to

==========
Replace boolean return value SharedBuffer::getAsBytes with DCHECK

SharedBuffer::getAsBytes returns false on failure, but XHR, the only caller,
calls it in a situation it never fails.

BUG=None
R=tyoshino@chromium.org, hiroshige@chromium.org
==========

[yhirano, 2016-11-28 08:09:18]

yhirano@chromium.org changed reviewers:
+ hiroshige@chromium.org

[yhirano, 2016-11-28 08:09:55]

Description was changed from

==========
Replace boolean return value SharedBuffer::getAsBytes with DCHECK

SharedBuffer::getAsBytes returns false on failure, but XHR, the only caller,
calls it in a situation it never fails.

BUG=None
R=tyoshino@chromium.org, hiroshige@chromium.org
==========

to

==========
Replace boolean return value SharedBuffer::getAsBytes with DCHECK

SharedBuffer::getAsBytes returns false on failure, but XHR, the only caller,
calls it in a situation where it never fails.

BUG=None
R=tyoshino@chromium.org, hiroshige@chromium.org
==========

[yhirano, 2016-11-28 08:11:41]

On 2016/11/28 05:29:33, tyoshino wrote:
>
https://codereview.chromium.org/2530323002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp (right):
> 
>
https://codereview.chromium.org/2530323002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp:394:
> OOM_CRASH();
> This comment was correct when it was written. But now, the condition in the
> if-clause is not returning success/fail of buffer allocation. Allocation
happens
> on L387.
> 
>
https://chromium.googlesource.com/chromium/src/+/b2da98fba6daaf142757f3aa1b94...
> When this comment was written, SharedBuffer::getAsArrayBuffer() contained a
> buffer allocation call which is invocation of
> ArrayBuffer::createUninitialized(). ArrayBuffer::createUninitialized()
returned
> nullptr on allocation failure.
> 
>
https://chromium.googlesource.com/chromium/src/+/3dfbd2ef9d62505887e29f276625...
> This change moved the DOMArrayBuffer creation out of getAsArrayBuffer(), but
>
https://chromium.googlesource.com/chromium/src/+/3dfbd2ef9d62505887e29f276625...
> create() was returning nullptr when the given buffer is nullptr. So, it was
> still correct.
> 
>
https://chromium.googlesource.com/chromium/src/+/612660c50e4ae437e379b299fa75...
> This change removed the nullptr check code from DOMArrayBuffer. The commit log
> is suggesting that we should use createOrNull() when we want to get nullptr on
> allocation failure, but it wasn't changed to do so.
> 
>
https://chromium.googlesource.com/chromium/src/+/4a7f1339c93c0b19ec75779d93a0...
> And, on the same day, this change refactored allocation and serialization into
> two separate lines.
> 
> So, we should:
> - call ArrayBuffer::createOrNull(). If it returns nullptr, call OOM_CRASH().
> - call DOMArrayBuffer* create(PassRefPtr<WTF::ArrayBuffer> buffer) with the
> created ArrayBuffer.
> - when getAsBytes() fails, CRASH() as it's bad but not allocation failure.

Thank you! As talked offline, I think we should fix SharedBuffer and
ArrayBuffer. I changed the issue description and added hiroshige@ for
SharedBuffer.

[tyoshino (SeeGerritForStatus), 2016-11-28 08:57:27]

lgtm

[commit-bot: I haz the power, 2016-11-28 10:42:08]

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

[commit-bot: I haz the power, 2016-11-28 10:42:09]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-11-29 08:09:04]

lgtm

[yhirano, 2016-11-29 08:10:45]

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

[yhirano, 2016-11-29 08:10:46]

+haraken@ for OWNER review.

[haraken, 2016-11-29 09:24:13]

LGTM

[yhirano, 2016-11-29 10:10:05]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2016-11-29 10:10:33]

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, 2016-11-29 12:11:44]

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

[commit-bot: I haz the power, 2016-11-29 12:11:45]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yhirano, 2016-11-29 12:16:36]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2016-11-29 12:16:50]

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, 2016-11-29 13:52:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480421796838660,
"parent_rev": "b479f4eabfa1143bac21c927bce5662a78f2e003", "commit_rev":
"631da07933d8f688075eeca2bd781b0f703d362f"}

[commit-bot: I haz the power, 2016-11-29 13:52:27]

Description was changed from

==========
Replace boolean return value SharedBuffer::getAsBytes with DCHECK

SharedBuffer::getAsBytes returns false on failure, but XHR, the only caller,
calls it in a situation where it never fails.

BUG=None
R=tyoshino@chromium.org, hiroshige@chromium.org
==========

to

==========
Replace boolean return value SharedBuffer::getAsBytes with DCHECK

SharedBuffer::getAsBytes returns false on failure, but XHR, the only caller,
calls it in a situation where it never fails.

BUG=None
R=tyoshino@chromium.org, hiroshige@chromium.org
==========

[commit-bot: I haz the power, 2016-11-29 13:52:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-29 13:54:42]

Description was changed from

==========
Replace boolean return value SharedBuffer::getAsBytes with DCHECK

SharedBuffer::getAsBytes returns false on failure, but XHR, the only caller,
calls it in a situation where it never fails.

BUG=None
R=tyoshino@chromium.org, hiroshige@chromium.org
==========

to

==========
Replace boolean return value SharedBuffer::getAsBytes with DCHECK

SharedBuffer::getAsBytes returns false on failure, but XHR, the only caller,
calls it in a situation where it never fails.

BUG=None
R=tyoshino@chromium.org, hiroshige@chromium.org

Committed: https://crrev.com/32fb9a2770ad1d1b427b3d56351187b5c1d7f25d
Cr-Commit-Position: refs/heads/master@{#434977}
==========

[commit-bot: I haz the power, 2016-11-29 13:54:44]

Patchset 2 (id:??) landed as
https://crrev.com/32fb9a2770ad1d1b427b3d56351187b5c1d7f25d
Cr-Commit-Position: refs/heads/master@{#434977}