Added initial set of logging for AudioOutputController.

content/browser/renderer_host/media/audio_output_delegate_impl.cc

 // Copyright 2016 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.
 
 #include "content/browser/renderer_host/media/audio_output_delegate_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "content/browser/media/audio_stream_monitor.h"
 #include "content/browser/media/capture/audio_mirroring_manager.h"
 #include "content/browser/media/media_internals.h"
 #include "content/browser/renderer_host/media/audio_sync_reader.h"
+#include "content/browser/renderer_host/media/media_stream_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/media_observer.h"
 #include "media/audio/audio_output_controller.h"
 
 namespace content {
 
 // This class trampolines callbacks from the controller to the delegate. Since
 // callbacks from the controller are stopped asynchronously, this class holds
 // a weak pointer to the delegate, allowing the delegate to stop callbacks
 // synchronously.
 class AudioOutputDelegateImpl::ControllerEventHandler
     : public media::AudioOutputController::EventHandler {
  public:
   explicit ControllerEventHandler(
       base::WeakPtr<AudioOutputDelegateImpl> delegate);
 
  private:
   void OnControllerCreated() override;
   void OnControllerPlaying() override;
   void OnControllerPaused() override;
   void OnControllerError() override;
+  void OnLog(const std::string& message) override;
 
   base::WeakPtr<AudioOutputDelegateImpl> delegate_;
+  int stream_id_;  // Retained separately for logging.
 };
 
 AudioOutputDelegateImpl::ControllerEventHandler::ControllerEventHandler(
     base::WeakPtr<AudioOutputDelegateImpl> delegate)
-    : delegate_(std::move(delegate)) {}
+    : delegate_(std::move(delegate)), stream_id_(delegate_->stream_id_) {}

[Max Morin, 2017/05/31 11:52:40]

Since you're dereferencing |delegate_|, maybe add
DCHECK_CURRENTLY_ON(BrowserThread::IO) here? (The weak pointer may only be used
on one thread).

[ossu-chromium, 2017/05/31 13:47:20]

I've no reasonable place to put it - in the code block would be too late (the
dereference would already have happened). Also want stream_id_ to be const,
which it wasn't yet here. I've instead made stream_id a separate parameter, so
the caller has to ensure it can safely access it (which it already does).

[Max Morin, 2017/05/31 13:51:59]

Alright, that's fine by me.

 
 void AudioOutputDelegateImpl::ControllerEventHandler::OnControllerCreated() {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&AudioOutputDelegateImpl::SendCreatedNotification,
                      delegate_));
 }
 
 void AudioOutputDelegateImpl::ControllerEventHandler::OnControllerPlaying() {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&AudioOutputDelegateImpl::UpdatePlayingState, delegate_,
                      true));
 }
 
 void AudioOutputDelegateImpl::ControllerEventHandler::OnControllerPaused() {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&AudioOutputDelegateImpl::UpdatePlayingState, delegate_,
                      false));
 }
 
 void AudioOutputDelegateImpl::ControllerEventHandler::OnControllerError() {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&AudioOutputDelegateImpl::OnError, delegate_));
 }
 
+void AudioOutputDelegateImpl::ControllerEventHandler::OnLog(
+    const std::string& message) {
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::BindOnce(
+          [](int stream_id, const std::string& message) {
+            DCHECK_CURRENTLY_ON(BrowserThread::IO);
+            std::ostringstream oss;
+            oss << "[stream_id=" << stream_id << "] " << message;
+            const std::string out_message = oss.str();
+            content::MediaStreamManager::SendMessageToNativeLog(out_message);

[Max Morin, 2017/05/31 11:52:40]

Since I'm nit-picking anyways, remove unnecessary content::. DCHECK_CURRENTLY_ON
feels unnecessary when you use a lambda, since it's clear that this is posted to
the IO thread here and cannot be referenced from anywhere else.

[Max Morin, 2017/05/31 11:56:23]

Oh, I just noticed SendMessageToNativeLog is safe to call from any thread so
posting a task is not needed :)
(https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/medi...)

[ossu-chromium, 2017/05/31 11:59:45]

Ah, yes, you are right! The task was only posted to the IO thread so that we'd
be able to access delegate_, but since we aren't anymore, this should be fine to
just fold into OnLog directly. Unless there's some reason to keep the DVLOG call
on the IO thread? I bet there isn't.

I'll simplify further and post a new PS.

+            DVLOG(1) << out_message;
+          },
+          stream_id_, message));
+}
+
 std::unique_ptr<media::AudioOutputDelegate> AudioOutputDelegateImpl::Create(
     EventHandler* handler,
     media::AudioManager* audio_manager,
     std::unique_ptr<media::AudioLog> audio_log,
     AudioMirroringManager* mirroring_manager,
     MediaObserver* media_observer,
     int stream_id,
     int render_frame_id,
     int render_process_id,
     const media::AudioParameters& params,
@@ skipping 139 matching lines @@
   audio_log_->OnError(stream_id_);
   subscriber_->OnStreamError(stream_id_);
 }
 
 media::AudioOutputController* AudioOutputDelegateImpl::GetControllerForTesting()
     const {
   return controller_.get();
 }
 
 }  // namespace content