Commit ca115697 authored by Raymond Toy's avatar Raymond Toy Committed by Commit Bot

Audio thread should not access destination node

The AudioDestinationNode is an object managed by Oilpan so the audio
thread should not access it.  However, the audio thread needs
information (currentTime, etc) from the destination node. So instead
of accessing the audio destination handler (a scoped_refptr) via the
destination node, add a new member to the base audio context that
holds onto the destination handler.

The destination handler is not an oilpan object and lives at least as
long as the base audio context.

Bug: 860626, 860522, 863951
Test: Test case from 860522 doesn't crash on asan build
Change-Id: I3add844d4eb8fdc7e05b89292938b843a0abbb99
Reviewed-on: https://chromium-review.googlesource.com/1138974
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: default avatarHongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575509}
parent 9df35338
...@@ -120,6 +120,14 @@ void BaseAudioContext::Initialize() { ...@@ -120,6 +120,14 @@ void BaseAudioContext::Initialize() {
if (destination_node_) { if (destination_node_) {
destination_node_->Handler().Initialize(); destination_node_->Handler().Initialize();
// TODO(crbug.com/863951). The audio thread needs some things from the
// destination handler like the currentTime. But the audio thread
// shouldn't access the |destination_node_| since it's an Oilpan object.
// Thus, get the destination handler, a non-oilpan object, so we can get
// the items directly from the handler instead of through the destination
// node.
destination_handler_ = &destination_node_->GetAudioDestinationHandler();
// The AudioParams in the listener need access to the destination node, so // The AudioParams in the listener need access to the destination node, so
// only create the listener if the destination node exists. // only create the listener if the destination node exists.
listener_ = AudioListener::Create(*this); listener_ = AudioListener::Create(*this);
......
...@@ -127,24 +127,12 @@ class MODULES_EXPORT BaseAudioContext ...@@ -127,24 +127,12 @@ class MODULES_EXPORT BaseAudioContext
AudioDestinationNode* destination() const; AudioDestinationNode* destination() const;
size_t CurrentSampleFrame() const { size_t CurrentSampleFrame() const {
// TODO(crbug.com/863951): |destination_node| is a GC-mananged object and return destination_handler_->CurrentSampleFrame();
// should not be touched by the audio rendering thread.
return destination_node_ ? destination_node_->GetAudioDestinationHandler()
.CurrentSampleFrame()
: 0;
} }
double currentTime() const { double currentTime() const { return destination_handler_->CurrentTime(); }
// TODO(crbug.com/863951): |destination_node| is a GC-mananged object and
// should not be touched by the audio rendering thread.
return destination_node_
? destination_node_->GetAudioDestinationHandler().CurrentTime()
: 0;
}
float sampleRate() const { float sampleRate() const { return destination_handler_->SampleRate(); }
return destination_node_->GetAudioDestinationHandler().SampleRate();
}
String state() const; String state() const;
AudioContextState ContextState() const { return context_state_; } AudioContextState ContextState() const { return context_state_; }
...@@ -453,6 +441,9 @@ class MODULES_EXPORT BaseAudioContext ...@@ -453,6 +441,9 @@ class MODULES_EXPORT BaseAudioContext
AudioIOPosition output_position_; AudioIOPosition output_position_;
// The handler associated with the above |destination_node_|.
scoped_refptr<AudioDestinationHandler> destination_handler_;
Member<AudioWorklet> audio_worklet_; Member<AudioWorklet> audio_worklet_;
// In order to update some information (e.g. current frame) in // In order to update some information (e.g. current frame) in
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment