Bug 1397708 - remove HTMLMediaElement::mBegun. See comment 12 for the root cause. draft
authorJW Wang <jwwang@mozilla.com>
Tue, 10 Oct 2017 07:04:18 +0800
changeset 678375 5b20a678b58aa5faa9be3edc8e879f4cf6620e88
parent 678373 9aca979c3f4afdc306157c9b25ac6083c87dbd30
child 735313 faef175d6032cb283ed6452f8aa73b3b450fb46e
push id83907
push userjwwang@mozilla.com
push dateWed, 11 Oct 2017 08:54:16 +0000
bugs1397708
milestone58.0a1
Bug 1397708 - remove HTMLMediaElement::mBegun. See comment 12 for the root cause. When network state is changed to IDLE, mBegun is also set to false. [1] And then when HTMLMediaElement::DownloadResumed(false) is called, network state is not changed to LOADING for mBegun is false [2]. This prevents us from firing 'progress' events for the network state is IDLE. See comment 12 for more details. [1] http://searchfox.org/mozilla-central/rev/b53e29293c9e9a2905f4849f4e3c415e2013f0cb/dom/html/HTMLMediaElement.cpp#6077 [2] http://searchfox.org/mozilla-central/rev/b53e29293c9e9a2905f4849f4e3c415e2013f0cb/dom/html/HTMLMediaElement.cpp#5673 MozReview-Commit-ID: DOfqKZXAqaz
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
dom/media/ChannelMediaResource.cpp
dom/media/MediaDecoderOwner.h
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -3976,17 +3976,16 @@ HTMLMediaElement::HTMLMediaElement(alrea
     mLastCurrentTime(0.0),
     mFragmentStart(-1.0),
     mFragmentEnd(-1.0),
     mDefaultPlaybackRate(1.0),
     mPlaybackRate(1.0),
     mPreservesPitch(true),
     mPlayed(new TimeRanges(ToSupports(OwnerDoc()))),
     mCurrentPlayRangeStart(-1.0),
-    mBegun(false),
     mLoadedDataFired(false),
     mAutoplaying(true),
     mAutoplayEnabled(true),
     mPaused(true, *this),
     mStatsShowing(false),
     mAllowCasting(false),
     mIsCasting(false),
     mAudioCaptured(false),
@@ -5658,26 +5657,23 @@ HTMLMediaElement::NotifySuspendedByCache
   mDownloadSuspendedByCache = aSuspendedByCache;
 }
 
 void HTMLMediaElement::DownloadSuspended()
 {
   if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING) {
     DispatchAsyncEvent(NS_LITERAL_STRING("progress"));
   }
-  if (mBegun) {
-    ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_IDLE);
-  }
-}
-
-void HTMLMediaElement::DownloadResumed(bool aForceNetworkLoading)
-{
-  if (mBegun || aForceNetworkLoading) {
-    ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_LOADING);
-  }
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_IDLE);
+}
+
+void
+HTMLMediaElement::DownloadResumed()
+{
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_LOADING);
 }
 
 void HTMLMediaElement::CheckProgress(bool aHaveNewProgress)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING);
 
   TimeStamp now = TimeStamp::NowLoRes();
@@ -6064,29 +6060,22 @@ void HTMLMediaElement::ChangeNetworkStat
   if (mNetworkState == aState) {
     return;
   }
 
   nsMediaNetworkState oldState = mNetworkState;
   mNetworkState = aState;
   LOG(LogLevel::Debug, ("%p Network state changed to %s", this, gNetworkStateToString[aState]));
 
