Change play promises behaviour in the load algorithm.

third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 747 matching lines @@
     m_displayMode = Unknown;
 
     // 1 - Abort any already-running instance of the resource selection algorithm for this element.
     m_loadState = WaitingForSource;
     m_currentSourceNode = nullptr;
 
     // 2 - If there are any tasks from the media element's media element event task source in
     // one of the task queues, then remove those tasks.
     cancelPendingEventsAndCallbacks();
 
-    rejectPlayPromises(AbortError, "The play() request was interrupted by a new load request.");
-
     // 3 - If the media element's networkState is set to NETWORK_LOADING or NETWORK_IDLE, queue
     // a task to fire a simple event named abort at the media element.
     if (m_networkState == NETWORK_LOADING || m_networkState == NETWORK_IDLE)
         scheduleEvent(EventTypeNames::abort);
 
     resetMediaPlayerAndMediaSource();
 
     // 4 - If the media element's networkState is not set to NETWORK_EMPTY, then run these substeps
     if (m_networkState != NETWORK_EMPTY) {
         // 4.1 - Queue a task to fire a simple event named emptied at the media element.
         scheduleEvent(EventTypeNames::emptied);
 
-        // 4.2 - If a fetching process is in progress for the media element, the user agent should stop it.
+        // 4.2 - Take pending play promises and reject pending play promises with the result and an "AbortError" DOMException.
+        rejectPlayPromises(AbortError, "The play() request was interrupted by a new load request.");
+
+        // 4.3 - If a fetching process is in progress for the media element, the user agent should stop it.
         setNetworkState(NETWORK_EMPTY);
 
-        // 4.3 - Forget the media element's media-resource-specific tracks.
+        // 4.5 - Forget the media element's media-resource-specific tracks.
         forgetResourceSpecificTracks();
 
-        // 4.4 - If readyState is not set to HAVE_NOTHING, then set it to that state.
+        // 4.6 - If readyState is not set to HAVE_NOTHING, then set it to that state.
         m_readyState = HAVE_NOTHING;
         m_readyStateMaximum = HAVE_NOTHING;
 
-        // 4.5 - If the paused attribute is false, then set it to true.
+        // 4.6 - If the paused attribute is false, then set it to true.
         m_paused = true;
 
-        // 4.6 - If seeking is true, set it to false.
+        // 4.7 - If seeking is true, set it to false.
         m_seeking = false;
 
-        // 4.7 - Set the current playback position to 0.
+        // 4.8 - Set the current playback position to 0.
         //       Set the official playback position to 0.
         //       If this changed the official playback position, then queue a task to fire a simple event named timeupdate at the media element.
         // FIXME: Add support for firing this event.
 
-        // 4.8 - Set the initial playback position to 0.
+        // 4.9 - Set the initial playback position to 0.
         // FIXME: Make this less subtle. The position only becomes 0 because the ready state is HAVE_NOTHING.
         invalidateCachedTime();
 
-        // 4.9 - Set the timeline offset to Not-a-Number (NaN).
-        // 4.10 - Update the duration attribute to Not-a-Number (NaN).
+        // 4.10 - Set the timeline offset to Not-a-Number (NaN).
+        // 4.11 - Update the duration attribute to Not-a-Number (NaN).
 
         cueTimeline().updateActiveCues(0);
     } else if (!m_paused) {
         // TODO(foolip): There is a proposal to always reset the paused state
         // in the media element load algorithm, to avoid a bogus play() promise
         // rejection: https://github.com/whatwg/html/issues/869
         // This is where that change would have an effect, and it is measured to
         // verify the assumption that it's a very rare situation.
         UseCounter::count(document(), UseCounter::HTMLMediaElementLoadNetworkEmptyNotPaused);

[foolip, 2016/08/03 14:32:56]

Can you remove this counter (also from UseCounter.h) since we've concluded that
option isn't going to work out?

[mlamouri (slow - plz ping), 2016/08/03 15:57:55]

Sounds good but I would prefer to do that in a follow-up because it would
require metrics changes (histograms.xml) and it's not quite related to this
change.

[foolip, 2016/08/03 16:31:51]

Acknowledged.

     }
 
     // 5 - Set the playbackRate attribute to the value of the defaultPlaybackRate attribute.
     setPlaybackRate(defaultPlaybackRate());
 
     // 6 - Set the error attribute to null and the autoplaying flag to true.
     m_error = nullptr;
     m_autoplaying = true;
 
     // 7 - Invoke the media element's resource selection algorithm.
@@ skipping 566 matching lines @@
 }
 
 void HTMLMediaElement::cancelPendingEventsAndCallbacks()
 {
     BLINK_MEDIA_LOG << "cancelPendingEventsAndCallbacks(" << (void*)this << ")";
     m_asyncEventQueue->cancelAllEvents();
 
     for (HTMLSourceElement* source = Traversal<HTMLSourceElement>::firstChild(*this); source; source = Traversal<HTMLSourceElement>::nextSibling(*source))
         source->cancelPendingErrorEvent();
 
-    m_playPromiseResolveTask->cancel();
-    m_playPromiseResolveList.clear();
-    m_playPromiseRejectTask->cancel();
-    m_playPromiseRejectList.clear();
+    if (m_playPromiseResolveTask->isPending()) {

[foolip, 2016/08/03 14:32:56]

Can you update the documentation of HTMLMediaElement::invokeLoadAlgorithm to
match the spec change and if it then looks more natural, move this out to where
the documentation is? At least the resolveScheduledPlayPromises() and
rejectScheduledPlayPromises() calls I guess would end up outside this method.

Then there's the question of order that came up in the spec review. We already
have the task order problem documented in the TODO in
HTMLMediaElement::scheduleResolvePlayPromises, so I think it's OK if we can't
exactly match the spec, but we should write the spec in a way that we think will
at least be possible later to implement.

How would things have to change for "in the order that the corresponding tasks
were queued" to be implementable, or do you think the spec should say something
different?

[mlamouri (slow - plz ping), 2016/08/03 15:57:56]

Done.

+        m_playPromiseResolveTask->cancel();
+        resolveScheduledPlayPromises();
+    }
+
+    if (m_playPromiseRejectTask->isPending()) {
+        m_playPromiseRejectTask->cancel();
+        rejectScheduledPlayPromises();
+    }
 }
 
 void HTMLMediaElement::networkStateChanged()
 {
     setNetworkState(webMediaPlayer()->getNetworkState());
 }
 
 void HTMLMediaElement::mediaLoadingFailed(WebMediaPlayer::NetworkState error)
 {
     stopPeriodicTimers();
@@ skipping 2602 matching lines @@
 
 IntRect HTMLMediaElement::AutoplayHelperClientImpl::absoluteBoundingBoxRect() const
 {
     IntRect result;
     if (LayoutObject* object = m_element->layoutObject())
         result = object->absoluteBoundingBoxRect();
     return result;
 }
 
 } // namespace blink