Simplify notification of a dirty listener.

Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

The race from reading from the hash table of panners in AudioListener
and adding to the table from the main thread is also removed.

BUG=612948, 612955, 613332
TEST=none

Committed: https://crrev.com/a6beb9f31fba1c7892fbfb4911f34e5a13492183
Cr-Commit-Position: refs/heads/master@{#395738}

[Raymond Toy, 2016-05-23 23:13:30]

Description was changed from

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
wants to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.

BUG=612948
TEST=none
==========

to

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs to compute a
new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking and setting
of the listener state happens on the audio thread.

BUG=612948
TEST=none
==========

[Raymond Toy, 2016-05-23 23:14:02]

Description was changed from

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs to compute a
new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking and setting
of the listener state happens on the audio thread.

BUG=612948
TEST=none
==========

to

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

BUG=612948
TEST=none
==========

[Raymond Toy, 2016-05-24 16:18:44]

Description was changed from

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

BUG=612948
TEST=none
==========

to

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

The race from reading from the hash table of panners in AudioListener
and adding to the table from the main thread is also removed.


BUG=612948, 612955, 613332
TEST=none
==========

[Raymond Toy, 2016-05-24 16:34:44]

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

[Raymond Toy, 2016-05-24 16:34:45]

PTAL.

Manually verified that both crashes in 613332 are fixed as well as the tests in
612948.

Can't reproduce the issue in 612955, but the code paths in the stack are gone so
that particular issue can't happen anymore.

[hongchan, 2016-05-24 17:00:23]

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

https://codereview.chromium.org/2006563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:37:
AudioListener::AudioListener(AbstractAudioContext& context)
Seems like we're getting context in the constructor. Perhaps we should introduce
m_context and context() getter method in the class?

https://codereview.chromium.org/2006563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:207:
DCHECK(!isMainThread());
Then this can be context()->isAudioThread().

[Raymond Toy, 2016-05-24 17:22:09]

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

https://codereview.chromium.org/2006563003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:37:
AudioListener::AudioListener(AbstractAudioContext& context)
On 2016/05/24 17:00:23, hoch wrote:
> Seems like we're getting context in the constructor. Perhaps we should
introduce
> m_context and context() getter method in the class?

I thought about that, but decided that saving it just so we can assert on the
audio thread isn't worth it adding the extra slot.

[hongchan, 2016-05-24 17:25:07]

On 2016/05/24 17:22:09, Raymond Toy wrote:
>
https://codereview.chromium.org/2006563003/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):
> 
>
https://codereview.chromium.org/2006563003/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:37:
> AudioListener::AudioListener(AbstractAudioContext& context)
> On 2016/05/24 17:00:23, hoch wrote:
> > Seems like we're getting context in the constructor. Perhaps we should
> introduce
> > m_context and context() getter method in the class?
> 
> I thought about that, but decided that saving it just so we can assert on the
> audio thread isn't worth it adding the extra slot.

Hmm. I think make an explicit association between a context and the listener is
beneficial. It is one-to-one relationship anyway. No?

[Raymond Toy, 2016-05-24 17:36:04]

On 2016/05/24 17:25:07, hoch wrote:
> On 2016/05/24 17:22:09, Raymond Toy wrote:
> >
>
https://codereview.chromium.org/2006563003/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/modules/webaudio/AudioListener.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2006563003/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/modules/webaudio/AudioListener.cpp:37:
> > AudioListener::AudioListener(AbstractAudioContext& context)
> > On 2016/05/24 17:00:23, hoch wrote:
> > > Seems like we're getting context in the constructor. Perhaps we should
> > introduce
> > > m_context and context() getter method in the class?
> > 
> > I thought about that, but decided that saving it just so we can assert on
the
> > audio thread isn't worth it adding the extra slot.
> 
> Hmm. I think make an explicit association between a context and the listener
is
> beneficial. It is one-to-one relationship anyway. No?

The context creates the listener, certainly. Adding the link from the listener
to the context is just yet another object that we need to take care of and make
sure it's all handled correctly and everything.

Besides, I think I may have forgotten to add a listenerlock in updateState to
prevent the positions from moving (in the main thread) while computing the dirty
state.

[hongchan, 2016-05-24 18:47:41]

lgtm

https://codereview.chromium.org/2006563003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webaudio/AudioListener.h (right):

https://codereview.chromium.org/2006563003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webaudio/AudioListener.h:182: // the audio
thread.
Thanks for 80-col wrap.

[Raymond Toy, 2016-05-24 18:49:38]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2016-05-24 18:50:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2006563003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2006563003/60001

[commit-bot: I haz the power, 2016-05-24 19:00:42]

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

[commit-bot: I haz the power, 2016-05-24 19:00:43]

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

[Raymond Toy, 2016-05-24 19:48:48]

The CQ bit was checked by rtoy@chromium.org

[Raymond Toy, 2016-05-24 19:48:48]

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/2006563003/#ps80001
(title: "Fix typo")

[commit-bot: I haz the power, 2016-05-24 19:49:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2006563003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2006563003/80001

[commit-bot: I haz the power, 2016-05-24 23:48:18]

Description was changed from

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

The race from reading from the hash table of panners in AudioListener
and adding to the table from the main thread is also removed.


BUG=612948, 612955, 613332
TEST=none
==========

to

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

The race from reading from the hash table of panners in AudioListener
and adding to the table from the main thread is also removed.


BUG=612948, 612955, 613332
TEST=none
==========

[commit-bot: I haz the power, 2016-05-24 23:48:19]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-24 23:49:42]

Description was changed from

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

The race from reading from the hash table of panners in AudioListener
and adding to the table from the main thread is also removed.


BUG=612948, 612955, 613332
TEST=none
==========

to

==========
Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

The race from reading from the hash table of panners in AudioListener
and adding to the table from the main thread is also removed.

BUG=612948, 612955, 613332
TEST=none

Committed: https://crrev.com/a6beb9f31fba1c7892fbfb4911f34e5a13492183
Cr-Commit-Position: refs/heads/master@{#395738}
==========

[commit-bot: I haz the power, 2016-05-24 23:49:43]

Patchset 5 (id:??) landed as
https://crrev.com/a6beb9f31fba1c7892fbfb4911f34e5a13492183
Cr-Commit-Position: refs/heads/master@{#395738}