Looking up device id by session id for AudioRendererMixerInput

content/renderer/media/audio_renderer_mixer_manager_unittest.cc

Index: content/renderer/media/audio_renderer_mixer_manager_unittest.cc
diff --git a/content/renderer/media/audio_renderer_mixer_manager_unittest.cc b/content/renderer/media/audio_renderer_mixer_manager_unittest.cc
index b1371b222832965662e90c8e084c6c1208c4596b..58ac091985e042cbcea2493b29ca834873866410 100644
--- a/content/renderer/media/audio_renderer_mixer_manager_unittest.cc
+++ b/content/renderer/media/audio_renderer_mixer_manager_unittest.cc
@@ -6,6 +6,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "content/renderer/media/audio_device_factory.h"
 #include "content/renderer/media/audio_renderer_mixer_manager.h"
 #include "ipc/ipc_message.h"
 #include "media/audio/audio_parameters.h"
@@ -25,24 +26,33 @@ static const int kBufferSize = 8192;
 static const media::ChannelLayout kChannelLayout = media::CHANNEL_LAYOUT_STEREO;
 static const media::ChannelLayout kAnotherChannelLayout =
     media::CHANNEL_LAYOUT_2_1;
-static const std::string kDefaultDeviceId;
-static const url::Origin kSecurityOrigin;
+static const char* const kDefaultDeviceId =

[Henrik Grunell, 2016/03/08 21:09:57]

Why not use media::AudioManagerBase::kDefaultDeviceId directly?

[o1ka, 2016/04/05 15:13:38]

Because this is shorter?

+    media::AudioManagerBase::kDefaultDeviceId;
+static const char kAnotherDeviceId[] = "another-device-id";
+static const char kMatchedDeviceId[] = "matched-device-id";
+static const char kNonexistentDeviceId[] = "nonexistent-device-id";
 
 static const int kRenderFrameId = 124;
 static const int kAnotherRenderFrameId = 678;
 
 using media::AudioParameters;
 
