Implement setSinkId() for media elements without src.

third_party/WebKit/Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.cpp

Index: third_party/WebKit/Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.cpp
diff --git a/third_party/WebKit/Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.cpp b/third_party/WebKit/Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.cpp
index 930dc3c40b88e366c893b33e4f2691cb812f706b..55207135dddeaced0982f8c0faa3530f9fe7d589 100644
--- a/third_party/WebKit/Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.cpp
+++ b/third_party/WebKit/Source/modules/audio_output_devices/HTMLMediaElementAudioOutputDevice.cpp
@@ -8,11 +8,77 @@
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "core/dom/ExecutionContext.h"
+#include "modules/audio_output_devices/AudioOutputDeviceClient.h"
 #include "modules/audio_output_devices/SetSinkIdCallbacks.h"
 #include "public/platform/WebSecurityOrigin.h"
 
 namespace blink {
 
+class SetSinkIdResolver : public ScriptPromiseResolver {

[Peter Beverloo, 2015/11/12 16:45:42]

note: you've split out the smaller SetSinkIdCallbacks to its own file, but
define this in-file.

You may want to consider moving it to its own file, but I'm OK with keeping it
here too, whatever you prefer. If you keep it in this file, please do put it in
an anonymous namespace.

[Guido Urdaneta, 2015/11/12 18:45:44]

Done.

+    WTF_MAKE_NONCOPYABLE(SetSinkIdResolver);
+public:
+    static SetSinkIdResolver* create(ScriptState*, HTMLMediaElement&, const String& sinkId);
+    ~SetSinkIdResolver() = default;

[Peter Beverloo, 2015/11/12 16:45:42]

+override

[Guido Urdaneta, 2015/11/12 18:45:44]

Done.

+
+    DECLARE_VIRTUAL_TRACE();
+
+private:
+    SetSinkIdResolver(ScriptState*, HTMLMediaElement&, const String& sinkId);
+    void timerFired(Timer<SetSinkIdResolver>*);
+
+    RefPtrWillBeMember<HTMLMediaElement> m_element;
+    String m_sinkId;
+    Timer<SetSinkIdResolver> m_timer;
+};
+
+SetSinkIdResolver* SetSinkIdResolver::create(ScriptState* scriptState, HTMLMediaElement& element, const String& sinkId)
+{
+    SetSinkIdResolver* resolver = new SetSinkIdResolver(scriptState, element, sinkId);
+    resolver->suspendIfNeeded();
+    resolver->keepAliveWhilePending();
+    return resolver;
+}
+
+SetSinkIdResolver::SetSinkIdResolver(ScriptState* scriptState, HTMLMediaElement& element, const String& sinkId)
+    : ScriptPromiseResolver(scriptState)
+    , m_element(element)
+    , m_sinkId(sinkId)
+    , m_timer(this, &SetSinkIdResolver::timerFired)
+{
+    m_timer.startOneShot(0, BLINK_FROM_HERE);
+}
+
+void SetSinkIdResolver::timerFired(Timer<SetSinkIdResolver>* timer)
+{
+    if (m_sinkId == HTMLMediaElementAudioOutputDevice::sinkId(*m_element)) {

[Peter Beverloo, 2015/11/12 16:45:42]

nit: since we can do this check synchronously, we should do it in
HTMLMediaElementAudioOutputDevice::setSinkId().

This (1) matches the spec, (2) remove the overhead of starting a timer and (3)
remove the need to use the static accessor.

[Guido Urdaneta, 2015/11/12 18:45:44]

Done.

+        resolve();
+        return;
+    }
+
+    ExecutionContext* context = executionContext();
+    ASSERT(context && context->isDocument());
+    OwnPtr<SetSinkIdCallbacks> callbacks = adoptPtr(new SetSinkIdCallbacks(this, *m_element, m_sinkId));
+    WebMediaPlayer* webMediaPlayer = m_element->webMediaPlayer();
+    if (webMediaPlayer) {
+        // Using leakPtr() to transfer ownership because |webMediaPlayer| is a platform object that takes raw pointers
+        webMediaPlayer->setSinkId(m_sinkId, WebSecurityOrigin(context->securityOrigin()), callbacks.leakPtr());
+    } else {
+        if (AudioOutputDeviceClient* client = AudioOutputDeviceClient::from(context)) {
+            client->checkIfAudioSinkExistsAndIsAuthorized(context, m_sinkId, callbacks.release());

[Peter Beverloo, 2015/11/12 16:45:42]

Note that there will still be a gap between updating the value (which happens
during the timer execution) and resolving the promise (which happens using a
microtask at the end of the event loop), but since neither are in the
developer's control this should be OK.

Alex (scheduling team) tells me that it's effectively indeterminable.

[Guido Urdaneta, 2015/11/12 18:45:44]

Acknowledged.

+        } else {
+            // The context has been detached. Impossible to get a security origin to check.
+            ASSERT(context->activeDOMObjectsAreStopped());
+            reject(DOMException::create(SecurityError, "Impossible to authorize device for detached context"));
+        }
+    }
+}
+
+DEFINE_TRACE(SetSinkIdResolver)
+{
+    visitor->trace(m_element);
+    ScriptPromiseResolver::trace(visitor);
+}
+
 HTMLMediaElementAudioOutputDevice::HTMLMediaElementAudioOutputDevice()
     : m_sinkId("")
 {
@@ -31,17 +97,7 @@ void HTMLMediaElementAudioOutputDevice::setSinkId(const String& sinkId)
 
 ScriptPromise HTMLMediaElementAudioOutputDevice::setSinkId(ScriptState* scriptState, HTMLMediaElement& element, const String& sinkId)
 {
-    ASSERT(scriptState);
-
-    WebMediaPlayer* webMediaPlayer = element.webMediaPlayer();
-    if (!webMediaPlayer)
-        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(AbortError, "No media player available"));
-
-    ExecutionContext* context = scriptState->executionContext();
-    ASSERT(context && context->isDocument());
-
-    ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
-    webMediaPlayer->setSinkId(sinkId, WebSecurityOrigin(context->securityOrigin()), new SetSinkIdCallbacks(resolver, element, sinkId));
+    SetSinkIdResolver* resolver = SetSinkIdResolver::create(scriptState, element, sinkId);
     return resolver->promise();
 }