Fix/Add content_unittests to work for the code behind --enable-audio-track-processing

content/renderer/media/webrtc_audio_capturer_unittest.cc

Index: content/renderer/media/webrtc_audio_capturer_unittest.cc
diff --git a/content/renderer/media/webrtc_audio_capturer_unittest.cc b/content/renderer/media/webrtc_audio_capturer_unittest.cc
index a56d6da23cd2542ffa9ade851462894f9842b600..a010add51b8a9d67c267c709b7de4223cf1e5d73 100644
--- a/content/renderer/media/webrtc_audio_capturer_unittest.cc
+++ b/content/renderer/media/webrtc_audio_capturer_unittest.cc
@@ -2,7 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/command_line.h"
 #include "base/logging.h"
+#include "content/public/common/content_switches.h"
+#include "content/renderer/media/mock_media_constraint_factory.h"
 #include "content/renderer/media/webrtc/webrtc_local_audio_track_adapter.h"
 #include "content/renderer/media/webrtc_audio_capturer.h"
 #include "content/renderer/media/webrtc_local_audio_track.h"
@@ -38,16 +41,34 @@ class MockPeerConnectionAudioSink : public PeerConnectionAudioSink {
  public:
   MockPeerConnectionAudioSink() {}
   ~MockPeerConnectionAudioSink() {}
-  MOCK_METHOD9(OnData, int(const int16* audio_data,
-                           int sample_rate,
-                           int number_of_channels,
-                           int number_of_frames,
-                           const std::vector<int>& channels,
-                           int audio_delay_milliseconds,
-                           int current_volume,
-                           bool need_audio_processing,
-                           bool key_pressed));
-  MOCK_METHOD1(OnSetFormat, void(const media::AudioParameters& params));
+  virtual int OnData(const int16* audio_data, int sample_rate,
+                     int number_of_channels, int number_of_frames,
+                     const std::vector<int>& channels,
+                     int audio_delay_milliseconds, int current_volume,
+                     bool need_audio_processing, bool key_pressed) OVERRIDE {
+    EXPECT_EQ(sample_rate, params_.sample_rate());
+    EXPECT_EQ(number_of_channels, params_.channels());
+    EXPECT_EQ(number_of_frames, params_.frames_per_buffer());
+    OnDataCallback(audio_data, channels, audio_delay_milliseconds,
+                   current_volume, need_audio_processing, key_pressed);
+    return 0;
+  }
+  MOCK_METHOD6(OnDataCallback, void(const int16* audio_data,
+                                    const std::vector<int>& channels,
+                                    int audio_delay_milliseconds,
+                                    int current_volume,
+                                    bool need_audio_processing,
+                                    bool key_pressed));
+  virtual void OnSetFormat(const media::AudioParameters& params) OVERRIDE {
+    params_ = params;
+    FormatIsSet();
+  }
+  MOCK_METHOD0(FormatIsSet, void());
+
+  const media::AudioParameters& params() const { return params_; }

[tommi (sloooow) - chröme, 2014/03/11 13:30:58]

nit: audio_parameters() const

[no longer working on chromium, 2014/03/11 14:34:44]

:) Just realized that it was not used. Removed.

+
+ private:
+  media::AudioParameters params_;
 };
 
 }  // namespace
@@ -63,14 +84,21 @@ class WebRtcAudioCapturerTest : public testing::Test {
       : params_(media::AudioParameters::AUDIO_PCM_LOW_LATENCY,
                 media::CHANNEL_LAYOUT_STEREO, 48000, 16, 128) {
 #endif
-    blink::WebMediaConstraints constraints;
+  }
+
+  void EnableAudioTrackProcessing() {
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kEnableAudioTrackProcessing);
+  }
+
+  void VerifyAudioParams(const blink::WebMediaConstraints& constraints,
+                         bool need_audio_processing) {
     capturer_ = WebRtcAudioCapturer::CreateCapturer(
         -1, StreamDeviceInfo(MEDIA_DEVICE_AUDIO_CAPTURE,
                              "", "", params_.sample_rate(),
                              params_.channel_layout(),
                              params_.frames_per_buffer()),
-        constraints,
-        NULL);
+        constraints, NULL);
     capturer_source_ = new MockCapturerSource();
     EXPECT_CALL(*capturer_source_.get(), Initialize(_, capturer_.get(), -1));
     capturer_->SetCapturerSourceForTesting(capturer_source_, params_);
