[RemotePlayback API] Resolve/reject prompt() properly when the element is 'connected'.

third_party/WebKit/Source/modules/remoteplayback/RemotePlayback.cpp

 // Copyright 2016 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 "modules/remoteplayback/RemotePlayback.h"
 
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/modules/v8/RemotePlaybackAvailabilityCallback.h"
 #include "core/HTMLNames.h"
 #include "core/dom/DOMException.h"
@@ skipping 161 matching lines @@
   if (!UserGestureIndicator::utilizeUserGesture()) {
     resolver->reject(DOMException::create(
         InvalidAccessError, "RemotePlayback::prompt() requires user gesture."));
     return promise;
   }
 
   if (m_state == WebRemotePlaybackState::Disconnected) {
     m_promptPromiseResolver = resolver;
     m_mediaElement->requestRemotePlayback();
   } else {
+    m_promptPromiseResolver = resolver;
     m_mediaElement->requestRemotePlaybackControl();
-    // TODO(avayvod): Need to keep the resolver until user chooses to stop
-    // the remote playback (resolve) or dismisses the UI (reject).
-    // Steps 11 and 12 of the prompt() algorithm.
-    // https://crbug.com/647441
-    resolver->resolve();
   }
 
   return promise;
 }
 
 String RemotePlayback::state() const {
   return remotePlaybackStateToString(m_state);
 }
 
 bool RemotePlayback::hasPendingActivity() const {
   return hasEventListeners() || !m_availabilityCallbacks.isEmpty() ||
          m_promptPromiseResolver;
 }
 
 void RemotePlayback::notifyInitialAvailability(int callbackId) {
   // May not find the callback if the website cancels it fast enough.
   auto iter = m_availabilityCallbacks.find(callbackId);
   if (iter == m_availabilityCallbacks.end())
     return;
 
   iter->value->call(this, m_availability);
 }
 
 void RemotePlayback::stateChanged(WebRemotePlaybackState state) {
   // We may get a "disconnected" state change while in the "disconnected"
   // state if initiated connection fails. So cleanup the promise resolvers
   // before checking if anything changed.
-  // TODO(avayvod): cleanup this logic when we implementing the "connecting"
+  // TODO(avayvod): cleanup this logic when implementing the "connecting"
   // state.
   if (m_promptPromiseResolver) {
-    if (state != WebRemotePlaybackState::Disconnected)
-      m_promptPromiseResolver->resolve();
-    else
+    if (m_state != WebRemotePlaybackState::Connected &&
+        state == WebRemotePlaybackState::Disconnected) {
       m_promptPromiseResolver->reject(DOMException::create(
           AbortError, "Failed to connect to the remote device."));
+    } else {
+      m_promptPromiseResolver->resolve();

[mlamouri (slow - plz ping), 2016/11/02 14:59:10]

So if we go from not connected to connected or from connected to disconnected,
we resolve? Sounds like a comment could help understand what's happening here :)
Ideally, the code could be made more self-explanatory :)

[whywhat, 2016/11/02 21:05:00]

Yes, if we go from not-connected to disconnected - it means prompt tried to
establish a connection and failed - we reject with error. Otherwise it's an
expected transition (connected to disconnected or anything to
connecting/connected) - so we resolve.

Expanded the comment above the if's

[whywhat, 2016/11/03 00:10:34]

It becomes a bit easier to follow with the 'connecting' state I believe:
https://codereview.chromium.org/2464163004

+    }
     m_promptPromiseResolver = nullptr;
   }
 
   if (m_state == state)
     return;
 
   m_state = state;
   switch (m_state) {
     case WebRemotePlaybackState::Connecting:
       dispatchEvent(Event::create(EventTypeNames::connecting));
@@ skipping 52 matching lines @@
 }
 
 DEFINE_TRACE_WRAPPERS(RemotePlayback) {
   for (auto callback : m_availabilityCallbacks.values()) {
     visitor->traceWrappers(callback);
   }
   EventTargetWithInlineData::traceWrappers(visitor);
 }
 
 }  // namespace blink