Remove unnecessary attribute "gain" in AudioBuffer and AudioBufferSourceNode interfaces

Remove unnecessary attribute "gain" in AudioBuffer and AudioBufferSourceNode interfaces 

It has been removed in recent specification which is not used in AudioBuffer and AudioBufferSourceNode. 
There was nothing with "gain" attribute. 

http://www.w3.org/TR/webaudio/#AudioBuffer 
http://www.w3.org/TR/webaudio/#AudioBufferSourceNode-section

BUG=350928, 350770
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171073

[KhNo, 2014-03-10 04:14:35]

Please review this patch. :)

[Raymond Toy (Google), 2014-03-10 16:39:46]

On 2014/03/10 04:14:35, KhNo wrote:
> Please review this patch. :)

I have not yet reviewed this, but in general, I would prefer changes in legacy
features to wait until Blink goes fully unprefixed.  This should happen in a few
weeks
at which time we will remove support for webkitAudioContext and friends and also
remove
support for legacy methods and attributes.

[KhNo, 2014-03-10 17:11:59]

On 2014/03/10 16:39:46, Raymond Toy (Google) wrote:
> On 2014/03/10 04:14:35, KhNo wrote:
> > Please review this patch. :)
> 
> I have not yet reviewed this, but in general, I would prefer changes in legacy
> features to wait until Blink goes fully unprefixed.  This should happen in a
few
> weeks
> at which time we will remove support for webkitAudioContext and friends and
also
> remove
> support for legacy methods and attributes.

Yes. I understood what you mentioned. However, "gain" was read-only attribute in
original code.
It can not be set. Just it was created in constructor and no used.
JS developer could do nothing with this attribute. Do we consider legacy mode
for this case ?

[Raymond Toy, 2014-03-10 17:24:59]

On 2014/03/10 17:11:59, KhNo wrote:
> On 2014/03/10 16:39:46, Raymond Toy (Google) wrote:
> > On 2014/03/10 04:14:35, KhNo wrote:
> > > Please review this patch. :)
> > 
> > I have not yet reviewed this, but in general, I would prefer changes in
legacy
> > features to wait until Blink goes fully unprefixed.  This should happen in a
> few
> > weeks
> > at which time we will remove support for webkitAudioContext and friends and
> also
> > remove
> > support for legacy methods and attributes.
> 
> Yes. I understood what you mentioned. However, "gain" was read-only attribute
in
> original code.
> It can not be set. Just it was created in constructor and no used.
> JS developer could do nothing with this attribute. Do we consider legacy mode
> for this case ?

They could read it. I don't know what use it would be to a developer.

But I'd like to do this kind of clean up all at once after removing support for
webkitAudioContext. That makes it really clear that all the legacy stuff is
gone.

[KhNo, 2014-03-10 17:34:33]

On 2014/03/10 17:24:59, Raymond Toy wrote:
> On 2014/03/10 17:11:59, KhNo wrote:
> > On 2014/03/10 16:39:46, Raymond Toy (Google) wrote:
> > > On 2014/03/10 04:14:35, KhNo wrote:
> > > > Please review this patch. :)
> > > 
> > > I have not yet reviewed this, but in general, I would prefer changes in
> legacy
> > > features to wait until Blink goes fully unprefixed.  This should happen in
a
> > few
> > > weeks
> > > at which time we will remove support for webkitAudioContext and friends
and
> > also
> > > remove
> > > support for legacy methods and attributes.
> > 
> > Yes. I understood what you mentioned. However, "gain" was read-only
attribute
> in
> > original code.
> > It can not be set. Just it was created in constructor and no used.
> > JS developer could do nothing with this attribute. Do we consider legacy
mode
> > for this case ?
> 
> They could read it. I don't know what use it would be to a developer.
> 
> But I'd like to do this kind of clean up all at once after removing support
for
> webkitAudioContext. That makes it really clear that all the legacy stuff is
> gone.

I see. Thank you. I will just keep waiting the chance for it.

[Raymond Toy, 2014-03-10 17:46:08]

