Caching AudioOutputDevice instances in mixer manager

media/base/audio_renderer_mixer_input.h

 // Copyright (c) 2012 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.
 //
 // THREAD SAFETY
 //
 // This class is generally not thread safe. Callers should ensure thread safety.
 // For instance, the |sink_lock_| in WebAudioSourceProvider synchronizes access
 // to this object across the main thread (for WebAudio APIs) and the
 // media thread (for HTMLMediaElement APIs).
@@ skipping 25 matching lines @@
   typedef base::Callback<AudioRendererMixer*(const AudioParameters& params,
                                              const std::string& device_id,
                                              const url::Origin& security_origin,
                                              OutputDeviceStatus* device_status)>
       GetMixerCB;
   typedef base::Callback<void(const AudioParameters& params,
                               const std::string& device_id,
                               const url::Origin& security_origin)>
       RemoveMixerCB;
 
+  typedef base::Callback<OutputDeviceInfo(const std::string& device_id,
+                                          const url::Origin& security_origin)>
+      GetDeviceInfoCB;
+
   AudioRendererMixerInput(const GetMixerCB& get_mixer_cb,

[miu, 2016/05/12 21:53:06]

Hmm...This is now the third callback being injected, which is starting to abuse
the main purpose of Callbacks (to invoke unknown external implementation after
an asynchronous operation completes). Consider replacing these with an interface
like:

  class MixerPool {
    virtual AudioRendererMixer* AcquireMixer(...) = 0;
    virtual void ReleaseMixer(...) = 0;
    virtual OutputDeviceInfo GetDeviceInfo(...) = 0;
  };

...and the constructor becomes:

  AudioRendererMixerInput(MixerPool* pool,
                          const std::string& device_id,
                          const url::Origin& security_origin);

The reason having all these callbacks is messy is because it becomes harder to
reason about object lifetimes and ownership/encapsulation of functionality.
You'd have to look for where each Callback object is being created and how
base::Bind() was used, and whether it calls via a raw, ref-counted, or weak
pointer. Also, there are subtle "surprises" in how copies of base::Callback
objects can invalidate the original (e.g., when bound with move types like
std::unique_ptr<>).

A teammate of mine recently did a big clean-up like this (reducing an
11-argument constructor down to 4 arguments!), and we actually resolved a few
bugs once the object graph became clearer:
https://codereview.chromium.org/1878883003/ and
https://codereview.chromium.org/1515023002/

[o1ka, 2016/05/17 17:17:24]

Done. (I actually was going to do that at some point, but then forgot about it
:) )

                           const RemoveMixerCB& remove_mixer_cb,
+                          const GetDeviceInfoCB& get_device_info_cb,
                           const std::string& device_id,
                           const url::Origin& security_origin);
 
   // SwitchableAudioRendererSink implementation.
   void Start() override;
   void Stop() override;
   void Play() override;
   void Pause() override;
   bool SetVolume(double volume) override;
   OutputDeviceInfo GetOutputDeviceInfo() override;
@@ skipping 22 matching lines @@
 
   // AudioConverter::InputCallback implementation.
   double ProvideInput(AudioBus* audio_bus,
                       base::TimeDelta buffer_delay) override;
 
   // Callbacks provided during construction which allow AudioRendererMixerInput
   // to retrieve a mixer during Initialize() and notify when it's done with it.
   const GetMixerCB get_mixer_cb_;
   const RemoveMixerCB remove_mixer_cb_;
 
+  // Callback to get output device information in the absence of |mixer_|.
+  const GetDeviceInfoCB get_device_info_cb_;
+
   // AudioParameters received during Initialize().
   AudioParameters params_;
 
   // ID of hardware device to use
   std::string device_id_;
   url::Origin security_origin_;
 
   // AudioRendererMixer provided through |get_mixer_cb_| during Initialize(),
   // guaranteed to live (at least) until |remove_mixer_cb_| is called.
   AudioRendererMixer* mixer_;
 
   // Source of audio data which is provided to the mixer.
   AudioRendererSink::RenderCallback* callback_;
 
   // Error callback for handing to AudioRendererMixer.
   const base::Closure error_cb_;
 
   // Pending switch-device callback, in case SwitchOutputDevice() is invoked
   // before Start()
   OutputDeviceStatusCB pending_switch_callback_;
   std::string pending_switch_device_id_;
   url::Origin pending_switch_security_origin_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioRendererMixerInput);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_AUDIO_RENDERER_MIXER_INPUT_H_