Support for AudioContextOptions latencyHint.

content/renderer/media/renderer_webaudiodevice_impl_unittest.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/renderer/media/renderer_webaudiodevice_impl.h"
+
+#include "base/bind.h"
+#include "content/renderer/media/audio_device_factory.h"
+#include "media/base/audio_capturer_source.h"
+#include "media/base/mock_audio_renderer_sink.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using testing::_;
+
+namespace content {
+
+namespace {
+
+const int kHardwareSampleRate = 44100;
+const int kHardwareBufferSize = 128;
+const int kRenderFrameId = 100;
+
+int MockFrameIdFromCurrentContext() {
+  return kRenderFrameId;
+}
+
+media::AudioParameters MockGetOutputDeviceParameters(
+    int frame_id,
+    int session_id,
+    const std::string& device_id,
+    const url::Origin& security_origin) {
+  return media::AudioParameters(media::AudioParameters::AUDIO_PCM_LOW_LATENCY,
+                                media::CHANNEL_LAYOUT_STEREO,
+                                kHardwareSampleRate, 16, kHardwareBufferSize);
+}
+
+class MockRendererWebAudioDeviceImpl : public RendererWebAudioDeviceImpl {

[o1ka, 2016/12/07 20:56:01]

Nit: such class is usually called  FooUnderTest. (this is not a mock, this is
the real class we are testing).

(BTW I realized that an alternative to passing callbacks as constructor
parameters may be wrapping static calls in RendererWebAudioDevice virtual
functions, and overloading them in this "UnderTest" object - since you have it
anyways. This may look a bit neater -  but up to you, current approach is fine.)

[Andrew MacPherson, 2016/12/08 10:06:12]

Done, fixed the class name. Re: virtual methods I had considered that but as the
methods are used in the WebAudioDevice constructor I believe that C++ would not
call the most-derived version of the virtual method. Not 100% sure here though
and if you're ok with the current approach I'll leave it as-is. :)

[o1ka, 2016/12/08 10:57:19]

Sounds good.

+ public:
+  MockRendererWebAudioDeviceImpl(
+      media::ChannelLayout layout,
+      const blink::WebAudioLatencyHint& latency_hint,
+      blink::WebAudioDevice::RenderCallback* callback,
+      int session_id,
+      const url::Origin& security_origin)
+      : RendererWebAudioDeviceImpl(layout,
+                                   latency_hint,
+                                   callback,
+                                   session_id,
+                                   security_origin,
+                                   base::Bind(&MockGetOutputDeviceParameters),
+                                   base::Bind(&MockFrameIdFromCurrentContext)) {
+  }
+};
+
+}  // namespace
+
+class RendererWebAudioDeviceImplTest
+    : public blink::WebAudioDevice::RenderCallback,
+      public AudioDeviceFactory,
+      public testing::Test {
+ public:
+  MOCK_METHOD3(render,
+               void(const blink::WebVector<float*>& sourceData,
+                    const blink::WebVector<float*>& destinationData,
+                    size_t numberOfFrames));
+
+ protected:
+  RendererWebAudioDeviceImplTest() {}
+
+  void SetupDevice(blink::WebAudioLatencyHint latencyHint) {
+    webaudio_device_.reset(new MockRendererWebAudioDeviceImpl(
+        media::CHANNEL_LAYOUT_MONO, latencyHint, this, 0, url::Origin()));
+  }
+
+  MOCK_METHOD1(CreateAudioCapturerSource,
+               scoped_refptr<media::AudioCapturerSource>(int));
+  MOCK_METHOD4(CreateFinalAudioRendererSink,
+               scoped_refptr<media::AudioRendererSink>(int,
+                                                       int,
+                                                       const std::string&,
+                                                       const url::Origin&));
+  MOCK_METHOD5(
+      CreateSwitchableAudioRendererSink,
+      scoped_refptr<media::SwitchableAudioRendererSink>(SourceType,
+                                                        int,
+                                                        int,
+                                                        const std::string&,
+                                                        const url::Origin&));
+
+  scoped_refptr<media::AudioRendererSink> CreateAudioRendererSink(
+      SourceType source_type,
+      int render_frame_id,
+      int session_id,
+      const std::string& device_id,
+      const url::Origin& security_origin) {
+    scoped_refptr<media::MockAudioRendererSink> fake_sink(

[o1ka, 2016/12/07 20:56:01]

Is this local needed?

[Andrew MacPherson, 2016/12/08 10:06:12]

Done.

+        new media::MockAudioRendererSink(
+            device_id, media::OUTPUT_DEVICE_STATUS_OK,
+            MockGetOutputDeviceParameters(render_frame_id, session_id,
+                                          device_id, security_origin)));
+    mock_sink_ = fake_sink;
+
+    EXPECT_CALL(*mock_sink_.get(), Start());

[o1ka, 2016/12/07 20:56:01]

Do you start WebAudioDeviceImpl in each test? Will TestLatencyHint fail?

[Andrew MacPherson, 2016/12/08 10:06:12]

The CreateAudioRendererSink method is only invoked in the
WebAudioDevice::start() method so the LatencyHint tests pass as start() is never
called there.

[o1ka, 2016/12/08 10:57:19]

Acknowledged.

+    EXPECT_CALL(*mock_sink_.get(), Play());
+

[o1ka, 2016/12/07 20:56:01]

We should ensure the sink gets stopped - this is a requirement for sink users.

[Andrew MacPherson, 2016/12/08 10:06:12]

Done, I've moved the EXPECT_CALL for Stop up to where the ones for Start and
Play are. ATM this is never hit because there's a DCHECK(!sink) in the
WebAudioDevice destructor that gets hit first if someone forgets a stop() but
it's there in case this changes.

+    return fake_sink;
+  }
+
+  void TearDown() override {
+    webaudio_device_.reset();
+    mock_sink_ = nullptr;
+  }
+
+  std::unique_ptr<RendererWebAudioDeviceImpl> webaudio_device_;
+  scoped_refptr<media::MockAudioRendererSink> mock_sink_;
+};
+
+TEST_F(RendererWebAudioDeviceImplTest, TestLatencyHint) {
+  SetupDevice(blink::WebAudioLatencyHint(
+      blink::WebAudioLatencyHint::CategoryInteractive));
+  int interactive_frames = webaudio_device_->framesPerBuffer();
+
+  SetupDevice(
+      blink::WebAudioLatencyHint(blink::WebAudioLatencyHint::CategoryBalanced));
+  int balanced_frames = webaudio_device_->framesPerBuffer();
+
+  SetupDevice(
+      blink::WebAudioLatencyHint(blink::WebAudioLatencyHint::CategoryPlayback));
+  int playback_frames = webaudio_device_->framesPerBuffer();
+
+  EXPECT_GT(playback_frames, balanced_frames);
+  EXPECT_GT(balanced_frames, interactive_frames);

[o1ka, 2016/12/07 20:56:01]

Have you considered comparing with media::AudioLatency values?

[Andrew MacPherson, 2016/12/08 10:06:12]

I've added a latencyType() method to the WebAudioDevice and a check to ensure
that the AudioLatency::LatencyType values are ordered as expected. I've also
left in the comparisons on framesPerBuffer sizes, let me know if this is what
you intended!

[o1ka, 2016/12/08 10:57:19]

Sorry, I was unclear. latencyType() method is not needed. I meant comparing
number of frames with AudioLatency::Get... function results.

[Andrew MacPherson, 2016/12/08 12:22:54]

Ah, sorry, I think I read too quickly and misunderstood you. I have updated this
now to compare the frame sizes to the ones returned from the AudioLatency calls.

[o1ka, 2016/12/08 13:10:15]

Great, thanks!

+}
+
+TEST_F(RendererWebAudioDeviceImplTest, TestStartStop) {
+  SetupDevice(blink::WebAudioLatencyHint(
+      blink::WebAudioLatencyHint::CategoryInteractive));
+  webaudio_device_->start();
+  EXPECT_CALL(*mock_sink_.get(), Stop());
+  webaudio_device_->stop();
+}
+

[o1ka, 2016/12/07 20:56:01]

Could you add testing that sampleRate() and framesPerBuffer() return correct
values before and after WAD is started, and after it is stopped? And that a
start-stop-start-stop sequence works correctly.

[Andrew MacPherson, 2016/12/08 10:06:12]

Done, I just modified the very simple StartStop test to start and stop twice and
to validate the framesPerBuffer and sampleRate after each call.

+}  // namespace content