Bug 1108838 - Dispatch "stalled" even when no bytes have been received. r=cpearce, a=sledru
authorKarl Tomlinson <karlt+@karlt.net>
Fri, 28 Nov 2014 17:07:15 +1300
changeset 242831 b07f9144d190
parent 242830 15e3be526862
child 242832 36535f9806e6
push id4319
push userryanvm@gmail.com
push date2015-01-14 14:36 +0000
treeherdermozilla-beta@f6d5f2303fea [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, sledru
bugs1108838
milestone36.0
Bug 1108838 - Dispatch "stalled" even when no bytes have been received. r=cpearce, a=sledru This is important for MediaSource, where there is no initial request to set up the stall counter by sending an initial progress event. For sources using ChannelMediaResource, this means that stalled can now fire before an HTTP response is received. Also reset stalled timer on transitions to NETWORK_LOADING, and don't run the progress timer while stalled.
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
dom/media/MediaResource.cpp
dom/media/RtspMediaResource.cpp
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -3086,68 +3086,87 @@ void HTMLMediaElement::CheckProgress(boo
 
   if (aHaveNewProgress) {
     mDataTime = now;
   }
 
   // If this is the first progress, or PROGRESS_MS has passed since the last
   // progress event fired and more data has arrived since then, fire a
   // progress event.
-  if (!mDataTime.IsNull() &&
-      (mProgressTime.IsNull() ||
-       (now - mProgressTime >= TimeDuration::FromMilliseconds(PROGRESS_MS) &&
-        mDataTime > mProgressTime))) {
+  NS_ASSERTION((mProgressTime.IsNull() && !aHaveNewProgress) ||
+               !mDataTime.IsNull(),
+               "null TimeStamp mDataTime should not be used in comparison");
+  if (mProgressTime.IsNull() ? aHaveNewProgress
+      : (now - mProgressTime >= TimeDuration::FromMilliseconds(PROGRESS_MS) &&
+         mDataTime > mProgressTime)) {
     DispatchAsyncEvent(NS_LITERAL_STRING("progress"));
     // Resolution() ensures that future data will have now > mProgressTime,
     // and so will trigger another event.  mDataTime is not reset because it
     // is still required to detect stalled; it is similarly offset by
     // resolution to indicate the new data has not yet arrived.
     mProgressTime = now - TimeDuration::Resolution();
     if (mDataTime > mProgressTime) {
       mDataTime = mProgressTime;
     }
-  }
-
-  if (!mDataTime.IsNull() &&
-      now - mDataTime >= TimeDuration::FromMilliseconds(STALL_MS)) {
+    if (!mProgressTimer) {
+      NS_ASSERTION(aHaveNewProgress,
+                   "timer dispatched when there was no timer");
+      // Were stalled.  Restart timer.
+      StartProgressTimer();
+    }
+  }
+
+  if (now - mDataTime >= TimeDuration::FromMilliseconds(STALL_MS)) {
     DispatchAsyncEvent(NS_LITERAL_STRING("stalled"));
-    // Null it out
-    mDataTime = TimeStamp();
+    NS_ASSERTION(mProgressTimer, "detected stalled without timer");
+    // Stop timer events, which prevents repeated stalled events until there
+    // is more progress.
+    StopProgress();
   }
 }
 
 /* static */
 void HTMLMediaElement::ProgressTimerCallback(nsITimer* aTimer, void* aClosure)
 {
   auto decoder = static_cast<HTMLMediaElement*>(aClosure);
   decoder->CheckProgress(false);
 }
 
-nsresult HTMLMediaElement::StartProgress()
+void HTMLMediaElement::StartProgressTimer()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mNetworkState == nsIDOMHTMLMediaElement::NETWORK_LOADING);
   NS_ASSERTION(!mProgressTimer, "Already started progress timer.");
 
   mProgressTimer = do_CreateInstance("@mozilla.org/timer;1");
-  return mProgressTimer->InitWithFuncCallback(ProgressTimerCallback,
-                                              this,
-                                              PROGRESS_MS,
-                                              nsITimer::TYPE_REPEATING_SLACK);
+  mProgressTimer->InitWithFuncCallback(ProgressTimerCallback,
+                                       this,
+                                       PROGRESS_MS,
+                                       nsITimer::TYPE_REPEATING_SLACK);
 }
 
