Reset readystates when webmediaplayer is cleared

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 3026 matching lines @@
 
 void HTMLMediaElement::userCancelledLoad()
 {
     WTF_LOG(Media, "HTMLMediaElement::userCancelledLoad(%p)", this);
 
     // If the media data fetching process is aborted by the user:
 
     // 1 - The user agent should cancel the fetching process.
     clearMediaPlayer(-1);
 
-    if (m_networkState == NETWORK_EMPTY || m_completelyLoaded || m_isFinalizing)
+    if (m_networkState == NETWORK_EMPTY || m_completelyLoaded || m_isFinalizing) {
+        m_readyState = HAVE_NOTHING;

[philipj_slow, 2015/10/20 11:24:53]

Shouldn't the readyState be reset unconditionally? I guess this happens here
merely so that it happens before the function returns, but moving up the "Reset
m_readyState since m_webMediaPlayer is gone" bit to right after the
clearMediaPlayer(-1) call would be more clear I think.

[Srirama, 2015/10/20 11:31:34]

I thought about that, but it will affect step4 in this function. We are
resetting networkstate based on readystate value. So is it fine to move it up?

[philipj_slow, 2015/10/20 11:52:00]

Oh, that's annoying. I think the problem here is that the spec text is written
in a way that makes sense if you merely stop the network activity but still
allow the data already fetched to be played, which implies keeping the player
alive. However, the first thing we do is to clear the player...

Have you dug into the purpose of this early return? Is it really a good idea to
skip over the `m_currentSourceNode = nullptr` step, for example?

+        m_readyStateMaximum = HAVE_NOTHING;
         return;
+    }
 
     // 2 - Set the error attribute to a new MediaError object whose code attribute is set to MEDIA_ERR_ABORTED.
     m_error = MediaError::create(MediaError::MEDIA_ERR_ABORTED);
 
     // 3 - Queue a task to fire a simple event named error at the media element.
     scheduleEvent(EventTypeNames::abort);
 
     // 4 - If the media element's readyState attribute has a value equal to HAVE_NOTHING, set the
     // element's networkState attribute to the NETWORK_EMPTY value and queue a task to fire a
     // simple event named emptied at the element. Otherwise, set the element's networkState
     // attribute to the NETWORK_IDLE value.
     if (m_readyState == HAVE_NOTHING) {
         m_networkState = NETWORK_EMPTY;
         scheduleEvent(EventTypeNames::emptied);
     } else {
         m_networkState = NETWORK_IDLE;
     }
 
     // 5 - Set the element's delaying-the-load-event flag to false. This stops delaying the load event.
     setShouldDelayLoadEvent(false);
 
     // 6 - Abort the overall resource selection algorithm.
     m_currentSourceNode = nullptr;
 
     // Reset m_readyState since m_webMediaPlayer is gone.
     m_readyState = HAVE_NOTHING;
+    m_readyStateMaximum = HAVE_NOTHING;
     invalidateCachedTime();
     updateMediaController();
     cueTimeline().updateActiveCues(0);
 }
 
 void HTMLMediaElement::clearMediaPlayerAndAudioSourceProviderClientWithoutLocking()
 {
 #if ENABLE(WEB_AUDIO)
     audioSourceProvider().setClient(nullptr);
 #endif
@@ skipping 729 matching lines @@
     visitor->trace(m_client);
 }
 
 DEFINE_TRACE(HTMLMediaElement::AudioSourceProviderImpl)
 {
     visitor->trace(m_client);
 }
 #endif
 
 }