webaudio: Avoid recursive acquisition of graph mutex due to overriding...

webaudio: Avoid recursive acquisition of graph mutex due to overriding AudioNode::connect and disconnect.

Instead of overriding these members, have them call new virtual functions which
handle only the logic delegated to the derived class. This is executed with the
graph lock already held, and thus it is not necessary to recursively acquire it.

Bug: 856641
Change-Id: I26847e6719b03ecfe203fa4077367672c7656d7f
Reviewed-on: https://chromium-review.googlesource.com/1118922
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573508}

third_party/blink/renderer/modules/webaudio/analyser_node.cc

@@ -147,7 +147,7 @@ void AnalyserHandler::SetSmoothingTimeConstant(
   }
 }
 
-void AnalyserHandler::UpdatePullStatus() {
+void AnalyserHandler::UpdatePullStatusIfNeeded() {
   DCHECK(Context()->IsGraphOwner());
 
   if (Output(0).IsConnected()) {

third_party/blink/renderer/modules/webaudio/analyser_node.h

@@ -80,7 +80,7 @@ class AnalyserHandler final : public AudioBasicInspectorHandler {
   // because the node must get pulled even if there are no inputs or
   // outputs so that the internal state is properly updated with the
   // correct time data.
-  void UpdatePullStatus() override;
+  void UpdatePullStatusIfNeeded() override;
 
   bool RequiresTailProcessing() const final;
   double TailTime() const final;

third_party/blink/renderer/modules/webaudio/audio_basic_inspector_node.cc

@@ -50,30 +50,6 @@ void AudioBasicInspectorHandler::PullInputs(size_t frames_to_process) {
   Input(0).Pull(Output(0).Bus(), frames_to_process);
 }
 
-AudioNode* AudioBasicInspectorNode::connect(AudioNode* destination,
-                                            unsigned output_index,
-                                            unsigned input_index,
-                                            ExceptionState& exception_state) {
-  DCHECK(IsMainThread());
-
-  BaseAudioContext::GraphAutoLocker locker(context());
-
-  AudioNode::connect(destination, output_index, input_index, exception_state);
-  static_cast<AudioBasicInspectorHandler&>(Handler()).UpdatePullStatus();
-
-  return destination;
-}
-
-void AudioBasicInspectorNode::disconnect(unsigned output_index,
-                                         ExceptionState& exception_state) {
-  DCHECK(IsMainThread());
-
-  BaseAudioContext::GraphAutoLocker locker(context());
-
-  AudioNode::disconnect(output_index, exception_state);
-  static_cast<AudioBasicInspectorHandler&>(Handler()).UpdatePullStatus();
-}
-
 void AudioBasicInspectorHandler::CheckNumberOfChannelsForInput(
     AudioNodeInput* input) {
   DCHECK(Context()->IsAudioThread());
@@ -93,10 +69,10 @@ void AudioBasicInspectorHandler::CheckNumberOfChannelsForInput(
 
   AudioHandler::CheckNumberOfChannelsForInput(input);
 
-  UpdatePullStatus();
+  UpdatePullStatusIfNeeded();
 }
 
-void AudioBasicInspectorHandler::UpdatePullStatus() {
+void AudioBasicInspectorHandler::UpdatePullStatusIfNeeded() {
   DCHECK(Context()->IsGraphOwner());
 
   if (Output(0).IsConnected()) {

third_party/blink/renderer/modules/webaudio/audio_basic_inspector_node.h

@@ -30,9 +30,6 @@
 
 namespace blink {
 
-class BaseAudioContext;
-class ExceptionState;
-
 // AudioBasicInspectorNode is an AudioNode with one input and one output where
 // the output might not necessarily connect to another node's input.
 // If the output is not connected to any other node, then the
@@ -54,7 +51,7 @@ class AudioBasicInspectorHandler : public AudioHandler {
   double TailTime() const override { return 0; }
   double LatencyTime() const override { return 0; }
 
-  virtual void UpdatePullStatus();
+  void UpdatePullStatusIfNeeded() override;
 
  protected:
   // When setting to true, AudioBasicInspectorHandler will be pulled
@@ -66,15 +63,6 @@ class AudioBasicInspectorNode : public AudioNode {
  protected:
   explicit AudioBasicInspectorNode(BaseAudioContext& context)
       : AudioNode(context) {}
-
- private:
-  // TODO(tkent): Should AudioBasicInspectorNode override other variants of
-  // connect() and disconnect()?
-  AudioNode* connect(AudioNode*,
-                     unsigned output_index,
-                     unsigned input_index,
-                     ExceptionState&) final;
-  void disconnect(unsigned output_index, ExceptionState&) final;
 };
 
 }  // namespace blink

third_party/blink/renderer/modules/webaudio/audio_node.cc

@@ -730,6 +730,8 @@ AudioNode* AudioNode::connect(AudioNode* destination,
     connected_nodes_[output_index] = new HeapHashSet<Member<AudioNode>>();
   connected_nodes_[output_index]->insert(destination);
 
+  Handler().UpdatePullStatusIfNeeded();
+
   return destination;
 }
 
@@ -773,6 +775,8 @@ void AudioNode::connect(AudioParam* param,
   if (!connected_params_[output_index])
     connected_params_[output_index] = new HeapHashSet<Member<AudioParam>>();
   connected_params_[output_index]->insert(param);
+
+  Handler().UpdatePullStatusIfNeeded();
 }
 
 void AudioNode::DisconnectAllFromOutput(unsigned output_index) {
@@ -812,6 +816,8 @@ void AudioNode::disconnect() {
   // Disconnect all outgoing connections.
   for (unsigned i = 0; i < numberOfOutputs(); ++i)
     DisconnectAllFromOutput(i);
+
+  Handler().UpdatePullStatusIfNeeded();
 }
 
 void AudioNode::disconnect(unsigned output_index,
@@ -831,6 +837,8 @@ void AudioNode::disconnect(unsigned output_index,
   }
   // Disconnect all outgoing connections from the given output.
   DisconnectAllFromOutput(output_index);
+
+  Handler().UpdatePullStatusIfNeeded();
 }
 
 void AudioNode::disconnect(AudioNode* destination,
@@ -859,6 +867,8 @@ void AudioNode::disconnect(AudioNode* destination,
         "the given destination is not connected.");
     return;
   }
+
+  Handler().UpdatePullStatusIfNeeded();
 }
 
 void AudioNode::disconnect(AudioNode* destination,
@@ -895,6 +905,8 @@ void AudioNode::disconnect(AudioNode* destination,
         "output (" + String::Number(output_index) +
             ") is not connected to the given destination.");
   }
+
+  Handler().UpdatePullStatusIfNeeded();
 }
 
 void AudioNode::disconnect(AudioNode* destination,
@@ -934,6 +946,8 @@ void AudioNode::disconnect(AudioNode* destination,
             ") of the destination.");
     return;
   }
+
+  Handler().UpdatePullStatusIfNeeded();
 }
 
 void AudioNode::disconnect(AudioParam* destination_param,
@@ -958,6 +972,8 @@ void AudioNode::disconnect(AudioParam* destination_param,
                                       "the given AudioParam is not connected.");
     return;
   }
+
+  Handler().UpdatePullStatusIfNeeded();
 }
 
 void AudioNode::disconnect(AudioParam* destination_param,
@@ -985,6 +1001,8 @@ void AudioNode::disconnect(AudioParam* destination_param,
             String::Number(output_index) + ") are not connected.");
     return;
   }
+
+  Handler().UpdatePullStatusIfNeeded();
 }
 
 unsigned AudioNode::numberOfInputs() const {

third_party/blink/renderer/modules/webaudio/audio_node.h

@@ -238,6 +238,10 @@ class MODULES_EXPORT AudioHandler : public ThreadSafeRefCounted<AudioHandler> {
   void UpdateChannelCountMode();
   void UpdateChannelInterpretation();
 
+  // Called when this node's outputs may have become connected or disconnected
+  // to handle automatic pull nodes.
+  virtual void UpdatePullStatusIfNeeded() {}
+
  protected:
   // Inputs and outputs must be created before the AudioHandler is
   // initialized.
@@ -315,13 +319,13 @@ class MODULES_EXPORT AudioNode : public EventTargetWithInlineData {
 
   void HandleChannelOptions(const AudioNodeOptions&, ExceptionState&);
 
-  virtual AudioNode* connect(AudioNode*,
-                             unsigned output_index,
-                             unsigned input_index,
-                             ExceptionState&);
+  AudioNode* connect(AudioNode*,
+                     unsigned output_index,
+                     unsigned input_index,
+                     ExceptionState&);
   void connect(AudioParam*, unsigned output_index, ExceptionState&);
   void disconnect();
-  virtual void disconnect(unsigned output_index, ExceptionState&);
+  void disconnect(unsigned output_index, ExceptionState&);
   void disconnect(AudioNode*, ExceptionState&);
   void disconnect(AudioNode*, unsigned output_index, ExceptionState&);
   void disconnect(AudioNode*,