-class AudioRendererMixerManagerTest : public testing::Test {
+class AudioRendererMixerManagerTest : public testing::Test,
+                                      public AudioDeviceFactory {
  public:
-  AudioRendererMixerManagerTest() {
-    manager_.reset(new AudioRendererMixerManager());
-
-    // We don't want to deal with instantiating a real AudioOutputDevice since
-    // it's not important to our testing, so we inject a mock.
-    mock_sink_ = new media::MockAudioRendererSink();
-    manager_->SetAudioRendererSinkForTesting(mock_sink_.get());
-  }
+  AudioRendererMixerManagerTest()
+      : manager_(new AudioRendererMixerManager()),
+        mock_sink_(new media::MockAudioRendererMixerSink()),
+        mock_sink_no_device_(new media::MockAudioRendererMixerSink(
+            kNonexistentDeviceId,
+            media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND)),
+        mock_sink_matched_device_(new media::MockAudioRendererMixerSink(
+            kMatchedDeviceId,
+            media::OUTPUT_DEVICE_STATUS_OK)),
+        mock_sink_for_session_id_(new media::MockAudioRendererMixerSink(
+            kMatchedDeviceId,
+            media::OUTPUT_DEVICE_STATUS_OK)),
+        kSecurityOrigin2(GURL("http://localhost")) {}
 
   media::AudioRendererMixer* GetMixer(
       int source_render_frame_id,
@@ -62,20 +72,60 @@ class AudioRendererMixerManagerTest : public testing::Test {
                                  security_origin);
   }
 
-  void UseNonexistentSink() {
-    mock_sink_ = new media::MockAudioRendererSink(
-        media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND);
-    manager_->SetAudioRendererSinkForTesting(mock_sink_.get());
-  }
-
   // Number of instantiated mixers.
   int mixer_count() {
     return manager_->mixers_.size();
   }
 
  protected:
+  MOCK_METHOD1(CreateInputDevice, media::AudioInputDevice*(int));
+  MOCK_METHOD5(CreateRestartableAudioRendererSink,
+               media::RestartableAudioRendererSink*(SourceType,
+                                                    int,
+                                                    int,
+                                                    const std::string&,
+                                                    const url::Origin&));
+  MOCK_METHOD5(CreateAudioRendererSink,
+               media::AudioRendererSink*(SourceType,
+                                         int,
+                                         int,
+                                         const std::string&,
+                                         const url::Origin&));
+
+  media::AudioRendererMixerSink* CreateAudioRendererMixerSink(
+      int render_frame_id,
+      int session_id,
+      const std::string& device_id,
+      const url::Origin& security_origin) {
+    if ((device_id == kDefaultDeviceId) || (device_id == kAnotherDeviceId)) {
+      // We don't care about separate sinks for this devices
+      return mock_sink_.get();
+    }
+    if (device_id == kNonexistentDeviceId) {
+      // The think to return device errror
+      return mock_sink_no_device_.get();
+    }
+    if (device_id.empty()) {
+      // The sink used to get device ID from session ID if it's not empty
+      return session_id ? mock_sink_for_session_id_.get() : mock_sink_.get();
+    }
+    if (device_id == kMatchedDeviceId) {
+      return mock_sink_matched_device_.get();
+    }
+
+    NOTREACHED();
+    return nullptr;
+  }
+
   scoped_ptr<AudioRendererMixerManager> manager_;
-  scoped_refptr<media::MockAudioRendererSink> mock_sink_;
+  scoped_refptr<media::MockAudioRendererMixerSink> mock_sink_;
+  scoped_refptr<media::MockAudioRendererMixerSink> mock_sink_no_device_;
+  scoped_refptr<media::MockAudioRendererMixerSink> mock_sink_matched_device_;
+  scoped_refptr<media::MockAudioRendererMixerSink> mock_sink_for_session_id_;
+
+  // To avoid global/static non-POD constants.
+  const url::Origin kSecurityOrigin;
+  const url::Origin kSecurityOrigin2;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(AudioRendererMixerManagerTest);
@@ -194,13 +244,13 @@ TEST_F(AudioRendererMixerManagerTest, CreateInput) {
   // Create two mixer inputs and ensure this doesn't instantiate any mixers yet.
   EXPECT_EQ(mixer_count(), 0);
   media::FakeAudioRenderCallback callback(0);
-  scoped_refptr<media::AudioRendererMixerInput> input(
-      manager_->CreateInput(kRenderFrameId, kDefaultDeviceId, kSecurityOrigin));
+  scoped_refptr<media::AudioRendererMixerInput> input(manager_->CreateInput(
+      kRenderFrameId, 0, kDefaultDeviceId, kSecurityOrigin));
   input->Initialize(params, &callback);
   EXPECT_EQ(mixer_count(), 0);
   media::FakeAudioRenderCallback another_callback(1);
   scoped_refptr<media::AudioRendererMixerInput> another_input(
-      manager_->CreateInput(kAnotherRenderFrameId, kDefaultDeviceId,
+      manager_->CreateInput(kAnotherRenderFrameId, 0, kDefaultDeviceId,
                             kSecurityOrigin));
   another_input->Initialize(params, &another_callback);
   EXPECT_EQ(mixer_count(), 0);
@@ -214,10 +264,89 @@ TEST_F(AudioRendererMixerManagerTest, CreateInput) {
 
   // Destroying the inputs should destroy the mixers.
   input->Stop();
-  input = NULL;
+  input = nullptr;
   EXPECT_EQ(mixer_count(), 1);
   another_input->Stop();
-  another_input = NULL;
+  another_input = nullptr;
+  EXPECT_EQ(mixer_count(), 0);
+}
+
+// Verify CreateInput() provided with session id creates AudioRendererMixerInput
+// with the appropriate callbacks and they are working as expected.
+TEST_F(AudioRendererMixerManagerTest, CreateInputWithSessionId) {
+  // Expect AudioRendererMixerManager to call Start and Stop on our mock twice
+  // each: for kDefaultDeviceId and for kAnotherDeviceId. Note: Under normal
+  // conditions, each mixer would get its own sink!
+  EXPECT_CALL(*mock_sink_.get(), Start()).Times(2);
+  EXPECT_CALL(*mock_sink_.get(), Stop()).Times(2);
+
+  // Expect AudioRendererMixerManager to call Start and Stop on the matched sink
+  // once.
+  EXPECT_CALL(*mock_sink_matched_device_.get(), Start()).Times(1);

[Henrik Grunell, 2016/03/08 21:09:57]

Nit/Optional: I don't think you need .Times(1).

[o1ka, 2016/04/05 15:13:38]

Acknowledged.

+  EXPECT_CALL(*mock_sink_matched_device_.get(), Stop()).Times(1);
+
+  media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR,
+                                kChannelLayout, kSampleRate, kBitsPerChannel,
+                                kBufferSize);
+  media::FakeAudioRenderCallback callback(0);
+  EXPECT_EQ(mixer_count(), 0);

[Henrik Grunell, 2016/03/08 21:09:57]

The syntax is actually EXPECT_EQ(expected, actual), i.e. flip the arguments.
Here and elsewhere. I see it's incorrect in the whole file, no need to change
already existing code.

https://code.google.com/p/chromium/codesearch#chromium/src/testing/gtest/incl...

[o1ka, 2016/04/05 15:13:38]

Done.

+
+  // Empty device id, zero session id;
+  scoped_refptr<media::AudioRendererMixerInput> input_to_default_device(
+      manager_->CreateInput(kRenderFrameId, 0,  // session_id
+                            "", kSecurityOrigin));

[Henrik Grunell, 2016/03/08 21:09:57]

Nit: Does it take an std::string? Change "" -> std::string(). Elsewhere too.

[o1ka, 2016/04/05 15:13:38]

Done.

+  input_to_default_device->Initialize(params, &callback);
+  EXPECT_EQ(mixer_count(), 0);
+
+  // Specific device id, zero session id;
+  scoped_refptr<media::AudioRendererMixerInput> input_to_matched_device(
+      manager_->CreateInput(kRenderFrameId, 0,  // session_id
+                            kMatchedDeviceId, kSecurityOrigin));
+  input_to_matched_device->Initialize(params, &callback);
+  EXPECT_EQ(mixer_count(), 0);
+
+  // Specific device id, non-zero session id (to be ignored);
+  scoped_refptr<media::AudioRendererMixerInput> input_to_another_device(
+      manager_->CreateInput(kRenderFrameId, 1,  // session id
+                            kAnotherDeviceId, kSecurityOrigin));
+  input_to_another_device->Initialize(params, &callback);
+  EXPECT_EQ(mixer_count(), 0);
+
+  // Empty device id, non-zero session id;
+  scoped_refptr<media::AudioRendererMixerInput>
+      input_to_matched_device_with_session_id(
+          manager_->CreateInput(kRenderFrameId, 2,  // session id
+                                "", kSecurityOrigin));
+  input_to_matched_device_with_session_id->Initialize(params, &callback);
+  EXPECT_EQ(mixer_count(), 0);
+
+  // Implicitly test that AudioRendererMixerInput was provided with the expected
+  // callbacks needed to acquire an AudioRendererMixer and remove it.
+  input_to_default_device->Start();
+  EXPECT_EQ(mixer_count(), 1);
+
+  input_to_another_device->Start();
+  EXPECT_EQ(mixer_count(), 2);
+
+  input_to_matched_device->Start();
+  EXPECT_EQ(mixer_count(), 3);
+
+  // Should go to the same device as the input above.
+  input_to_matched_device_with_session_id->Start();
+  EXPECT_EQ(mixer_count(), 3);
+
+  // Destroying the inputs should destroy the mixers.
+  input_to_default_device->Stop();
+  input_to_default_device = nullptr;
+  EXPECT_EQ(mixer_count(), 2);
+  input_to_another_device->Stop();
+  input_to_another_device = nullptr;
+  EXPECT_EQ(mixer_count(), 1);
+  input_to_matched_device->Stop();
+  input_to_matched_device = nullptr;
+  EXPECT_EQ(mixer_count(), 1);
+  input_to_matched_device_with_session_id->Stop();
+  input_to_matched_device_with_session_id = nullptr;
   EXPECT_EQ(mixer_count(), 0);
 }
 
@@ -236,16 +365,14 @@ TEST_F(AudioRendererMixerManagerTest, MixerDevices) {
   ASSERT_TRUE(mixer1);
   EXPECT_EQ(mixer_count(), 1);
 
-  std::string device_id2("fake-device-id");
-  media::AudioRendererMixer* mixer2 =
-      GetMixer(kRenderFrameId, params, device_id2, kSecurityOrigin, nullptr);
+  media::AudioRendererMixer* mixer2 = GetMixer(
+      kRenderFrameId, params, kAnotherDeviceId, kSecurityOrigin, nullptr);
   ASSERT_TRUE(mixer2);
   EXPECT_EQ(mixer_count(), 2);
   EXPECT_NE(mixer1, mixer2);
 
-  url::Origin security_origin2(GURL("http://localhost"));
   media::AudioRendererMixer* mixer3 = GetMixer(
-      kRenderFrameId, params, kDefaultDeviceId, security_origin2, nullptr);
+      kRenderFrameId, params, kAnotherDeviceId, kSecurityOrigin2, nullptr);
   ASSERT_TRUE(mixer3);
   EXPECT_EQ(mixer_count(), 3);
   EXPECT_NE(mixer1, mixer3);
@@ -253,9 +380,52 @@ TEST_F(AudioRendererMixerManagerTest, MixerDevices) {
 
   RemoveMixer(kRenderFrameId, params, kDefaultDeviceId, kSecurityOrigin);
   EXPECT_EQ(mixer_count(), 2);
-  RemoveMixer(kRenderFrameId, params, device_id2, kSecurityOrigin);
+  RemoveMixer(kRenderFrameId, params, kAnotherDeviceId, kSecurityOrigin);
+  EXPECT_EQ(mixer_count(), 1);
+  RemoveMixer(kRenderFrameId, params, kAnotherDeviceId, kSecurityOrigin2);
+  EXPECT_EQ(mixer_count(), 0);
+}
+
+// Verify GetMixer() correctly deduplicate mixers with the same
+// parameters, different security origins but default device ID
+TEST_F(AudioRendererMixerManagerTest, OneMixerDiffferenrOriginsDefaultDevice) {
+  EXPECT_CALL(*mock_sink_.get(), Start()).Times(1);
+  EXPECT_CALL(*mock_sink_.get(), Stop()).Times(1);
+  EXPECT_EQ(mixer_count(), 0);
+
+  media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR,
+                                kChannelLayout, kSampleRate, kBitsPerChannel,
+                                kBufferSize);
+  media::AudioRendererMixer* mixer1 = GetMixer(
+      kRenderFrameId, params, kDefaultDeviceId, kSecurityOrigin, nullptr);
+  ASSERT_TRUE(mixer1);
+  EXPECT_EQ(mixer_count(), 1);
+
+  media::AudioRendererMixer* mixer2 =
+      GetMixer(kRenderFrameId, params, "", kSecurityOrigin, nullptr);
+  ASSERT_TRUE(mixer2);
+  EXPECT_EQ(mixer_count(), 1);
+  EXPECT_EQ(mixer1, mixer2);
+
+  media::AudioRendererMixer* mixer3 = GetMixer(
+      kRenderFrameId, params, kDefaultDeviceId, kSecurityOrigin2, nullptr);
+  ASSERT_TRUE(mixer3);
+  EXPECT_EQ(mixer_count(), 1);
+  EXPECT_EQ(mixer1, mixer3);
+
+  media::AudioRendererMixer* mixer4 =
+      GetMixer(kRenderFrameId, params, "", kSecurityOrigin2, nullptr);
+  ASSERT_TRUE(mixer4);
+  EXPECT_EQ(mixer_count(), 1);
+  EXPECT_EQ(mixer1, mixer4);
+
+  RemoveMixer(kRenderFrameId, params, kDefaultDeviceId, kSecurityOrigin);
+  EXPECT_EQ(mixer_count(), 1);
+  RemoveMixer(kRenderFrameId, params, "", kSecurityOrigin);
+  EXPECT_EQ(mixer_count(), 1);
+  RemoveMixer(kRenderFrameId, params, kDefaultDeviceId, kSecurityOrigin2);
   EXPECT_EQ(mixer_count(), 1);
-  RemoveMixer(kRenderFrameId, params, kDefaultDeviceId, security_origin2);
+  RemoveMixer(kRenderFrameId, params, "", kSecurityOrigin2);
   EXPECT_EQ(mixer_count(), 0);
 }
 
@@ -263,15 +433,13 @@ TEST_F(AudioRendererMixerManagerTest, MixerDevices) {
 // status code when a nonexistent device is requested.
 TEST_F(AudioRendererMixerManagerTest, NonexistentDevice) {
   EXPECT_EQ(mixer_count(), 0);
-  UseNonexistentSink();
   media::AudioParameters params(AudioParameters::AUDIO_PCM_LINEAR,
                                 kChannelLayout, kSampleRate, kBitsPerChannel,
                                 kBufferSize);
-  std::string nonexistent_device_id("nonexistent-device-id");
   media::OutputDeviceStatus device_status = media::OUTPUT_DEVICE_STATUS_OK;
-  EXPECT_CALL(*mock_sink_.get(), Stop());
+  EXPECT_CALL(*mock_sink_no_device_.get(), Stop());
   media::AudioRendererMixer* mixer =
-      GetMixer(kRenderFrameId, params, nonexistent_device_id, kSecurityOrigin,
+      GetMixer(kRenderFrameId, params, kNonexistentDeviceId, kSecurityOrigin,
                &device_status);
   EXPECT_FALSE(mixer);
   EXPECT_EQ(device_status, media::OUTPUT_DEVICE_STATUS_ERROR_NOT_FOUND);