Introduce debug recording Mojo implementation.

* add mojo interfaces for enabling debug recording and creating debug recording files.
* add debug recording session factory to toggle between media/audio/ and services/audio/ debug recording implementations.

Design doc: http://doc/123YGEhyyDqbO1440rwdg0T_4UPxNyZmJboczjFIZpMI#heading=h.xpzwbhip9is6

This CL depends on [1]. Switching to mojo implementation will be done in [2].

[1] https://crrev.com/c/875927
[2] https://crrev.com/c/921281

Bug: 788657
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Idbbfdb89c6ad4ff373ac25db83b5ed4ef6167b86
Reviewed-on: https://chromium-review.googlesource.com/810809
Reviewed-by: Tommi <tommi@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Marina Ciocea <marinaciocea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539134}

chrome/browser/BUILD.gn

@@ -4152,6 +4152,7 @@ jumbo_split_static_library("browser") {
     deps += [
       "//components/webrtc_logging/browser",
       "//components/webrtc_logging/common",
+      "//services/audio/public/cpp",
       "//third_party/webrtc_overrides",
       "//third_party/webrtc_overrides:init_webrtc",
     ]

chrome/browser/media/webrtc/DEPS

@@ -1,6 +1,7 @@
 include_rules = [
   # TODO(mash): Remove. http://crbug.com/723880
   "+ash/shell.h",
+  "+services/audio/public/cpp",
   "+third_party/libyuv",
   "+third_party/webrtc",
 ]

chrome/browser/media/webrtc/audio_debug_recordings_handler.cc

@@ -17,7 +17,10 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/common/service_manager_connection.h"
 #include "media/audio/audio_debug_recording_session.h"
+#include "services/audio/public/cpp/debug_recording_session_factory.h"
+#include "services/service_manager/public/cpp/connector.h"
 
 using content::BrowserThread;
 
@@ -106,8 +109,10 @@ void AudioDebugRecordingsHandler::DoStartAudioDebugRecordings(
       log_directory, ++current_audio_debug_recordings_id_);
   host->EnableAudioDebugRecordings(prefix_path);
 
-  audio_debug_recording_session_ =
-      media::AudioDebugRecordingSession::Create(prefix_path);
+  audio_debug_recording_session_ = audio::CreateAudioDebugRecordingSession(
+      prefix_path, content::ServiceManagerConnection::GetForProcess()
+                       ->GetConnector()
+                       ->Clone());
 
   if (delay.is_zero()) {
     const bool is_stopped = false, is_manual_stop = false;

content/browser/webrtc/webrtc_internals.cc

@@ -23,6 +23,7 @@
 #include "media/audio/audio_debug_recording_session.h"
 #include "media/audio/audio_manager.h"
 #include "media/media_features.h"
+#include "services/audio/public/cpp/debug_recording_session_factory.h"
 #include "services/device/public/mojom/constants.mojom.h"
 #include "services/device/public/mojom/wake_lock_provider.mojom.h"
 #include "services/service_manager/public/cpp/connector.h"
@@ -518,8 +519,11 @@ void WebRTCInternals::OnRendererExit(int render_process_id) {
 void WebRTCInternals::EnableAudioDebugRecordingsOnAllRenderProcessHosts() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(!audio_debug_recording_session_);
-  audio_debug_recording_session_ = media::AudioDebugRecordingSession::Create(
-      audio_debug_recordings_file_path_);
+  audio_debug_recording_session_ = audio::CreateAudioDebugRecordingSession(
+      audio_debug_recordings_file_path_,
+      content::ServiceManagerConnection::GetForProcess()
+          ->GetConnector()
+          ->Clone());
 
   for (RenderProcessHost::iterator i(
            content::RenderProcessHost::AllHostsIterator());

content/public/app/mojo/content_browser_manifest.json

@@ -71,7 +71,7 @@
       },
       "requires": {
         "*": [ "app" ],
-        "audio": [ "info" ],
+        "audio": [ "info", "debug_recording"],
         "cdm": [ "media:cdm" ],
         "content_gpu": [ "browser" ],
         "content_plugin": [ "browser" ],

media/audio/BUILD.gn

@@ -73,7 +73,6 @@ source_set("audio") {
     "audio_debug_recording_helper.h",
     "audio_debug_recording_manager.cc",
     "audio_debug_recording_manager.h",
-    "audio_debug_recording_session.cc",
     "audio_debug_recording_session.h",
     "audio_debug_recording_session_impl.cc",
     "audio_debug_recording_session_impl.h",
@@ -338,6 +337,8 @@ static_library("test_support") {
   visibility = [ "//media:test_support" ]
   testonly = true
   sources = [
+    "audio_debug_recording_test.cc",
+    "audio_debug_recording_test.h",
     "audio_device_info_accessor_for_tests.cc",
     "audio_device_info_accessor_for_tests.h",
     "audio_system_test_util.cc",
@@ -361,6 +362,7 @@ static_library("test_support") {
   ]
   deps = [
     "//base",
+    "//base/test:test_support",
 
     # Do not add any other //media deps except this; it will automatically pull
     # a dep on //media which is required to ensure test_support targets all use

media/audio/audio_debug_file_writer.cc

@@ -304,8 +304,4 @@ bool AudioDebugFileWriter::WillWrite() {
   return !!file_writer_;
 }
 
-const base::FilePath::CharType* AudioDebugFileWriter::GetFileExtension() {
-  return FILE_PATH_LITERAL("wav");
-}
-
 }  // namespace media

media/audio/audio_debug_file_writer.h

@@ -52,11 +52,6 @@ class MEDIA_EXPORT AudioDebugFileWriter {
   // called from any sequence.
   virtual bool WillWrite();
 
-  // Gets the extension for the file type the as a string, for example "wav".
-  // Can be called before calling Start() to add the appropriate extension to
-  // the filename.
-  virtual const base::FilePath::CharType* GetFileExtension();
-
  protected:
   const AudioParameters params_;
 

media/audio/audio_debug_file_writer_unittest.cc

@@ -242,12 +242,6 @@ TEST_P(AudioDebugFileWriterTest, WaveRecordingTest) {
   RecordAndVerifyOnce();
 }
 
-TEST_P(AudioDebugFileWriterTest, GetFileExtension) {
-  debug_writer_.reset(new AudioDebugFileWriter(params_));
-  EXPECT_EQ(FILE_PATH_LITERAL("wav"),
-            base::FilePath::StringType(debug_writer_->GetFileExtension()));
-}
-
 TEST_P(AudioDebugFileWriterSingleThreadTest,
        DeletedBeforeRecordingFinishedOnFileThread) {
   debug_writer_.reset(new AudioDebugFileWriter(params_));

media/audio/audio_debug_recording_helper.cc

@@ -32,13 +32,13 @@ AudioDebugRecordingHelper::~AudioDebugRecordingHelper() {
 
 void AudioDebugRecordingHelper::EnableDebugRecording(
     const base::FilePath& file_name_suffix,
-    CreateFileCallback create_file_callback) {
+    CreateWavFileCallback create_file_callback) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!debug_writer_);
 
   debug_writer_ = CreateAudioDebugFileWriter(params_);
   std::move(create_file_callback)
-      .Run(file_name_suffix.AddExtension(debug_writer_->GetFileExtension()),
+      .Run(file_name_suffix,
            base::BindOnce(&AudioDebugRecordingHelper::StartDebugRecordingToFile,
                           weak_factory_.GetWeakPtr()));
 }

media/audio/audio_debug_recording_helper.h

@@ -50,7 +50,7 @@ class AudioDebugRecorder {
 // soundcard thread -> file thread.
 class MEDIA_EXPORT AudioDebugRecordingHelper : public AudioDebugRecorder {
  public:
-  using CreateFileCallback = base::OnceCallback<void(
+  using CreateWavFileCallback = base::OnceCallback<void(
       const base::FilePath&,
       base::OnceCallback<void(base::File)> reply_callback)>;
 
@@ -63,7 +63,7 @@ class MEDIA_EXPORT AudioDebugRecordingHelper : public AudioDebugRecorder {
   // Enable debug recording. Creates |debug_writer_| and runs
   // |create_file_callback| to create debug recording file.
   virtual void EnableDebugRecording(const base::FilePath& file_name_suffix,
-                                    CreateFileCallback create_file_callback);
+                                    CreateWavFileCallback create_file_callback);
 
   // Disable debug recording. Destroys |debug_writer_|.
   virtual void DisableDebugRecording();

media/audio/audio_debug_recording_helper_unittest.cc

@@ -43,8 +43,7 @@ const base::FilePath::CharType kBaseFileName[] =
 const base::FilePath::CharType kFileNameSuffix[] =
     FILE_PATH_LITERAL("output.1");
 
-// The file extension the mock should return in GetFileExtension().
-const base::FilePath::CharType kFileExtension[] = FILE_PATH_LITERAL("wav");
+const base::FilePath::CharType kWavExtension[] = FILE_PATH_LITERAL("wav");
 
 }  // namespace
 
@@ -76,7 +75,6 @@ class MockAudioDebugFileWriter : public AudioDebugFileWriter {
   }
 
   MOCK_METHOD0(WillWrite, bool());
-  MOCK_METHOD0(GetFileExtension, const base::FilePath::CharType*());
 
   // Set reference data to compare against. Must be called before Write() is
   // called.
@@ -107,11 +105,10 @@ class AudioDebugRecordingHelperUnderTest : public AudioDebugRecordingHelper {
 
  private:
   // Creates the mock writer. After the mock writer is returned, we always
-  // expect GetFileExtension() and Start() to be called on it by the helper.
+  // expect Start() to be called on it by the helper.
   std::unique_ptr<AudioDebugFileWriter> CreateAudioDebugFileWriter(
       const AudioParameters& params) override {
     MockAudioDebugFileWriter* writer = new MockAudioDebugFileWriter(params);
-    EXPECT_CALL(*writer, GetFileExtension()).WillOnce(Return(kFileExtension));
     EXPECT_CALL(*writer, DoStart(true));
     return base::WrapUnique<AudioDebugFileWriter>(writer);
   }
@@ -139,20 +136,21 @@ class AudioDebugRecordingHelperTest : public ::testing::Test {
   MOCK_METHOD0(OnAudioDebugRecordingHelperDestruction, void());
 
   // Bound and passed to AudioDebugRecordingHelper::EnableDebugRecording as
-  // AudioDebugRecordingHelper::CreateFileCallback.
-  void CreateFile(const base::FilePath& file_name_suffix,
-                  base::OnceCallback<void(base::File)> reply_callback) {
+  // AudioDebugRecordingHelper::CreateWavFileCallback.
+  void CreateWavFile(const base::FilePath& file_name_suffix,
+                     base::OnceCallback<void(base::File)> reply_callback) {
     // Check that AudioDebugRecordingHelper::EnableDebugRecording adds file
     // extension to file name suffix.
-    EXPECT_EQ(file_name_suffix_.AddExtension(kFileExtension), file_name_suffix);
+    EXPECT_EQ(file_name_suffix_, file_name_suffix);
     base::ScopedTempDir temp_dir;
     ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
     base::FilePath base_file_path(
         temp_dir.GetPath().Append(base::FilePath(kBaseFileName)));
     base::FilePath file_path =
-        base_file_path.AddExtension(file_name_suffix.value());
-    base::File debug_file(base::File(
-        file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE));
+        base_file_path.AddExtension(file_name_suffix.value())
+            .AddExtension(kWavExtension);
+    base::File debug_file(
+        file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
     // Run |reply_callback| with a valid file for expected
     // MockAudioDebugFileWriter::Start mocked call to happen.
     std::move(reply_callback).Run(std::move(debug_file));
@@ -199,7 +197,7 @@ TEST_F(AudioDebugRecordingHelperTest, EnableDisable) {
 
   recording_helper->EnableDebugRecording(
       file_name_suffix_,
-      base::BindOnce(&AudioDebugRecordingHelperTest::CreateFile,
+      base::BindOnce(&AudioDebugRecordingHelperTest::CreateWavFile,
                      base::Unretained(this)));
   EXPECT_CALL(*static_cast<MockAudioDebugFileWriter*>(
                   recording_helper->debug_writer_.get()),
@@ -208,7 +206,7 @@ TEST_F(AudioDebugRecordingHelperTest, EnableDisable) {
 
   recording_helper->EnableDebugRecording(
       file_name_suffix_,
-      base::BindOnce(&AudioDebugRecordingHelperTest::CreateFile,
+      base::BindOnce(&AudioDebugRecordingHelperTest::CreateWavFile,
                      base::Unretained(this)));
   EXPECT_CALL(*static_cast<MockAudioDebugFileWriter*>(
                   recording_helper->debug_writer_.get()),
@@ -242,7 +240,7 @@ TEST_F(AudioDebugRecordingHelperTest, OnData) {
 
   recording_helper->EnableDebugRecording(
       file_name_suffix_,
-      base::BindOnce(&AudioDebugRecordingHelperTest::CreateFile,
+      base::BindOnce(&AudioDebugRecordingHelperTest::CreateWavFile,
                      base::Unretained(this)));
   MockAudioDebugFileWriter* mock_audio_file_writer =
       static_cast<MockAudioDebugFileWriter*>(
@@ -264,7 +262,7 @@ TEST_F(AudioDebugRecordingHelperTest, OnData) {
   // disabling.
   recording_helper->EnableDebugRecording(
       file_name_suffix_,
-      base::BindOnce(&AudioDebugRecordingHelperTest::CreateFile,
+      base::BindOnce(&AudioDebugRecordingHelperTest::CreateWavFile,
                      base::Unretained(this)));
   mock_audio_file_writer = static_cast<MockAudioDebugFileWriter*>(
       recording_helper->debug_writer_.get());

media/audio/audio_debug_recording_manager.cc

@@ -42,7 +42,7 @@ AudioDebugRecordingManager::AudioDebugRecordingManager(
 AudioDebugRecordingManager::~AudioDebugRecordingManager() = default;
 
 void AudioDebugRecordingManager::EnableDebugRecording(
-    CreateFileCallback create_file_callback) {
+    CreateWavFileCallback create_file_callback) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!create_file_callback.is_null());
   create_file_callback_ = std::move(create_file_callback);

media/audio/audio_debug_recording_manager.h

@@ -57,7 +57,7 @@ namespace media {
 
 class MEDIA_EXPORT AudioDebugRecordingManager {
  public:
-  using CreateFileCallback = base::RepeatingCallback<void(
+  using CreateWavFileCallback = base::RepeatingCallback<void(
       const base::FilePath&,
       base::OnceCallback<void(base::File)> reply_callback)>;
 
@@ -66,7 +66,7 @@ class MEDIA_EXPORT AudioDebugRecordingManager {
   virtual ~AudioDebugRecordingManager();
 
   // Enables and disables debug recording.
-  virtual void EnableDebugRecording(CreateFileCallback create_file_callback);
+  virtual void EnableDebugRecording(CreateWavFileCallback create_file_callback);
   virtual void DisableDebugRecording();
 
   // Registers a source and returns a wrapped recorder. |stream_type| is added
@@ -110,7 +110,7 @@ class MEDIA_EXPORT AudioDebugRecordingManager {
 
   // Callback for creating debug recording files. When this is not null, debug
   // recording is enabled.
-  CreateFileCallback create_file_callback_;
+  CreateWavFileCallback create_file_callback_;
 
   base::WeakPtrFactory<AudioDebugRecordingManager> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(AudioDebugRecordingManager);

media/audio/audio_debug_recording_manager_unittest.cc

@@ -50,9 +50,9 @@ struct ScopedExpectEnableAfterCreateHelper {
 };
 
 // Function bound and passed to AudioDebugRecordingManager::EnableDebugRecording
-// as AudioDebugRecordingManager::CreateFileCallback.
-void CreateFile(const base::FilePath& file_path,
-                base::OnceCallback<void(base::File)>) {}
+// as AudioDebugRecordingManager::CreateWavFileCallback.
+void CreateWavFile(const base::FilePath& file_path,
+                   base::OnceCallback<void(base::File)>) {}
 
 }  // namespace
 
@@ -78,7 +78,7 @@ class MockAudioDebugRecordingHelper : public AudioDebugRecordingHelper {
 
   MOCK_METHOD1(DoEnableDebugRecording, void(const base::FilePath&));
   void EnableDebugRecording(const base::FilePath& file_name_suffix,
-                            AudioDebugRecordingHelper::CreateFileCallback
+                            AudioDebugRecordingHelper::CreateWavFileCallback
                                 create_file_callback) override {
     DoEnableDebugRecording(file_name_suffix);
   }
@@ -147,10 +147,10 @@ class AudioDebugRecordingManagerTest : public ::testing::Test {
 
 int AudioDebugRecordingManagerTest::expected_next_source_id_ = 1;
 
-// Shouldn't do anything but store the CreateFileCallback, i.e. no calls to
+// Shouldn't do anything but store the CreateWavFileCallback, i.e. no calls to
 // recorders.
 TEST_F(AudioDebugRecordingManagerTest, EnableDisable) {
-  manager_.EnableDebugRecording(base::BindRepeating(&CreateFile));
+  manager_.EnableDebugRecording(base::BindRepeating(&CreateWavFile));
   manager_.DisableDebugRecording();
 }
 
@@ -197,7 +197,7 @@ TEST_F(AudioDebugRecordingManagerTest, RegisterEnableDisable) {
     EXPECT_CALL(*mock_recording_helper, DisableDebugRecording());
   }
 
-  manager_.EnableDebugRecording(base::BindRepeating(&CreateFile));
+  manager_.EnableDebugRecording(base::BindRepeating(&CreateWavFile));
   manager_.DisableDebugRecording();
 }
 
@@ -210,7 +210,7 @@ TEST_F(AudioDebugRecordingManagerTest, RegisterEnableDisable) {
 TEST_F(AudioDebugRecordingManagerTest, EnableRegisterDisable) {
   ScopedExpectEnableAfterCreateHelper scoped_enable_after_create_helper;
 
-  manager_.EnableDebugRecording(base::BindRepeating(&CreateFile));
+  manager_.EnableDebugRecording(base::BindRepeating(&CreateWavFile));
 
   const AudioParameters params;
   std::vector<std::unique_ptr<AudioDebugRecorder>> recorders;

media/audio/audio_debug_recording_session.h

@@ -5,24 +5,16 @@
 #ifndef MEDIA_AUDIO_AUDIO_DEBUG_RECORDING_SESSION_H_
 #define MEDIA_AUDIO_AUDIO_DEBUG_RECORDING_SESSION_H_
 
-#include <memory>
-
 #include "base/macros.h"
 #include "media/base/media_export.h"
 
-namespace base {
-class FilePath;
-}
-
 namespace media {
 
-// Creating/Destroying an AudioDebugRecordingSession object enables/disables
-// audio debug recording.
+// Enables/disables audio debug recording on construction/destruction. Objects
+// are created using audio::CreateAudioDebugRecordingSession.
 class MEDIA_EXPORT AudioDebugRecordingSession {
  public:
-  virtual ~AudioDebugRecordingSession();
-  static std::unique_ptr<AudioDebugRecordingSession> Create(
-      const base::FilePath& debug_recording_file_path);
+  virtual ~AudioDebugRecordingSession() = default;
 
  protected:
   AudioDebugRecordingSession() = default;