Continue to process AnalyserNode if inputs are silent.

Continue to process AnalyserNode if inputs are silent.

If the input to an AnalyserNode is not connected or it is silent
(because of the silent hint), we must still process the node so that
the internal state is updated with the silent input.

This requires several things to be changed:

1. propagatesSilence() must return false so that
   AnalyserNode::process() is called even if the input is silent.
2. Can't disable output of an AnalyserNode when the input
   disconnects.  If the output were disabled, the node would not get
   called to process any more.
3. If the input is disconnected and the output is not connected, we
   cannot remove the node from the automatic pull list because then
   process() stops getting called.

Basically, process() must always get called as long as the AnalyserNode 
is alive.

BUG=683188
TEST=Analyser/handle-silent-inputs.html

Review-Url: https://codereview.chromium.org/2666063003
Cr-Commit-Position: refs/heads/master@{#449379}
Committed: https://chromium.googlesource.com/chromium/src/+/c9ef493e1d238a655011a872886903a1d44d52bd

[Raymond Toy, 2017-01-31 19:46:29]

Description was changed from

==========
Continue to process AnalyserNode if inputs are silent.

If the input to an AnalyserNode is not connected or it is silent
(because of the silent hint), we must still process the node so that
the internal state is updated with the silent input.

This requires several things to be changed:

1. propagatesSilence() must return false so that
   AnalyserNode::process() is called even if the input is silent.
2. Can't disable output of an AnalyserNode when the input
   disconnects.  If the output were disabled, the node would not get
   called to process any more.
3. If the input is disconnected and the output is not connected, we
   cannot remove the node from the automatic pull list because then
   process() stops getting called.

Basically, process() must always get called as long as it's alive.

BUG=683188
TEST=Analyser/handle-silent-inputs.html
==========

to

==========
Continue to process AnalyserNode if inputs are silent.

If the input to an AnalyserNode is not connected or it is silent
(because of the silent hint), we must still process the node so that
the internal state is updated with the silent input.

This requires several things to be changed:

1. propagatesSilence() must return false so that
   AnalyserNode::process() is called even if the input is silent.
2. Can't disable output of an AnalyserNode when the input
   disconnects.  If the output were disabled, the node would not get
   called to process any more.
3. If the input is disconnected and the output is not connected, we
   cannot remove the node from the automatic pull list because then
   process() stops getting called.

Basically, process() must always get called as long as the AnalyserNode 
is alive.

BUG=683188
TEST=Analyser/handle-silent-inputs.html
==========

[Raymond Toy, 2017-01-31 19:47:02]

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

[Raymond Toy, 2017-01-31 19:47:03]

PTAL

[hongchan, 2017-02-01 01:12:11]

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:58: // Must update
the internal state even if there are no inputs.
Not sure where this comment should be placed.

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp
(right):

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp:124: //
right time and FFT data.
We should at least consider refactoring this method; we can extract the common
part to the parent class, and leave the detail in the derived one. Not an easy
task, but we should try.

[Raymond Toy, 2017-02-01 21:21:02]

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp
(right):

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioBasicInspectorNode.cpp:124: //
right time and FFT data.
On 2017/02/01 01:12:11, hongchan wrote:
> We should at least consider refactoring this method; we can extract the common
> part to the parent class, and leave the detail in the derived one. Not an easy
> task, but we should try.

Done.  But a bit of duplication.

[Raymond Toy, 2017-02-01 21:23:47]

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:58: // Must update
the internal state even if there are no inputs.
On 2017/02/01 01:12:11, hongchan wrote:
> Not sure where this comment should be placed.

Would it help to rephrase it? Or just remove it? (But I think it's important to
say that we need to process the input because line 64 handles the case where
there are no inputs connected.)

[hongchan, 2017-02-03 18:47:40]

lgtm with nit

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:58: // Must update
the internal state even if there are no inputs.
On 2017/02/01 21:23:47, Raymond Toy wrote:
> On 2017/02/01 01:12:11, hongchan wrote:
> > Not sure where this comment should be placed.
> 
> Would it help to rephrase it? Or just remove it? (But I think it's important
to
> say that we need to process the input because line 64 handles the case where
> there are no inputs connected.)

The comment is not really relevant to what's happening at l.59. Isn't it? That
means this will confuse us when we come back later.

[Raymond Toy, 2017-02-03 18:50:00]

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp (right):

https://codereview.chromium.org/2666063003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AnalyserNode.cpp:58: // Must update
the internal state even if there are no inputs.
On 2017/02/03 18:47:40, hongchan wrote:
> On 2017/02/01 21:23:47, Raymond Toy wrote:
> > On 2017/02/01 01:12:11, hongchan wrote:
> > > Not sure where this comment should be placed.
> > 
> > Would it help to rephrase it? Or just remove it? (But I think it's important
> to
> > say that we need to process the input because line 64 handles the case where
> > there are no inputs connected.)
> 
> The comment is not really relevant to what's happening at l.59. Isn't it? That
> means this will confuse us when we come back later.

How about:

// Must always update internal state with any inputs.

[Raymond Toy, 2017-02-09 18:19:13]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2017-02-09 18:19:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/2666063003/#ps50001
(title: "Adjust comment per review.")

[commit-bot: I haz the power, 2017-02-09 18:19:55]

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-09 19:58:30]

CQ is committing da patch.

Bot data: {"patchset_id": 50001, "attempt_start_ts": 1486664353052000,
"parent_rev": "860ef963e7ecec486cce0690a1907581a9d1e581", "commit_rev":
"c9ef493e1d238a655011a872886903a1d44d52bd"}

[commit-bot: I haz the power, 2017-02-09 20:00:03]

Description was changed from

==========
Continue to process AnalyserNode if inputs are silent.

If the input to an AnalyserNode is not connected or it is silent
(because of the silent hint), we must still process the node so that
the internal state is updated with the silent input.

This requires several things to be changed:

1. propagatesSilence() must return false so that
   AnalyserNode::process() is called even if the input is silent.
2. Can't disable output of an AnalyserNode when the input
   disconnects.  If the output were disabled, the node would not get
   called to process any more.
3. If the input is disconnected and the output is not connected, we
   cannot remove the node from the automatic pull list because then
   process() stops getting called.

Basically, process() must always get called as long as the AnalyserNode 
is alive.

BUG=683188
TEST=Analyser/handle-silent-inputs.html
==========

to

==========
Continue to process AnalyserNode if inputs are silent.

If the input to an AnalyserNode is not connected or it is silent
(because of the silent hint), we must still process the node so that
the internal state is updated with the silent input.

This requires several things to be changed:

1. propagatesSilence() must return false so that
   AnalyserNode::process() is called even if the input is silent.
2. Can't disable output of an AnalyserNode when the input
   disconnects.  If the output were disabled, the node would not get
   called to process any more.
3. If the input is disconnected and the output is not connected, we
   cannot remove the node from the automatic pull list because then
   process() stops getting called.

Basically, process() must always get called as long as the AnalyserNode 
is alive.

BUG=683188
TEST=Analyser/handle-silent-inputs.html

Review-Url: https://codereview.chromium.org/2666063003
Cr-Commit-Position: refs/heads/master@{#449379}
Committed:
https://chromium.googlesource.com/chromium/src/+/c9ef493e1d238a655011a8728869...
==========

[commit-bot: I haz the power, 2017-02-09 20:00:05]

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