Allow AudioOutputDevice objects to be initialized with a specific hardware output device and store …

media/audio/audio_output_device.cc

Index: media/audio/audio_output_device.cc
diff --git a/media/audio/audio_output_device.cc b/media/audio/audio_output_device.cc
index f65e2aa19be980a4f5de845b1b937cd9e482fbe3..d77a3329d35d49c24f987162ccaa15729064055c 100644
--- a/media/audio/audio_output_device.cc
+++ b/media/audio/audio_output_device.cc
@@ -4,8 +4,6 @@
 
 #include "media/audio/audio_output_device.h"
 
-#include <string>
-
 #include "base/callback_helpers.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
@@ -42,37 +40,51 @@ class AudioOutputDevice::AudioThreadCallback
 AudioOutputDevice::AudioOutputDevice(
     scoped_ptr<AudioOutputIPC> ipc,
     const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner)
+    : AudioOutputDevice(ipc.Pass(),
+                        io_task_runner,
+                        0,
+                        std::string(),
+                        GURL::EmptyGURL()) {}
+
+AudioOutputDevice::AudioOutputDevice(
+    scoped_ptr<AudioOutputIPC> ipc,
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
+    int session_id,
+    const std::string& device_id,
+    const GURL& security_origin)
     : ScopedTaskRunnerObserver(io_task_runner),
       callback_(NULL),
       ipc_(ipc.Pass()),
       state_(IDLE),
+      start_on_authorized_(false),
       play_on_start_(true),
-      session_id_(-1),
+      session_id_(session_id),
+      device_id_(device_id),
+      security_origin_(security_origin),
       stopping_hack_(false),
