Bug 883731 - Progress update should be associated with network state transitions. Also centralize network state transition code. r=cpearce

content/html/content/public/HTMLMediaElement.h

@@ -204,6 +204,9 @@ public:
   // ongoing.
   virtual void DownloadResumed(bool aForceNetworkLoading = false) MOZ_FINAL MOZ_OVERRIDE;
 
+  // Called to indicate the download is progressing.
+  virtual void DownloadProgressed() MOZ_FINAL MOZ_OVERRIDE;
+
   // Called by the media decoder to indicate that the download has stalled
   // (no data has arrived for a while).
   virtual void DownloadStalled() MOZ_FINAL MOZ_OVERRIDE;
@@ -643,6 +646,12 @@ protected:
    */
   void ChangeReadyState(nsMediaReadyState aState);
 
+  /**
+   * Use this method to change the mNetworkState member, so required
+   * actions will be taken during the transition.
+   */
+  void ChangeNetworkState(nsMediaNetworkState aState);
+
   /**
    * These two methods are called by the WakeLockBoolWrapper when the wakelock
    * has to be created or released.

content/html/content/src/HTMLMediaElement.cpp

@@ -618,6 +618,11 @@ void HTMLMediaElement::ShutdownDecoder()
 {
   RemoveMediaElementFromURITable();
   NS_ASSERTION(mDecoder, "Must have decoder to shut down");
+  // TODO: This should be handled by ChangeNetworkState() so we have only one
+  // place to call StopProgress().
+  if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING) {
+    mDecoder->StopProgress();
+  }
   mDecoder->Shutdown();
   mDecoder = nullptr;
 }
@@ -661,7 +666,6 @@ void HTMLMediaElement::AbortExistingLoads()
   }
 
   mError = nullptr;
-  mBegun = false;
   mLoadedDataFired = false;
   mAutoplaying = true;
   mIsLoadingFromSourceChildren = false;
@@ -675,8 +679,8 @@ void HTMLMediaElement::AbortExistingLoads()
   mTags = nullptr;
 
   if (mNetworkState != nsIDOMHTMLMediaElement::NETWORK_EMPTY) {
-    mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
     NS_ASSERTION(!mDecoder && !mSrcStream, "How did someone setup a new stream/decoder already?");
+    ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_EMPTY);
     ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_NOTHING);
     mPaused = true;
 
@@ -702,7 +706,7 @@ void HTMLMediaElement::NoSupportedMediaSourceError()
   NS_ASSERTION(mDelayingLoadEvent, "Load event not delayed during source selection?");
 
   mError = new MediaError(this, nsIDOMMediaError::MEDIA_ERR_SRC_NOT_SUPPORTED);
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE;
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE);
   DispatchAsyncEvent(NS_LITERAL_STRING("error"));
   // This clears mDelayingLoadEvent, so AddRemoveSelfReference will be called
   ChangeDelayLoadStatus(false);
@@ -746,7 +750,7 @@ void HTMLMediaElement::RunInStableState(nsIRunnable* aRunnable)
 void HTMLMediaElement::QueueLoadFromSourceTask()
 {
   ChangeDelayLoadStatus(true);
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADING;
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_LOADING);
   RunInStableState(
     NS_NewRunnableMethod(this, &HTMLMediaElement::LoadFromSourceChildren));
 }
@@ -757,7 +761,7 @@ void HTMLMediaElement::QueueSelectResourceTask()
   if (mHaveQueuedSelectResource)
     return;
   mHaveQueuedSelectResource = true;
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE;
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE);
   RunInStableState(
     NS_NewRunnableMethod(this, &HTMLMediaElement::SelectResourceWrapper));
 }
@@ -818,7 +822,7 @@ void HTMLMediaElement::SelectResource()
       !HasSourceChildren(this)) {
     // The media element has neither a src attribute nor any source
     // element children, abort the load.
-    mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
+    ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_EMPTY);
     // This clears mDelayingLoadEvent, so AddRemoveSelfReference will be called
     ChangeDelayLoadStatus(false);
     return;
@@ -826,7 +830,7 @@ void HTMLMediaElement::SelectResource()
 
   ChangeDelayLoadStatus(true);
 
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADING;
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_LOADING);
   // Load event was delayed, and still is, so no need to call
   // AddRemoveSelfReference, since it must still be held
   DispatchAsyncEvent(NS_LITERAL_STRING("loadstart"));
@@ -922,7 +926,7 @@ void HTMLMediaElement::LoadFromSourceChildren()
       // Exhausted candidates, wait for more candidates to be appended to
       // the media element.
       mLoadWaitStatus = WAITING_FOR_SOURCE;
-      mNetworkState = nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE;
+      ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_NO_SOURCE);
       ChangeDelayLoadStatus(false);
       ReportLoadError("MediaLoadExhaustedCandidates");
       return;
@@ -991,8 +995,7 @@ void HTMLMediaElement::LoadFromSourceChildren()
 void HTMLMediaElement::SuspendLoad()
 {
   mSuspendedForPreloadNone = true;
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_IDLE;
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_IDLE);
-  DispatchAsyncEvent(NS_LITERAL_STRING("suspend"));
   ChangeDelayLoadStatus(false);
 }
 
@@ -1003,7 +1006,7 @@ void HTMLMediaElement::ResumeLoad(PreloadAction aAction)
   mSuspendedForPreloadNone = false;
   mPreloadAction = aAction;
   ChangeDelayLoadStatus(true);
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADING;
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_LOADING);
   if (!mIsLoadingFromSourceChildren) {
     // We were loading from the element's src attribute.
     if (NS_FAILED(LoadResource())) {
@@ -2155,11 +2158,10 @@ void HTMLMediaElement::SetPlayedOrSeeked(bool aValue)
 void
 HTMLMediaElement::ResetConnectionState()
 {
-  mBegun = false;
   SetCurrentTime(0);
   FireTimeUpdate(false);
   DispatchAsyncEvent(NS_LITERAL_STRING("ended"));
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_EMPTY);
   AddRemoveSelfReference();
   ChangeDelayLoadStatus(false);
   ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_NOTHING);
@@ -2647,7 +2649,7 @@ nsresult HTMLMediaElement::FinishDecoderSetup(MediaDecoder* aDecoder,
                                               nsIStreamListener** aListener,
                                               MediaDecoder* aCloneDonor)
 {
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADING;
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_LOADING);
 
   // Force a same-origin check before allowing events for this media resource.
   mMediaSecurityVerified = false;
@@ -2668,6 +2670,8 @@ nsresult HTMLMediaElement::FinishDecoderSetup(MediaDecoder* aDecoder,
   mDecoder->SetVolume(mMuted ? 0.0 : mVolume);
   mDecoder->SetPreservesPitch(mPreservesPitch);
   mDecoder->SetPlaybackRate(mPlaybackRate);
+  // Start progress timer for we are in NETWORK_LOADING.
+  mDecoder->StartProgress();
 
 #ifdef MOZ_EME
   if (mMediaKeys) {
@@ -2719,7 +2723,6 @@ nsresult HTMLMediaElement::FinishDecoderSetup(MediaDecoder* aDecoder,
   NS_ASSERTION(NS_SUCCEEDED(rv) == (MediaElementTableCount(this, mLoadingSrc) == 1),
     "Media element should have single table entry if decode initialized");
 
-  mBegun = true;
   return rv;
 }
 
@@ -2868,8 +2871,7 @@ void HTMLMediaElement::SetupSrcMediaStreamPlayback(DOMMediaStream* aStream)
   ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_METADATA);
   DispatchAsyncEvent(NS_LITERAL_STRING("durationchange"));
   DispatchAsyncEvent(NS_LITERAL_STRING("loadedmetadata"));
-  DispatchAsyncEvent(NS_LITERAL_STRING("suspend"));
+  ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_IDLE);
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_IDLE;
   AddRemoveSelfReference();
   // FirstFrameLoaded() will be called when the stream has current data.
 }
@@ -2996,13 +2998,12 @@ void HTMLMediaElement::Error(uint16_t aErrorCode)
                aErrorCode == nsIDOMMediaError::MEDIA_ERR_ABORTED,
                "Only use nsIDOMMediaError codes!");
   mError = new MediaError(this, aErrorCode);
-  mBegun = false;
   DispatchAsyncEvent(NS_LITERAL_STRING("error"));
   if (mReadyState == nsIDOMHTMLMediaElement::HAVE_NOTHING) {
-    mNetworkState = nsIDOMHTMLMediaElement::NETWORK_EMPTY;
+    ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_EMPTY);
     DispatchAsyncEvent(NS_LITERAL_STRING("emptied"));
   } else {
-    mNetworkState = nsIDOMHTMLMediaElement::NETWORK_IDLE;
+    ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_IDLE);
   }
   AddRemoveSelfReference();
   ChangeDelayLoadStatus(false);
@@ -3076,22 +3077,28 @@ void HTMLMediaElement::NotifySuspendedByCache(bool aIsSuspended)
 
 void HTMLMediaElement::DownloadSuspended()
 {
-  DispatchAsyncEvent(NS_LITERAL_STRING("progress"));
+  DownloadProgressed();
   if (mBegun) {
-    mNetworkState = nsIDOMHTMLMediaElement::NETWORK_IDLE;
+    ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_IDLE);
     AddRemoveSelfReference();
-    DispatchAsyncEvent(NS_LITERAL_STRING("suspend"));
   }
 }
 
 void HTMLMediaElement::DownloadResumed(bool aForceNetworkLoading)
 {
   if (mBegun || aForceNetworkLoading) {
-    mNetworkState = nsIDOMHTMLMediaElement::NETWORK_LOADING;
+    ChangeNetworkState(nsIDOMHTMLMediaElement::NETWORK_LOADING);
     AddRemoveSelfReference();
   }
 }
 
+void HTMLMediaElement::DownloadProgressed()
+{
+  if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING) {
+    DispatchAsyncEvent(NS_LITERAL_STRING("progress"));
+  }
+}
+
 void HTMLMediaElement::DownloadStalled()
 {
   if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING) {
@@ -3175,7 +3182,7 @@ void HTMLMediaElement::UpdateReadyStateForData(MediaDecoderOwner::NextFrameStatu
 }
 
 #ifdef PR_LOGGING
-static const char* gReadyStateToString[] = {
+static const char* const gReadyStateToString[] = {
   "HAVE_NOTHING",
   "HAVE_METADATA",
   "HAVE_CURRENT_DATA",
@@ -3234,6 +3241,50 @@ void HTMLMediaElement::ChangeReadyState(nsMediaReadyState aState)
   }
 }
 
+#ifdef PR_LOGGING
+static const char* const gNetworkStateToString[] = {
+  "EMPTY",
+  "IDLE",
+  "LOADING",
+  "NO_SOURCE"
+ };
+#endif
+
+void HTMLMediaElement::ChangeNetworkState(nsMediaNetworkState aState)
+{
+  if (mNetworkState == aState) {
+    return;
+  }
+
+  nsMediaNetworkState oldState = mNetworkState;
+  mNetworkState = aState;
+  LOG(PR_LOG_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;
+    if (mDecoder) {
+      // Stop progress notification when exiting NETWORK_LOADING.
+      mDecoder->StopProgress();
+    }
+  }
+
+  if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING) {
+    // Download is begun.
+    mBegun = true;
+    if (mDecoder) {
+      // Start progress notification when entering NETWORK_LOADING.
+      mDecoder->StartProgress();
+    }
+  } else if (mNetworkState == nsIDOMHTMLMediaElement::NETWORK_IDLE && !mError) {
+    // Fire 'suspend' event when entering NETWORK_IDLE and no error presented.
+    DispatchAsyncEvent(NS_LITERAL_STRING("suspend"));
+  }
+}
+
 bool HTMLMediaElement::CanActivateAutoplay()
 {
   // For stream inputs, we activate autoplay on HAVE_CURRENT_DATA because
@@ -3362,7 +3413,7 @@ bool HTMLMediaElement::IsPlaybackEnded() const
   // TODO:
   //   the current playback position is equal to the effective end of the media resource.
   //   See bug 449157.
-  return mNetworkState >= nsIDOMHTMLMediaElement::HAVE_METADATA &&
+  return mReadyState >= nsIDOMHTMLMediaElement::HAVE_METADATA &&
     mDecoder ? mDecoder->IsEnded() : false;
 }
 

content/media/MediaDecoder.cpp

@@ -139,7 +139,6 @@ void MediaDecoder::SetDormantIfNecessary(bool aDormant)
 
   if(aDormant) {
     // enter dormant state
-    StopProgress();
     DestroyDecodedStream();
     mDecoderStateMachine->SetDormant(true);
 
@@ -500,7 +499,8 @@ void MediaDecoder::Shutdown()
 
   ChangeState(PLAY_STATE_SHUTDOWN);
 
-  StopProgress();
+  // If we hit this assertion, there might be a bug in network state transition.
+  NS_ASSERTION(!mProgressTimer, "Progress timer should've been stopped.");
   mOwner = nullptr;
 
   MediaShutdownManager::Instance().Unregister(this);
@@ -1517,6 +1517,7 @@ static void ProgressCallback(nsITimer* aTimer, void* aClosure)
 
 void MediaDecoder::Progress(bool aTimer)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   if (!mOwner)
     return;
 
@@ -1532,7 +1533,7 @@ void MediaDecoder::Progress(bool aTimer)
        now - mProgressTime >= TimeDuration::FromMilliseconds(PROGRESS_MS)) &&
       !mDataTime.IsNull() &&
       now - mDataTime <= TimeDuration::FromMilliseconds(PROGRESS_MS)) {
-    mOwner->DispatchAsyncEvent(NS_LITERAL_STRING("progress"));
+    mOwner->DownloadProgressed();
     mProgressTime = now;
   }
 
@@ -1546,8 +1547,8 @@ void MediaDecoder::Progress(bool aTimer)
 
 nsresult MediaDecoder::StartProgress()
 {
-  if (mProgressTimer)
+  MOZ_ASSERT(NS_IsMainThread());
-    return NS_OK;
+  NS_ASSERTION(!mProgressTimer, "Already started progress timer.");
 
   mProgressTimer = do_CreateInstance("@mozilla.org/timer;1");
   return mProgressTimer->InitWithFuncCallback(ProgressCallback,
@@ -1558,8 +1559,8 @@ nsresult MediaDecoder::StartProgress()
 
 nsresult MediaDecoder::StopProgress()
 {
-  if (!mProgressTimer)
+  MOZ_ASSERT(NS_IsMainThread());
-    return NS_OK;
+  NS_ASSERTION(mProgressTimer, "Already stopped progress timer.");
 
   nsresult rv = mProgressTimer->Cancel();
   mProgressTimer = nullptr;

content/media/MediaDecoder.h

@@ -689,6 +689,12 @@ public:
     return mPlayState;
   }
 
+  // Called by the media element to start timer to update download progress.
+  nsresult StartProgress();
+
+  // Called by the media element to stop progress information timer.
+  nsresult StopProgress();
+
   // Fire progress events if needed according to the time and byte
   // constraints outlined in the specification. aTimer is true
   // if the method is called as a result of the progress timer rather
@@ -1166,12 +1172,6 @@ protected:
   // True if the stream is infinite (e.g. a webradio).
   bool mInfiniteStream;
 
-  // Start timer to update download progress information.
-  nsresult StartProgress();
-
-  // Stop progress information timer.
-  nsresult StopProgress();
-
   // Ensures our media stream has been pinned.
   void PinForSeek();
 

content/media/MediaDecoderOwner.h

@@ -18,6 +18,8 @@ class HTMLMediaElement;
 class MediaDecoderOwner
 {
 public:
+  // Called by the media decoder to indicate that the download is progressing.
+  virtual void DownloadProgressed() = 0;
   // Called by the media decoder to indicate that the download has stalled
   // (no data has arrived for a while).
   virtual void DownloadStalled() = 0;

content/media/MediaResource.cpp

@@ -905,8 +905,11 @@ void ChannelMediaResource::Resume()
         // There is (or may be) data to read at mOffset, so start reading it.
         // Need to recreate the channel.
         CacheClientSeek(mOffset, false);
+        element->DownloadResumed();
+      } else {
+        // The channel remains dead. Do not notify DownloadResumed() which
+        // will leave the media element in NETWORK_LOADING state.
       }
-      element->DownloadResumed();
     }
   }
 }

content/media/omx/MediaOmxCommonDecoder.cpp

@@ -101,7 +101,6 @@ MediaOmxCommonDecoder::PauseStateMachine()
   if (!mDecoderStateMachine) {
     return;
   }
-  StopProgress();
   mDecoderStateMachine->SetDormant(true);
 }