Support for AudioContextOptions latencyHint.

third_party/WebKit/Source/modules/webaudio/AudioContext.cpp

Index: third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
diff --git a/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp b/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
index 920678225ca55b9c22d8908fa4de6b3d7bce0f17..793180995559f5ad443652ce4be61f74f1b88074 100644
--- a/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
@@ -11,8 +11,10 @@
 #include "core/dom/ExceptionCode.h"
 #include "core/frame/UseCounter.h"
 #include "modules/webaudio/AudioBufferCallback.h"
+#include "modules/webaudio/AudioContextOptions.h"
 #include "platform/Histogram.h"
 #include "platform/audio/AudioUtilities.h"
+#include "public/platform/WebAudioLatencyHint.h"
 
 #if DEBUG_AUDIONODE_REFERENCES
 #include <stdio.h>
@@ -20,6 +22,25 @@
 
 namespace blink {
 
+namespace {
+
+WebAudioLatencyHint::Category LatencyHintCategoryFromString(String category) {

[o1ka, 2016/12/02 12:02:13]

Pass a string into WebAudioLatencyHint constructor and convert to category in
there?

[hongchan, 2016/12/02 17:40:11]

I agree with o1ka here. WebAudioLatencyHint is a part of |platform| and any
platform related task should stay out of the |modules|.

[Andrew MacPherson, 2016/12/05 14:12:52]

Done.

+  WebAudioLatencyHint::Category latencyCategory;
+  if (category == "interactive") {
+    latencyCategory = WebAudioLatencyHint::CategoryInteractive;
+  } else if (category == "balanced") {
+    latencyCategory = WebAudioLatencyHint::CategoryBalanced;
+  } else if (category == "playback") {
+    latencyCategory = WebAudioLatencyHint::CategoryPlayback;
+  } else {
+    NOTREACHED();
+    latencyCategory = WebAudioLatencyHint::CategoryInteractive;
+  }
+  return latencyCategory;
+}
+
+}  // namespace
+
 // Don't allow more than this number of simultaneous AudioContexts
 // talking to hardware.
 const unsigned MaxHardwareContexts = 6;
@@ -27,6 +48,21 @@ static unsigned s_hardwareContextCount = 0;
 static unsigned s_contextId = 0;
 
 AudioContext* AudioContext::create(Document& document,
+                                   const AudioContextOptions& contextOptions,
+                                   ExceptionState& exceptionState) {
+  WebAudioLatencyHint::Category latencyCategory =
+      WebAudioLatencyHint::CategoryInteractive;
+  if (contextOptions.latencyHint().isAudioContextLatencyCategory()) {
+    latencyCategory = LatencyHintCategoryFromString(
+        contextOptions.latencyHint().getAsAudioContextLatencyCategory());
+  }
+  // TODO: add support for latencyHint().isDouble()
+
+  return AudioContext::create(document, latencyCategory, exceptionState);

[o1ka, 2016/12/02 12:02:12]

Is it an implicit conversion in here? This is not really readable, could you
make the constructor explicit?

[hongchan, 2016/12/02 17:40:11]

So basically these two creators are always supposed to be chained like this?
Still not sure why we cannot do this in one call.

[Andrew MacPherson, 2016/12/05 14:12:53]

Done.

[Andrew MacPherson, 2016/12/05 14:12:53]

Done, refactored to a single constructor.

+}
+
+AudioContext* AudioContext::create(Document& document,
+                                   const WebAudioLatencyHint& latencyHint,
                                    ExceptionState& exceptionState) {
   DCHECK(isMainThread());
 
@@ -41,7 +77,7 @@ AudioContext* AudioContext::create(Document& document,
     return nullptr;
   }
 
-  AudioContext* audioContext = new AudioContext(document);
+  AudioContext* audioContext = new AudioContext(document, latencyHint);
   audioContext->suspendIfNeeded();
 
   if (!AudioUtilities::isValidAudioBufferSampleRate(
@@ -83,8 +119,9 @@ AudioContext* AudioContext::create(Document& document,
   return audioContext;
 }
 
-AudioContext::AudioContext(Document& document)
-    : BaseAudioContext(&document), m_contextId(s_contextId++) {}
+AudioContext::AudioContext(Document& document,
+                           const WebAudioLatencyHint& latencyHint)
+    : BaseAudioContext(&document, latencyHint), m_contextId(s_contextId++) {}
 
 AudioContext::~AudioContext() {
 #if DEBUG_AUDIONODE_REFERENCES
@@ -205,4 +242,8 @@ void AudioContext::stopRendering() {
   }
 }
 
+double AudioContext::baseLatency() const {
+  return framesPerBuffer() * 2 / static_cast<double>(sampleRate());

[o1ka, 2016/12/02 12:02:12]

can sampleRate() be zero?

[Raymond Toy, 2016/12/02 16:57:45]

Good catch!  Yes, it can be zero here.  If the context is closed, the sample
rate will be zero, and it's valid (but meaningless?) to query the baseLatency
value on a closed context.

However, see also https://bugs.chromium.org/p/chromium/issues/detail?id=670065. 
I want to fix this soon, and there won't be a divide-by-zero issue anymore.

[Andrew MacPherson, 2016/12/05 14:12:53]

Should that be fixed as part of this CL or should it wait for a subsequent one?
It would be straightforward to just cache the sampleRate in the BaseAudioContext
when the m_destinationNode is created if that makes sense.

[Raymond Toy, 2016/12/05 17:36:43]

We should do something now. My original idea was to cache the value when the
context was closed, not when the context is created.  For the bug that I filed,
we might need to do something more complicated, so you should prevent division
by zero in this CL.

[Andrew MacPherson, 2016/12/06 15:56:19]

Done, I've updated the sampleRate() method to use the existing
closedContextSampleRate() method if there is no m_destinationNode.

[Raymond Toy, 2016/12/06 23:55:40]

Awesome.

It would be great if you could add a layout test for this where you query the
baseLatency after the context is closed.

[Andrew MacPherson, 2016/12/07 10:55:24]

Done.

+}
+
 }  // namespace blink