Make AudioBufferSourceNode.buffer setter conforming

Make AudioBufferSourceNode.buffer setter conforming

Make ABSN.buffer setter conform to the spec.  We were already throwing
errors on attempts to set the buffer twice.  To conform, we need to
allow setting the buffer to null, but still throwing an error if the
buffer was ever set to a non-null value.

BUG=591089
TEST=AudioBufferSource/audiobuffersource-channels.html

Review-Url: https://codereview.chromium.org/2842003003
Cr-Commit-Position: refs/heads/master@{#491003}
Committed: https://chromium.googlesource.com/chromium/src/+/1ef23c62889c7c9fa4c93999a364c6939bf4da92

[Raymond Toy, 2017-07-14 18:22:21]

rtoy@chromium.org changed reviewers:
+ hongchan@chromium.org

[Raymond Toy, 2017-07-14 18:22:22]

PTAL.

[Raymond Toy, 2017-07-28 18:35:26]

Ping

[hongchan, 2017-07-28 18:45:33]

lgtm with nit

Sorry for the late response. I missed this in the transition to Gerrit.

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-channels.html
(right):

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-channels.html:42:
should(function() {
We prefer |()=>| over |function()|.

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp
(right):

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:396:
"buffer");
I am not so sure about the term "a non-null buffer". Is it a proper expression?
When you create a AudioBuffer, it cannot be a |null|. So it is always non-null.

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h (right):

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h:186: bool
buffer_has_been_set_;
Add ` = false;` and remove the comment about the default value.

[Raymond Toy, 2017-07-28 19:16:55]

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-channels.html
(right):

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/audiobuffersource-channels.html:42:
should(function() {
On 2017/07/28 at 18:45:32, hongchan wrote:
> We prefer |()=>| over |function()|.

I was fallowing the existing code which was using function().  Let's leave it
and convert function() to ()=> later.

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp
(right):

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.cpp:396:
"buffer");
On 2017/07/28 at 18:45:32, hongchan wrote:
> I am not so sure about the term "a non-null buffer". Is it a proper
expression? When you create a AudioBuffer, it cannot be a |null|. So it is
always non-null.

This is the absn.buffer attribute, so you can set it to random things, including
null.  Should I rephrase the message?

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h (right):

https://codereview.chromium.org/2842003003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioBufferSourceNode.h:186: bool
buffer_has_been_set_;
On 2017/07/28 at 18:45:32, hongchan wrote:
> Add ` = false;` and remove the comment about the default value.

The constructor needs to initialize other things, so I find it very convenient
to initialize everything there, even if we could initialize it somewhere else. 
One less file to look at.

[Raymond Toy, 2017-07-31 19:40:40]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2017-07-31 19:40:53]

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-07-31 20:54:35]

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

[commit-bot: I haz the power, 2017-07-31 20:54:36]

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

[Raymond Toy, 2017-07-31 21:16:45]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2017-07-31 21:16:54]

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-07-31 23:12:35]

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

[commit-bot: I haz the power, 2017-07-31 23:12:36]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Raymond Toy, 2017-08-01 14:35:18]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2017-08-01 14:35:29]

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-08-01 16:09:43]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1501598118278800,
"parent_rev": "e956a69209054e11b273de8433922a119399721e", "commit_rev":
"1ef23c62889c7c9fa4c93999a364c6939bf4da92"}

[commit-bot: I haz the power, 2017-08-01 16:09:57]

Description was changed from

==========
Make AudioBufferSourceNode.buffer setter conforming

Make ABSN.buffer setter conform to the spec.  We were already throwing
errors on attempts to set the buffer twice.  To conform, we need to
allow setting the buffer to null, but still throwing an error if the
buffer was ever set to a non-null value.

BUG=591089
TEST=AudioBufferSource/audiobuffersource-channels.html
==========

to

==========
Make AudioBufferSourceNode.buffer setter conforming

Make ABSN.buffer setter conform to the spec.  We were already throwing
errors on attempts to set the buffer twice.  To conform, we need to
allow setting the buffer to null, but still throwing an error if the
buffer was ever set to a non-null value.

BUG=591089
TEST=AudioBufferSource/audiobuffersource-channels.html

Review-Url: https://codereview.chromium.org/2842003003
Cr-Commit-Position: refs/heads/master@{#491003}
Committed:
https://chromium.googlesource.com/chromium/src/+/1ef23c62889c7c9fa4c93999a364...
==========

[commit-bot: I haz the power, 2017-08-01 16:10:00]

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