@@ -83,6 +111,43 @@ class WebRtcAudioCapturerTest : public testing::Test {
     static_cast<WebRtcLocalAudioSourceProvider*>(
         track_->audio_source_provider())->SetSinkParamsForTesting(params_);
     track_->Start();
+
+    // Connect a mock sink to the track.
+    scoped_ptr<MockPeerConnectionAudioSink> sink(
+        new MockPeerConnectionAudioSink());
+    track_->AddSink(sink.get());
+
+    int delay_ms = 65;
+    bool key_pressed = true;
+    double volume = 0.9;
+    // MaxVolume() in WebRtcAudioCapturer is hard-coded to return 255, we add

[phoglund_chromium, 2014/03/11 14:43:21]

Nit: prefer blank line before comment.

[no longer working on chromium, 2014/03/11 15:22:30]

Done.

+    // 0.5 to do the correct truncation as how the production code does.

[phoglund_chromium, 2014/03/11 14:43:21]

Nit: like the production code does.

[no longer working on chromium, 2014/03/11 15:22:30]

Done.

+    int expected_volume_value = volume * capturer_->MaxVolume() + 0.5;
+    scoped_ptr<media::AudioBus> audio_bus = media::AudioBus::Create(params_);

[phoglund_chromium, 2014/03/11 14:43:21]

Alright, umm, so you do 0.9 * 255 = 229.5 and add the 0.5 to get to 230. I think
I would just hard-code the 230 and explain the value in a comment. If MaxVolume
changes for whatever reason, I think it's fine for this test to break.

[no longer working on chromium, 2014/03/11 15:22:30]

Hard-coding the value is not preferred in any sense.
Also, the current code allows us to verify the behaviour regardless what
MaxVolume() returns. How about keeping the same behaviour?

[phoglund_chromium, 2014/03/11 16:20:34]

I think in tests it makes sense, since I think it makes sense to catch
unexpected changes to MaxVolume. Although that's not the main point of the test,
so it's your call if you want to make that change.

+    audio_bus->Zero();
+
+    media::AudioCapturerSource::CaptureCallback* callback =
+        static_cast<media::AudioCapturerSource::CaptureCallback*>(capturer_);
+    // Verify the sink is getting the correct values.

[phoglund_chromium, 2014/03/11 14:43:21]

Nit: blank line before comment.

[no longer working on chromium, 2014/03/11 15:22:30]

Done.

+    EXPECT_CALL(*sink, FormatIsSet());
+    EXPECT_CALL(*sink,
+                OnDataCallback(_, _, delay_ms, expected_volume_value,
+                               need_audio_processing, key_pressed));
+    callback->Capture(audio_bus.get(), delay_ms, volume, key_pressed);
+
+    // Verify the cached values in the capturer fits what we expect.
+    base::TimeDelta cached_delay;
+    int cached_volume = !expected_volume_value;
+    bool cached_key_pressed = !key_pressed;
+    capturer_->GetAudioProcessingParams(&cached_delay, &cached_volume,
+                                        &cached_key_pressed);
+    EXPECT_EQ(cached_delay.InMilliseconds(), delay_ms);
+    EXPECT_EQ(cached_volume, expected_volume_value);
+    EXPECT_EQ(cached_key_pressed, key_pressed);
+
+    track_->RemoveSink(sink.get());
+    EXPECT_CALL(*capturer_source_.get(), Stop());
+    capturer_->Stop();
   }
 
   media::AudioParameters params_;
@@ -94,49 +159,17 @@ class WebRtcAudioCapturerTest : public testing::Test {
 // Pass the delay value, volume and key_pressed info via capture callback, and
 // those values should be correctly stored and passed to the track.
 TEST_F(WebRtcAudioCapturerTest, VerifyAudioParams) {
-  // Connect a mock sink to the track.
-  scoped_ptr<MockPeerConnectionAudioSink> sink(
-      new MockPeerConnectionAudioSink());
-  track_->AddSink(sink.get());
-
-  int delay_ms = 65;
-  bool key_pressed = true;
-  double volume = 0.9;
-  // MaxVolume() in WebRtcAudioCapturer is hard-coded to return 255, we add 0.5
-  // to do the correct truncation as how the production code does.
-  int expected_volume_value = volume * capturer_->MaxVolume() + 0.5;
-  scoped_ptr<media::AudioBus> audio_bus = media::AudioBus::Create(params_);
-  audio_bus->Zero();
-#if defined(OS_ANDROID)
-  const int expected_buffer_size = params_.sample_rate() / 100;
-#else
-  const int expected_buffer_size = params_.frames_per_buffer();
-#endif
-  bool expected_need_audio_processing = true;
-  media::AudioCapturerSource::CaptureCallback* callback =
-      static_cast<media::AudioCapturerSource::CaptureCallback*>(capturer_);
-  // Verify the sink is getting the correct values.
-  EXPECT_CALL(*sink, OnSetFormat(_));
-  EXPECT_CALL(*sink,
-              OnData(_, params_.sample_rate(), params_.channels(),
-                     expected_buffer_size, _, delay_ms,
-                     expected_volume_value, expected_need_audio_processing,
-                     key_pressed)).Times(AtLeast(1));
-  callback->Capture(audio_bus.get(), delay_ms, volume, key_pressed);
-
-  // Verify the cached values in the capturer fits what we expect.
-  base::TimeDelta cached_delay;
-  int cached_volume = !expected_volume_value;
-  bool cached_key_pressed = !key_pressed;
-  capturer_->GetAudioProcessingParams(&cached_delay, &cached_volume,
-                                      &cached_key_pressed);
-  EXPECT_EQ(cached_delay.InMilliseconds(), delay_ms);
-  EXPECT_EQ(cached_volume, expected_volume_value);
-  EXPECT_EQ(cached_key_pressed, key_pressed);
-
-  track_->RemoveSink(sink.get());
-  EXPECT_CALL(*capturer_source_.get(), Stop());
-  capturer_->Stop();
+  // Use constraints with default settings.
+  blink::WebMediaConstraints constraints;
+  VerifyAudioParams(constraints, true);
+}
+
+TEST_F(WebRtcAudioCapturerTest, VerifyAudioParamsWithAudioProcessing) {
+  EnableAudioTrackProcessing();
+  // Turn off the default constraints.

[phoglund_chromium, 2014/03/11 14:43:21]

Perhaps explain why you turn off the default constraints?

[no longer working on chromium, 2014/03/11 15:22:30]

Done.

+  MockMediaConstraintFactory constraint_factory;
+  constraint_factory.DisableDefaultAudioConstraints();
+  VerifyAudioParams(constraint_factory.CreateWebMediaConstraints(), false);
 }
 
 }  // namespace content