-      current_switch_request_id_(0) {
+      switch_output_device_on_start_(false),
+      did_set_output_params_(true, false) {
   CHECK(ipc_);
 
   // The correctness of the code depends on the relative values assigned in the
   // State enum.
   static_assert(IPC_CLOSED < IDLE, "invalid enum value assignment 0");
-  static_assert(IDLE < CREATING_STREAM, "invalid enum value assignment 1");
-  static_assert(CREATING_STREAM < PAUSED, "invalid enum value assignment 2");
-  static_assert(PAUSED < PLAYING, "invalid enum value assignment 3");
+  static_assert(IDLE < NOT_AUTHORIZED, "invalid enum value assignment 1");
+  static_assert(NOT_AUTHORIZED < AUTHORIZING,
+                "invalid enum value assignment 2");
+  static_assert(AUTHORIZING < AUTHORIZED, "invalid enum value assignment 3");
+  static_assert(AUTHORIZED < CREATING_STREAM,
+                "invalid enum value assignment 4");
+  static_assert(CREATING_STREAM < PAUSED, "invalid enum value assignment 5");
+  static_assert(PAUSED < PLAYING, "invalid enum value assignment 6");
 }
 
-void AudioOutputDevice::InitializeWithSessionId(const AudioParameters& params,
-                                                RenderCallback* callback,
-                                                int session_id) {
-  DCHECK(!callback_) << "Calling InitializeWithSessionId() twice?";
+void AudioOutputDevice::Initialize(const AudioParameters& params,
+                                   RenderCallback* callback) {
+  DCHECK(!callback_) << "Calling Initialize() twice?";
   DCHECK(params.IsValid());
   audio_parameters_ = params;
   callback_ = callback;
-  session_id_ = session_id;
-}
-
-void AudioOutputDevice::Initialize(const AudioParameters& params,
-                                   RenderCallback* callback) {
-  InitializeWithSessionId(params, callback, 0);
 }
 
 AudioOutputDevice::~AudioOutputDevice() {
@@ -84,11 +96,18 @@ AudioOutputDevice::~AudioOutputDevice() {
   // its bound parameters in the correct thread instead of implicitly releasing
   // them in the thread where this destructor runs.
   if (!current_switch_callback_.is_null()) {
-    base::ResetAndReturn(&current_switch_callback_).Run(
-        SWITCH_OUTPUT_DEVICE_RESULT_ERROR_OBSOLETE);
+    base::ResetAndReturn(&current_switch_callback_)
+        .Run(SWITCH_OUTPUT_DEVICE_RESULT_ERROR_INTERNAL);
   }
 }
 
+void AudioOutputDevice::RequestDeviceAuthorization() {
+  task_runner()->PostTask(
+      FROM_HERE,
+      base::Bind(&AudioOutputDevice::RequestDeviceAuthorizationOnIOThread,
+                 this));
+}
+
 void AudioOutputDevice::Start() {
   DCHECK(callback_) << "Initialize hasn't been called";
   task_runner()->PostTask(FROM_HERE,
@@ -137,17 +156,55 @@ void AudioOutputDevice::SwitchOutputDevice(
     const std::string& device_id,
     const GURL& security_origin,
     const SwitchOutputDeviceCB& callback) {
-  DVLOG(1) << __FUNCTION__ << "(" << device_id << ")";
   task_runner()->PostTask(
       FROM_HERE, base::Bind(&AudioOutputDevice::SwitchOutputDeviceOnIOThread,
                             this, device_id, security_origin, callback));
 }
 
+AudioParameters AudioOutputDevice::GetOutputParameters() {
+  CHECK(!task_runner()->BelongsToCurrentThread());
+  did_set_output_params_.Wait();
+  return output_params_;
+}
+
+void AudioOutputDevice::RequestDeviceAuthorizationOnIOThread() {
+  DCHECK(task_runner()->BelongsToCurrentThread());
+  DCHECK_EQ(state_, IDLE);
+  state_ = AUTHORIZING;
+  ipc_->RequestDeviceAuthorization(this, session_id_, device_id_,
+                                   security_origin_);
+}
+
 void AudioOutputDevice::CreateStreamOnIOThread(const AudioParameters& params) {
   DCHECK(task_runner()->BelongsToCurrentThread());
-  if (state_ == IDLE) {
-    state_ = CREATING_STREAM;
-    ipc_->CreateStream(this, params, session_id_);
+  switch (state_) {
+    case IDLE:
+      if (did_set_output_params_.IsSignaled() && device_id_.empty() &&
+          security_origin_.is_empty()) {
+        state_ = CREATING_STREAM;
+        ipc_->CreateStream(this, params);
+      } else {
+        RequestDeviceAuthorizationOnIOThread();
+        start_on_authorized_ = true;
+      }
+      break;
+
+    case NOT_AUTHORIZED:
+      callback_->OnRenderError();
+      break;
+
+    case AUTHORIZING:
+      start_on_authorized_ = true;
+      break;
+
+    case AUTHORIZED:
+      state_ = CREATING_STREAM;
+      ipc_->CreateStream(this, params);
+      start_on_authorized_ = false;
+      break;
+
+    default:

[DaleCurtis, 2015/09/12 01:17:19]

I think we generally try to avoid using default in state machines. Can you add
the other states with a NOTREACHED() ?

[Guido Urdaneta, 2015/09/14 11:35:48]

Done.

+      break;
   }
 }
 
@@ -179,10 +236,13 @@ void AudioOutputDevice::ShutDownOnIOThread() {
   DCHECK(task_runner()->BelongsToCurrentThread());
 
   // Close the stream, if we haven't already.
-  if (state_ >= CREATING_STREAM) {
+  if (state_ >= AUTHORIZING)
     ipc_->CloseStream();
+
+  if (state_ >= NOT_AUTHORIZED)
     state_ = IDLE;
-  }
+
+  start_on_authorized_ = false;
 
   // We can run into an issue where ShutDownOnIOThread is called right after
   // OnStreamCreated is called in cases where Start/Stop are called before we
@@ -210,13 +270,21 @@ void AudioOutputDevice::SwitchOutputDeviceOnIOThread(
     const GURL& security_origin,
     const SwitchOutputDeviceCB& callback) {
   DCHECK(task_runner()->BelongsToCurrentThread());
-  DVLOG(1) << __FUNCTION__ << "(" << device_id << "," << security_origin << ")";
+
+  // Do not allow concurrent SwitchOutputDevice requests
+  if (!current_switch_callback_.is_null()) {
+    callback.Run(SWITCH_OUTPUT_DEVICE_RESULT_ERROR_INTERNAL);
+    return;
+  }
+
+  current_switch_callback_ = callback;
+  current_switch_device_id_ = device_id;
+  current_switch_security_origin_ = security_origin;
   if (state_ >= CREATING_STREAM) {
-    SetCurrentSwitchRequest(callback);
-    ipc_->SwitchOutputDevice(device_id, security_origin,
-                             current_switch_request_id_);
+    ipc_->SwitchOutputDevice(current_switch_device_id_,
+                             current_switch_security_origin_);
   } else {
-    callback.Run(SWITCH_OUTPUT_DEVICE_RESULT_ERROR_NOT_SUPPORTED);
+    switch_output_device_on_start_ = true;
   }
 }
 
@@ -250,6 +318,32 @@ void AudioOutputDevice::OnStateChanged(AudioOutputIPCDelegateState state) {
   }
 }
 
+void AudioOutputDevice::OnDeviceAuthorized(
+    bool success,
+    const media::AudioParameters& output_params) {
+  DCHECK(task_runner()->BelongsToCurrentThread());
+  DCHECK(state_ == IDLE || state_ == AUTHORIZING);

[DaleCurtis, 2015/09/12 01:17:19]

Log <<state_ so you can debug this easily.

[Guido Urdaneta, 2015/09/14 11:35:48]

Actually, it is impossible to get this while in IDLE since the delegate is
removed when AOD changes to IDLE state. I guess this makes the log unnecessary.

+
+  if (success && !did_set_output_params_.IsSignaled()) {
+    did_set_output_params_.Signal();

[DaleCurtis, 2015/09/12 01:17:19]

You always need to signal or a thread might get locked. You should just use an
invalid set of params though; also don't signal until after you set the params
or you'll have a race.

[Guido Urdaneta, 2015/09/14 11:35:48]

Done. Using the default initial invalid params in case of authorization failure.
Also signaling in the destructor just in case.

+    output_params_ = output_params;
+  }
+
+  // Stream was stopped
+  if (state_ == IDLE)
+    return;
+
+  if (!success) {
+    state_ = NOT_AUTHORIZED;
+    if (callback_)
+      callback_->OnRenderError();
+  }
+
+  state_ = AUTHORIZED;
+  if (start_on_authorized_)
+    CreateStreamOnIOThread(audio_parameters_);
+}
+
 void AudioOutputDevice::OnStreamCreated(
     base::SharedMemoryHandle handle,
     base::SyncSocket::Handle socket_handle,
@@ -278,46 +372,40 @@ void AudioOutputDevice::OnStreamCreated(
   // delete as they see fit. AudioOutputDevice should internally use WeakPtr
   // to handle teardown and thread hopping. See http://crbug.com/151051 for
   // details.
-  base::AutoLock auto_lock(audio_thread_lock_);
-  if (stopping_hack_)
-    return;
+  {
+    base::AutoLock auto_lock(audio_thread_lock_);
+    if (stopping_hack_)
+      return;
+
+    DCHECK(audio_thread_.IsStopped());
+    audio_callback_.reset(new AudioOutputDevice::AudioThreadCallback(
+        audio_parameters_, handle, length, callback_));
+    audio_thread_.Start(audio_callback_.get(), socket_handle,
+                        "AudioOutputDevice", true);
+    state_ = PAUSED;
 
-  DCHECK(audio_thread_.IsStopped());
-  audio_callback_.reset(new AudioOutputDevice::AudioThreadCallback(
-      audio_parameters_, handle, length, callback_));
-  audio_thread_.Start(
-      audio_callback_.get(), socket_handle, "AudioOutputDevice", true);
-  state_ = PAUSED;
-
-  // We handle the case where Play() and/or Pause() may have been called
-  // multiple times before OnStreamCreated() gets called.
-  if (play_on_start_)
-    PlayOnIOThread();
-}
+    // We handle the case where Play() and/or Pause() may have been called
+    // multiple times before OnStreamCreated() gets called.
+    if (play_on_start_)
+      PlayOnIOThread();
+  }
 
-void AudioOutputDevice::SetCurrentSwitchRequest(
-    const SwitchOutputDeviceCB& callback) {
-  DCHECK(task_runner()->BelongsToCurrentThread());
-  DVLOG(1) << __FUNCTION__;
-  // If there is a previous unresolved request, resolve it as obsolete
-  if (!current_switch_callback_.is_null()) {
-    base::ResetAndReturn(&current_switch_callback_).Run(
-        SWITCH_OUTPUT_DEVICE_RESULT_ERROR_OBSOLETE);
+  if (switch_output_device_on_start_) {
+    ipc_->SwitchOutputDevice(current_switch_device_id_,
+                             current_switch_security_origin_);
   }
-  current_switch_callback_ = callback;
-  current_switch_request_id_++;
 }
 
 void AudioOutputDevice::OnOutputDeviceSwitched(
-    int request_id,
-    SwitchOutputDeviceResult result) {
+    SwitchOutputDeviceResult result,
+    const media::AudioParameters& output_params) {
   DCHECK(task_runner()->BelongsToCurrentThread());
-  DCHECK(request_id <= current_switch_request_id_);
-  DVLOG(1) << __FUNCTION__
-           << "(" << request_id << ", " << result << ")";
-  if (request_id != current_switch_request_id_) {
-    return;
+  if (result == SWITCH_OUTPUT_DEVICE_RESULT_SUCCESS) {
+    session_id_ = 0;  // Output device is no longer attached to an input device
+    device_id_ = current_switch_device_id_;
+    security_origin_ = current_switch_security_origin_;
   }
+  DCHECK(!current_switch_callback_.is_null());
   base::ResetAndReturn(&current_switch_callback_).Run(result);
 }