Implementing AudioOutputDevice authorization timeout

media/audio/audio_output_device.cc

 // Copyright (c) 2012 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 "media/audio/audio_output_device.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <cmath>
 #include <utility>
 
 #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"
 #include "media/audio/audio_output_controller.h"
 #include "media/base/limits.h"
 
 namespace media {
 
 // Takes care of invoking the render callback on the audio thread.
 // An instance of this class is created for each capture stream in
@@ skipping 18 matching lines @@
   uint64_t callback_num_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioThreadCallback);
 };
 
 AudioOutputDevice::AudioOutputDevice(
     std::unique_ptr<AudioOutputIPC> ipc,
     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)),
       state_(IDLE),
       start_on_authorized_(false),
       play_on_start_(true),
       session_id_(session_id),
       device_id_(device_id),
       security_origin_(security_origin),
       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
   // State enum.
   static_assert(IPC_CLOSED < IDLE, "invalid enum value assignment 0");
   static_assert(IDLE < AUTHORIZING, "invalid enum value assignment 1");
   static_assert(AUTHORIZING < AUTHORIZED, "invalid enum value assignment 2");
   static_assert(AUTHORIZED < CREATING_STREAM,
                 "invalid enum value assignment 3");
   static_assert(CREATING_STREAM < PAUSED, "invalid enum value assignment 4");
@@ skipping 70 matching lines @@
                               : device_id_,
                           device_status_, output_params_);
 }
 
 void AudioOutputDevice::RequestDeviceAuthorizationOnIOThread() {
   DCHECK(task_runner()->BelongsToCurrentThread());
   DCHECK_EQ(state_, IDLE);
   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) {
   DCHECK(task_runner()->BelongsToCurrentThread());
   switch (state_) {
     case IPC_CLOSED:
       if (callback_)
         callback_->OnRenderError();
       break;
 
@@ skipping 53 matching lines @@
 void AudioOutputDevice::ShutDownOnIOThread() {
   DCHECK(task_runner()->BelongsToCurrentThread());
 
   // Close the stream, if we haven't already.
   if (state_ >= AUTHORIZING) {
     ipc_->CloseStream();
     state_ = IDLE;
   }
   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
   // Stop(). In most cases, the thread will already be stopped.
   //
   // Another situation is when the IO thread goes away before Stop() is called
   // in which case, we cannot use the message loop to close the thread handle
   // and can't rely on the main thread existing either.
   base::AutoLock auto_lock_(audio_thread_lock_);
   base::ThreadRestrictions::ScopedAllowIO allow_io;
   audio_thread_.Stop(NULL);
   audio_callback_.reset();
   stopping_hack_ = false;
 }
 
 void AudioOutputDevice::SetVolumeOnIOThread(double volume) {
   DCHECK(task_runner()->BelongsToCurrentThread());
   if (state_ >= CREATING_STREAM)
     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
   // call to Start() after Stop(). If the status for the second authorization
   // differs from the first, it will not be reflected in |device_status_|
   // to avoid a race.
   // This scenario is unlikely. If it occurs, the new value will be
   // different from OUTPUT_DEVICE_STATUS_OK, so the AudioOutputDevice
   // will enter the IPC_CLOSED state anyway, which is the safe thing to do.
   // This is preferable to holding a lock.
@@ skipping 24 matching lines @@
   } else {
     // Closing IPC forces a Signal(), so no clients are locked waiting
     // indefinitely after this method returns.
     ipc_->CloseStream();
     OnIPCClosed();
     if (callback_)
       callback_->OnRenderError();
   }
 }
 
+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,
     int length) {
   DCHECK(task_runner()->BelongsToCurrentThread());
   DCHECK(base::SharedMemory::IsHandleValid(handle));
 #if defined(OS_WIN)
   DCHECK(socket_handle);
 #else
   DCHECK_GE(socket_handle, 0);
@@ skipping 102 matching lines @@
 
   // Update the audio-delay measurement, inform about the number of skipped
   // frames, and ask client to render audio.  Since |output_bus_| is wrapping
   // the shared memory the Render() call is writing directly into the shared
   // memory.
   render_callback_->Render(output_bus_.get(), std::round(frames_delayed),
                            frames_skipped);
 }
 
 }  // namespace media