Pass camera facing info to audio client

content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc

Index: content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
diff --git a/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc b/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
index 3a626163e5938976a9efdeb076d290bee4612463..5d6de5db778bb4651f09bdb77dd21a5acd47cbb5 100644
--- a/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
+++ b/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "component/arc/video_facing.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/media/capture/audio_mirroring_manager.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
@@ -247,6 +248,12 @@ class MockControllerFactory : public AudioInputController::Factory {
   std::vector<scoped_refptr<MockController>> controller_list_;
 };
 
+class MockVideoCaptureObserver : public component::VideoCaptureObserver {
+ public:
+  MOCK_METHOD1(OnVideoCaptureStarted, void(component::VideoFacingMode));
+  MOCK_METHOD1(OnVideoCaptureStopped, void(component::VideoFacingMode));
+};

[chfremer, 2017/01/20 18:23:52]

I count a total of 9 places where we have unit tests that need this exact same
mock.
It seems awfully redundant to define it 9 times. Would it be possible to move it
to a separate file that can be shared between all the tests that need it?

[shenghao, 2017/01/24 12:52:01]

Done.

+
 }  // namespace
 
 class AudioInputRendererHostTest : public testing::Test {
@@ -260,7 +267,8 @@ class AudioInputRendererHostTest : public testing::Test {
         base::ThreadTaskRunnerHandle::Get(),
         base::ThreadTaskRunnerHandle::Get(), &log_factory_));
     media_stream_manager_ =
-        base::MakeUnique<MediaStreamManager>(audio_manager_.get());
+        base::MakeUnique<MediaStreamManager>(audio_manager_.get(),
+                                             new MockVideoCaptureObserver());

[chfremer, 2017/01/20 18:23:53]

Since neither MediaStreamManager nor VideoCaptureManager take ownership of this,
this looks like a memory leak (here and in the 8 other tests that do the same).

We seem to have a situation where sometimes we want VideoCaptureManager to take
ownership (e.g. in all the tests), and sometimes we don't, i.e. when passing in
a pointer to a global object.

My recommendation here is to make |capture_observer_| in VideoCaptureManager a
std::unique_ptr<>. Then, to pass in CrasAudioHandler::Get(), we should wrap it
using a class that forwards to a non-owned raw pointer, i.e.:

class NonOwnedVideoCaptureObserver : public VideoCaptureObserver {
 public:
  NonOwnedVideoCaptureObserver(VideoCaptureObserver* observer)
      : observer_(observer) {}

  void OnVideoCaptureStarted(component::VideoFacingMode facing_mode) {
    observer_->OnVideoCaptureStarted(facing_mode);
  }

  void OnVideoCaptureStopped(component::VideoFacingMode facing_mode) {
    observer_->OnVideoCaptureStopped(facing_mode);
  }

 private:
  VideoCaptureObserver* const observer_;
};

[shenghao, 2017/01/24 12:52:00]

Removed all the changes in tests.

     airh_ = new AudioInputRendererHostWithInterception(
         kRenderProcessId, kRendererPid, media::AudioManager::Get(),
         media_stream_manager_.get(), AudioMirroringManager::GetInstance(),