-nsresult HTMLMediaElement::StopProgress()
+void HTMLMediaElement::StartProgress()
+{
+  // Record the time now for detecting stalled.
+  mDataTime = TimeStamp::NowLoRes();
+  // Reset mProgressTime so that mDataTime is not indicating bytes received
+  // after the last progress event.
+  mProgressTime = TimeStamp();
+  StartProgressTimer();
+}
+
+void HTMLMediaElement::StopProgress()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  NS_ASSERTION(mProgressTimer, "Already stopped progress timer.");
-
-  nsresult rv = mProgressTimer->Cancel();
+  if (!mProgressTimer) {
+    return;
+  }
+
+  mProgressTimer->Cancel();
   mProgressTimer = nullptr;
-
-  return rv;
 }
 
 void HTMLMediaElement::DownloadProgressed()
 {
   if (mNetworkState != nsIDOMHTMLMediaElement::NETWORK_LOADING) {
     return;
   }
   CheckProgress(true);
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -882,21 +882,25 @@ protected:
    * just been detected.  Otherwise the method is called as a result of the
    * progress timer.
    */
   void CheckProgress(bool aHaveNewProgress);
   static void ProgressTimerCallback(nsITimer* aTimer, void* aClosure);
   /**
    * Start timer to update download progress.
    */
-  nsresult StartProgress();
+  void StartProgressTimer();
   /**
-   * Stop progress information timer.
+   * Start sending progress and/or stalled events.
    */
-  nsresult StopProgress();
+  void StartProgress();
+  /**
+   * Stop progress information timer and events.
+   */
+  void StopProgress();
 
   /**
    * Dispatches an error event to a child source element.
    */
   void DispatchAsyncSourceError(nsIContent* aSourceElement);
 
   /**
    * Resets the media element for an error condition as per aErrorCode.
@@ -1082,19 +1086,18 @@ protected:
   TimeStamp mTimeUpdateTime;
 
   // Time that the last progress event was fired. Read/Write from the
   // main thread only.
   TimeStamp mProgressTime;
 
   // Time that data was last read from the media resource. Used for
   // computing if the download has stalled and to rate limit progress events
-  // when data is arriving slower than PROGRESS_MS. A value of null indicates
-  // that a stall event has already fired and not to fire another one until
-  // more data is received. Read/Write from the main thread only.
+  // when data is arriving slower than PROGRESS_MS.
+  // Read/Write from the main thread only.
   TimeStamp mDataTime;
 
   // Media 'currentTime' value when the last timeupdate event occurred.
   // Read/Write from the main thread only.
   double mLastCurrentTime;
 
   // Logical start time of the media resource in seconds as obtained
   // from any media fragments. A negative value indicates that no
--- a/dom/media/MediaResource.cpp
+++ b/dom/media/MediaResource.cpp
@@ -362,18 +362,17 @@ ChannelMediaResource::OnStartRequest(nsI
   if (mSuspendCount > 0) {
     // Re-suspend the channel if it needs to be suspended
     // No need to call PossiblySuspend here since the channel is
     // definitely in the right state for us in OnStartRequest.
     mChannel->Suspend();
     mIgnoreResume = false;
   }
 
-  // Fires an initial progress event and sets up the stall counter so stall events
-  // fire if no download occurs within the required time frame.
+  // Fires an initial progress event.
   owner->DownloadProgressed();
 
   return NS_OK;
 }
 
 bool
 ChannelMediaResource::IsTransportSeekable()
 {
--- a/dom/media/RtspMediaResource.cpp
+++ b/dom/media/RtspMediaResource.cpp
@@ -735,18 +735,17 @@ RtspMediaResource::OnConnected(uint8_t a
       mRealTime = true;
       bool seekable = false;
       mDecoder->SetInfinite(true);
       mDecoder->SetMediaSeekable(seekable);
     }
   }
   MediaDecoderOwner* owner = mDecoder->GetMediaOwner();
   NS_ENSURE_TRUE(owner, NS_ERROR_FAILURE);
-  // Fires an initial progress event and sets up the stall counter so stall events
-  // fire if no download occurs within the required time frame.
+  // Fires an initial progress event.
   owner->DownloadProgressed();
 
   dom::HTMLMediaElement* element = owner->GetMediaElement();
   NS_ENSURE_TRUE(element, NS_ERROR_FAILURE);
 
   element->FinishDecoderSetup(mDecoder, this);
   mIsConnected = true;