-  // TODO: |mBegun| reflects the download status. We should be able to remove
-  // it and check |mNetworkState| only.
-
   if (oldState == nsIDOMHTMLMediaElement::NETWORK_LOADING) {
-    // Reset |mBegun| since we're not downloading anymore.
-    mBegun = false;
     // Stop progress notification when exiting NETWORK_LOADING.
     StopProgress();
   }
 
   if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING) {
-    // Download is begun.
-    mBegun = true;
     // Start progress notification when entering NETWORK_LOADING.
     StartProgress();
   } else if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_IDLE &&
              !mErrorSink->mError) {
     // Fire 'suspend' event when entering NETWORK_IDLE and no error presented.
     DispatchAsyncEvent(NS_LITERAL_STRING("suspend"));
   }
 
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -206,21 +206,17 @@ public:
   // Called by the media stream, on the main thread, when the download
   // has been suspended by the cache or because the element itself
   // asked the decoder to suspend the download.
   virtual void DownloadSuspended() final override;
 
   // Called by the media stream, on the main thread, when the download
   // has been resumed by the cache or because the element itself
   // asked the decoder to resumed the download.
-  // If aForceNetworkLoading is True, ignore the fact that the download has
-  // previously finished. We are downloading the middle of the media after
-  // having downloaded the end, we need to notify the element a download in
-  // ongoing.
-  virtual void DownloadResumed(bool aForceNetworkLoading = false) final override;
+  void DownloadResumed();
 
   // Called to indicate the download is progressing.
   virtual void DownloadProgressed() final override;
 
   // Called by the media decoder to indicate whether the media cache has
   // suspended the channel.
   virtual void NotifySuspendedByCache(bool aSuspendedByCache) final override;
 
@@ -1535,20 +1531,16 @@ protected:
   nsCOMPtr<nsITimer> mVideoDecodeSuspendTimer;
 
   // Encrypted Media Extension media keys.
   RefPtr<MediaKeys> mMediaKeys;
 
   // Stores the time at the start of the current 'played' range.
   double mCurrentPlayRangeStart;
 
-  // If true then we have begun downloading the media content.
-  // Set to false when completed, or not yet started.
-  bool mBegun;
-
   // True if loadeddata has been fired.
   bool mLoadedDataFired;
 
   // Indicates whether current playback is a result of user action
   // (ie. calling of the Play method), or automatic playback due to
   // the 'autoplay' attribute being set. A true value indicates the
   // latter case.
   // The 'autoplay' HTML attribute indicates that the video should
--- a/dom/media/ChannelMediaResource.cpp
+++ b/dom/media/ChannelMediaResource.cpp
@@ -516,17 +516,17 @@ ChannelMediaResource::OpenChannel(int64_
 
   rv = mChannel->AsyncOpen2(mListener);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Tell the media element that we are fetching data from a channel.
   MediaDecoderOwner* owner = mCallback->GetMediaOwner();
   NS_ENSURE_TRUE(owner, NS_ERROR_FAILURE);
   dom::HTMLMediaElement* element = owner->GetMediaElement();
-  element->DownloadResumed(true);
+  element->DownloadResumed();
 
   return NS_OK;
 }
 
 nsresult
 ChannelMediaResource::SetupChannelHeaders(int64_t aOffset)
 {
   // Always use a byte range request even if we're reading from the start
--- a/dom/media/MediaDecoderOwner.h
+++ b/dom/media/MediaDecoderOwner.h
@@ -91,25 +91,16 @@ public:
   // when the resource has completed seeking.
   virtual void SeekCompleted() = 0;
 
   // Called by the media stream, on the main thread, when the download
   // has been suspended by the cache or because the element itself
   // asked the decoder to suspend the download.
   virtual void DownloadSuspended() = 0;
 
-  // Called by the media stream, on the main thread, when the download
-  // has been resumed by the cache or because the element itself
-  // asked the decoder to resumed the download.
-  // If aForceNetworkLoading is True, ignore the fact that the download has
-  // previously finished. We are downloading the middle of the media after
-  // having downloaded the end, we need to notify the element a download in
-  // ongoing.
-  virtual void DownloadResumed(bool aForceNetworkLoading = false) = 0;
-
   // Called by the media decoder to indicate whether the media cache has
   // suspended the channel.
   virtual void NotifySuspendedByCache(bool aSuspendedByCache) = 0;
 
   // called to notify that the principal of the decoder's media resource has changed.
   virtual void NotifyDecoderPrincipalChanged() = 0;
 
   // The status of the next frame which might be available from the decoder