On 2014/03/10 17:34:33, KhNo wrote:
> On 2014/03/10 17:24:59, Raymond Toy wrote:
> > On 2014/03/10 17:11:59, KhNo wrote:
> > > On 2014/03/10 16:39:46, Raymond Toy (Google) wrote:
> > > > On 2014/03/10 04:14:35, KhNo wrote:
> > > > > Please review this patch. :)
> > > > 
> > > > I have not yet reviewed this, but in general, I would prefer changes in
> > legacy
> > > > features to wait until Blink goes fully unprefixed.  This should happen
in
> a
> > > few
> > > > weeks
> > > > at which time we will remove support for webkitAudioContext and friends
> and
> > > also
> > > > remove
> > > > support for legacy methods and attributes.
> > > 
> > > Yes. I understood what you mentioned. However, "gain" was read-only
> attribute
> > in
> > > original code.
> > > It can not be set. Just it was created in constructor and no used.
> > > JS developer could do nothing with this attribute. Do we consider legacy
> mode
> > > for this case ?
> > 
> > They could read it. I don't know what use it would be to a developer.
> > 
> > But I'd like to do this kind of clean up all at once after removing support
> for
> > webkitAudioContext. That makes it really clear that all the legacy stuff is
> > gone.
> 
> I see. Thank you. I will just keep waiting the chance for it.

When webkitAudioContext is gone, I'll be sure to review this then.

[KhNo, 2014-03-26 12:53:45]

Removed reviewer. Reviwer will be included again, when patch needs a review.

[Raymond Toy, 2014-04-07 20:17:12]

https://codereview.chromium.org/190973010/diff/30001/Source/modules/webaudio/...
File Source/modules/webaudio/AudioBufferSourceNode.cpp (left):

https://codereview.chromium.org/190973010/diff/30001/Source/modules/webaudio/...
Source/modules/webaudio/AudioBufferSourceNode.cpp:138:
outputBus->copyWithGainFrom(*outputBus, &m_lastGain, totalGain);
Do we still need to do some kind of copy here or does outputBus already contain
the desired data?

[KhNo, 2014-04-08 02:27:16]

https://codereview.chromium.org/190973010/diff/30001/Source/modules/webaudio/...
File Source/modules/webaudio/AudioBufferSourceNode.cpp (left):

https://codereview.chromium.org/190973010/diff/30001/Source/modules/webaudio/...
Source/modules/webaudio/AudioBufferSourceNode.cpp:138:
outputBus->copyWithGainFrom(*outputBus, &m_lastGain, totalGain);
On 2014/04/07 20:17:12, Raymond Toy wrote:
> Do we still need to do some kind of copy here or does outputBus already
contain
> the desired data?

It is just for applying gain to outputBus itself.
First parameter is outputBus.
Inside of copyWithGainFrom() it is filtered out if targetGain and lastMixGain
are same.
We don't need to do this.

[Raymond Toy, 2014-04-08 02:35:31]

Lgtm. But wait a bit to make sure webkitAudioContext is no longer supported.
On Apr 7, 2014 7:27 PM, <keonho07.kim@samsung.com> wrote:

>
> https://codereview.chromium.org/190973010/diff/30001/
> Source/modules/webaudio/AudioBufferSourceNode.cpp
> File Source/modules/webaudio/AudioBufferSourceNode.cpp (left):
>
> https://codereview.chromium.org/190973010/diff/30001/
> Source/modules/webaudio/AudioBufferSourceNode.cpp#oldcode138
> Source/modules/webaudio/AudioBufferSourceNode.cpp:138:
> outputBus->copyWithGainFrom(*outputBus, &m_lastGain, totalGain);
> On 2014/04/07 20:17:12, Raymond Toy wrote:
>
>> Do we still need to do some kind of copy here or does outputBus
>>
> already contain
>
>> the desired data?
>>
>
> It is just for applying gain to outputBus itself.
> First parameter is outputBus.
> Inside of copyWithGainFrom() it is filtered out if targetGain and
> lastMixGain are same.
> We don't need to do this.
>
> https://codereview.chromium.org/190973010/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Raymond Toy, 2014-04-08 16:37:52]

lgtm

lgtm using the button.

[KhNo, 2014-04-08 16:51:15]

On 2014/04/08 16:37:52, Raymond Toy wrote:
> lgtm
> 
> lgtm using the button.

Thanks Ray. :)

[KhNo, 2014-04-08 16:51:23]

The CQ bit was checked by keonho07.kim@samsung.com

[commit-bot: I haz the power, 2014-04-08 16:51:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keonho07.kim@samsung.com/190973010/30001

[commit-bot: I haz the power, 2014-04-08 18:34:14]

Change committed as 171073