Implementing AudioOutputDevice authorization timeout

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 a3effebfb43af118500197a6eed9a48093563f7d..7d5fa103dd3863c6dbdd7cb1a0c4c8e03edb9817 100644
--- a/media/audio/audio_output_device.cc
+++ b/media/audio/audio_output_device.cc
@@ -12,8 +12,10 @@
 
 #include "base/callback_helpers.h"
 #include "base/macros.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "media/audio/audio_device_description.h"
@@ -52,7 +54,8 @@ AudioOutputDevice::AudioOutputDevice(
     const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
     int session_id,
     const std::string& device_id,
-    const url::Origin& security_origin)
+    const url::Origin& security_origin,
+    base::TimeDelta authorization_timeout)
     : ScopedTaskRunnerObserver(io_task_runner),
       callback_(NULL),
       ipc_(std::move(ipc)),
@@ -65,7 +68,8 @@ AudioOutputDevice::AudioOutputDevice(
       stopping_hack_(false),
       did_receive_auth_(base::WaitableEvent::ResetPolicy::MANUAL,
                         base::WaitableEvent::InitialState::NOT_SIGNALED),
-      device_status_(OUTPUT_DEVICE_STATUS_ERROR_INTERNAL) {
+      device_status_(OUTPUT_DEVICE_STATUS_ERROR_INTERNAL),
+      auth_timeout_(authorization_timeout) {
   CHECK(ipc_);
 
   // The correctness of the code depends on the relative values assigned in the
@@ -156,6 +160,16 @@ void AudioOutputDevice::RequestDeviceAuthorizationOnIOThread() {
   state_ = AUTHORIZING;
   ipc_->RequestDeviceAuthorization(this, session_id_, device_id_,
                                    security_origin_);
+
+  // Create the timer on the thread it's used on. It's guaranteed to be
+  // deleted on the same thread since users must call Stop() before deleting
+  // AudioOutputDevice; see ShutDownOnIOThread().
+  auth_timeout_action_.reset(new base::OneShotTimer());
+  auth_timeout_action_->Start(
+      FROM_HERE, auth_timeout_,
+      base::Bind(&AudioOutputDevice::ProcessDeviceAuthorizationOnIOThread, this,
+                 OUTPUT_DEVICE_STATUS_ERROR_INTERNAL, media::AudioParameters(),

[DaleCurtis, 2016/06/13 18:30:09]

This is fine with me, but have you considered adding an
OUTPUT_DEVICE_STATUS_ERROR_TIMED_OUT to avoid needing the ProcessXXX()
trampoline method?

[o1ka, 2016/06/14 10:33:05]

Good idea! Done.

+                 std::string(), true /* timed out */));
 }
 
 void AudioOutputDevice::CreateStreamOnIOThread(const AudioParameters& params) {
@@ -229,6 +243,9 @@ void AudioOutputDevice::ShutDownOnIOThread() {
   }
   start_on_authorized_ = false;
 
+  // Destoy the timer on the thread it's used on.
+  auth_timeout_action_.reset(nullptr);

[DaleCurtis, 2016/06/13 18:30:09]

reset(nullptr) is unnecessary. just .reset()

[o1ka, 2016/06/14 10:33:05]

Done.

+
   // We can run into an issue where ShutDownOnIOThread is called right after
   // OnStreamCreated is called in cases where Start/Stop are called before we
   // get the OnStreamCreated callback.  To handle that corner case, we call
@@ -250,41 +267,26 @@ void AudioOutputDevice::SetVolumeOnIOThread(double volume) {
     ipc_->SetVolume(volume);
 }
 
-void AudioOutputDevice::OnStateChanged(AudioOutputIPCDelegateState state) {
+void AudioOutputDevice::ProcessDeviceAuthorizationOnIOThread(
+    OutputDeviceStatus device_status,
+    const media::AudioParameters& output_params,
+    const std::string& matched_device_id,
+    bool timed_out) {
   DCHECK(task_runner()->BelongsToCurrentThread());
 
-  // Do nothing if the stream has been closed.
-  if (state_ < CREATING_STREAM)
+  if (auth_timeout_action_) {

[DaleCurtis, 2016/06/13 18:30:09]

Just auth_timeout_action_.reset() is sufficient.

[o1ka, 2016/06/14 10:33:05]

Done.

+    auth_timeout_action_->Stop();
+    auth_timeout_action_.reset(nullptr);
+  }
+
+  // Do nothing if late authorization is received after timeout.
+  if (state_ == IPC_CLOSED && !timed_out)
     return;
 
-  // TODO(miu): Clean-up inconsistent and incomplete handling here.
-  // http://crbug.com/180640
-  switch (state) {
-    case AUDIO_OUTPUT_IPC_DELEGATE_STATE_PLAYING:
-    case AUDIO_OUTPUT_IPC_DELEGATE_STATE_PAUSED:
-      break;
-    case AUDIO_OUTPUT_IPC_DELEGATE_STATE_ERROR:
-      DLOG(WARNING) << "AudioOutputDevice::OnStateChanged(ERROR)";
-      // Don't dereference the callback object if the audio thread
-      // is stopped or stopping.  That could mean that the callback
-      // object has been deleted.
-      // TODO(tommi): Add an explicit contract for clearing the callback
-      // object.  Possibly require calling Initialize again or provide
-      // a callback object via Start() and clear it in Stop().
-      if (!audio_thread_.IsStopped())
-        callback_->OnRenderError();
-      break;
-    default:
-      NOTREACHED();
-      break;
-  }
-}
+  UMA_HISTOGRAM_BOOLEAN("Media.Audio.Render.OutputDeviceAuthorizationTimedOut",
+                        timed_out);
 
-void AudioOutputDevice::OnDeviceAuthorized(
-    OutputDeviceStatus device_status,
-    const media::AudioParameters& output_params,
-    const std::string& matched_device_id) {
-  DCHECK(task_runner()->BelongsToCurrentThread());
+  DCHECK(!timed_out || device_status == OUTPUT_DEVICE_STATUS_ERROR_INTERNAL);
   DCHECK_EQ(state_, AUTHORIZING);
 
   // It may happen that a second authorization is received as a result to a
@@ -329,6 +331,44 @@ void AudioOutputDevice::OnDeviceAuthorized(
   }
 }
 
+void AudioOutputDevice::OnStateChanged(AudioOutputIPCDelegateState state) {
+  DCHECK(task_runner()->BelongsToCurrentThread());
+
+  // Do nothing if the stream has been closed.
+  if (state_ < CREATING_STREAM)
+    return;
+
+  // TODO(miu): Clean-up inconsistent and incomplete handling here.
+  // http://crbug.com/180640
+  switch (state) {
+    case AUDIO_OUTPUT_IPC_DELEGATE_STATE_PLAYING:
+    case AUDIO_OUTPUT_IPC_DELEGATE_STATE_PAUSED:
+      break;
+    case AUDIO_OUTPUT_IPC_DELEGATE_STATE_ERROR:
+      DLOG(WARNING) << "AudioOutputDevice::OnStateChanged(ERROR)";
+      // Don't dereference the callback object if the audio thread
+      // is stopped or stopping.  That could mean that the callback
+      // object has been deleted.
+      // TODO(tommi): Add an explicit contract for clearing the callback
+      // object.  Possibly require calling Initialize again or provide
+      // a callback object via Start() and clear it in Stop().
+      if (!audio_thread_.IsStopped())
+        callback_->OnRenderError();
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
+}
+
+void AudioOutputDevice::OnDeviceAuthorized(
+    OutputDeviceStatus device_status,
+    const media::AudioParameters& output_params,
+    const std::string& matched_device_id) {
+  ProcessDeviceAuthorizationOnIOThread(device_status, output_params,
+                                       matched_device_id, false);
+}
+
 void AudioOutputDevice::OnStreamCreated(
     base::SharedMemoryHandle handle,
     base::SyncSocket::Handle socket_handle,