Switching VirtualKeyboardPrivate keyboard config call stack to receive HasInputDevices() responce a…

media/audio/audio_system_impl.cc

Index: media/audio/audio_system_impl.cc
diff --git a/media/audio/audio_system_impl.cc b/media/audio/audio_system_impl.cc
index 96300ba950df9e3c3cef1f2acd9ac369fd0b1be6..fdffa70fda0f70cbce1e60457eb2044e9287a254 100644
--- a/media/audio/audio_system_impl.cc
+++ b/media/audio/audio_system_impl.cc
@@ -33,9 +33,12 @@ AudioParameters GetInputParametersOnDeviceThread(AudioManager* audio_manager,
 AudioSystemImpl::AudioSystemImpl(AudioManager* audio_manager)
     : audio_manager_(audio_manager) {
   DCHECK(audio_manager_);
+  AudioSystem::SetInstance(this);
 }
 
-AudioSystemImpl::~AudioSystemImpl() {}
+AudioSystemImpl::~AudioSystemImpl() {
+  AudioSystem::ClearInstance(this);
+}
 
 // static
 std::unique_ptr<AudioSystem> AudioSystemImpl::Create(
@@ -59,6 +62,20 @@ void AudioSystemImpl::GetInputStreamParameters(
       std::move(on_params_cb));
 }
 
+void AudioSystemImpl::HasInputDevices(OnBoolCallback on_has_devices_cb) const {
+  if (GetTaskRunner()->BelongsToCurrentThread()) {

[tommi (sloooow) - chröme, 2017/02/09 16:39:34]

I guess this is necessary so that the on_has_devices_cb is called on the audio
thread?
If that's not a requirement and it's fine for the callback to be made on the
calling thread to HasInputDevices, then we can do this with one less PostTask -
right?

If we want to keep this as is, I think it would be good to document (and same
for GetInputStreamParameters I guess)

[o1ka, 2017/02/09 16:56:10]

It's never called on the audio thread unless it is called on audio thread. (I.e.
it is always called on the caller thread)
Calling the callback synchronously is something I avoid intentionally:
1) It will result in different behavior on Mac vs other platforms, which means 2
times more debugging;
2) It also means that the callback must be able to run in the context of the
method which calls HasInputDevices(), i.e., for example (though it's an example
of a bad design): if HasInputDevices() is called under a lock, then the callback
must not take the same lock.
3) It may also result in nested calls to AudioSystem (if a callback also makes
AudioSystem call).

So, "Callback may be called synchronously or posted later" approach seems to
complicate things and reduce debugability.

WDYT?
What exactly needs to be documented? Now it says that "// Replies are
asynchronously sent to the thread the call is issued on" (see AudioSystem
interface)

[tommi (sloooow) - chröme, 2017/02/10 10:51:07]

Thanks  - now looking at this again, I see I misread the code.  I read it
incorrectly as |if (!BelongsToCurrentThread())|.  Now I see you're actually
saving a PostTask and not adding.

+    GetTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(on_has_devices_cb, audio_manager_->HasAudioInputDevices()));
+    return;
+  }
+  base::PostTaskAndReplyWithResult(
+      GetTaskRunner(), FROM_HERE,
+      base::Bind(&AudioManager::HasAudioInputDevices,
+                 base::Unretained(audio_manager_)),
+      std::move(on_has_devices_cb));
+}
+
 AudioManager* AudioSystemImpl::GetAudioManager() const {
   return audio_manager_;
 }