Bug 883731 - Remove ResourceLoaded notifications since they don't make much sense with a media cache. r=cpearce

Based on roc's patch in https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=883731&attachment=767505.

content/html/content/public/HTMLMediaElement.h

@@ -162,15 +162,9 @@ public:
   virtual void MetadataLoaded(const MediaInfo* aInfo,
                               const MetadataTags* aTags) MOZ_FINAL MOZ_OVERRIDE;
 
-  // Called by the video decoder object, on the main thread,
-  // when it has read the first frame of the video
-  // aResourceFullyLoaded should be true if the resource has been
-  // fully loaded and the caller will call ResourceLoaded next.
-  virtual void FirstFrameLoaded(bool aResourceFullyLoaded) MOZ_FINAL MOZ_OVERRIDE;
-
-  // Called by the video decoder object, on the main thread,
-  // when the resource has completed downloading.
-  virtual void ResourceLoaded() MOZ_FINAL MOZ_OVERRIDE;
+  // Called by the decoder object, on the main thread,
+  // when it has read the first frame of the video or audio.
+  virtual void FirstFrameLoaded() MOZ_FINAL MOZ_OVERRIDE;
 
   // Called by the video decoder object, on the main thread,
   // when the resource has a network error during loading.
@@ -238,10 +232,6 @@ public:
   // HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA.
   virtual void UpdateReadyStateForData(MediaDecoderOwner::NextFrameStatus aNextFrame) MOZ_FINAL MOZ_OVERRIDE;
 
-  // Use this method to change the mReadyState member, so required
-  // events can be fired.
-  void ChangeReadyState(nsMediaReadyState aState);
-
   // Return true if we can activate autoplay assuming enough data has arrived.
   bool CanActivateAutoplay();
 
@@ -540,7 +530,7 @@ public:
 
   already_AddRefed<Promise> SetMediaKeys(MediaKeys* mediaKeys,
                                          ErrorResult& aRv);
-  
+
   MediaWaitingFor WaitingFor() const;
 
   mozilla::dom::EventHandlerNonNull* GetOnencrypted();
@@ -648,6 +638,11 @@ protected:
     nsCOMPtr<nsITimer> mTimer;
   };
 
+  /** Use this method to change the mReadyState member, so required
+   * events can be fired.
+   */
+  void ChangeReadyState(nsMediaReadyState aState);
+
   /**
    * These two methods are called by the WakeLockBoolWrapper when the wakelock
    * has to be created or released.
@@ -933,7 +928,7 @@ protected:
   // desired, and we'll seek to the sync point (keyframe and/or start of the
   // next block of audio samples) preceeding seek target.
   void Seek(double aTime, SeekTarget::Type aSeekType, ErrorResult& aRv);
-  
+
   // Update the audio channel playing state
   void UpdateAudioChannelPlayingState();
 
@@ -1104,9 +1099,8 @@ protected:
   // Set to false when completed, or not yet started.
   bool mBegun;
 
-  // True when the decoder has loaded enough data to display the
-  // first frame of the content.
-  bool mLoadedFirstFrame;
+  // 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

content/html/content/src/HTMLMediaElement.cpp

@@ -661,7 +661,8 @@ void HTMLMediaElement::AbortExistingLoads()
   }
 
   mError = nullptr;
-  mLoadedFirstFrame = false;
+  mBegun = false;
+  mLoadedDataFired = false;
   mAutoplaying = true;
   mIsLoadingFromSourceChildren = false;
   mSuspendedAfterFirstFrame = false;
@@ -2027,7 +2028,7 @@ HTMLMediaElement::HTMLMediaElement(already_AddRefed<mozilla::dom::NodeInfo>& aNo
     mPlayed(new TimeRanges),
     mCurrentPlayRangeStart(-1.0),
     mBegun(false),
-    mLoadedFirstFrame(false),
+    mLoadedDataFired(false),
     mAutoplaying(true),
     mAutoplayEnabled(true),
     mPaused(true),
@@ -2776,7 +2777,7 @@ public:
     mHaveCurrentData = true;
     if (mElement) {
       nsRefPtr<HTMLMediaElement> deathGrip = mElement;
-      mElement->FirstFrameLoaded(false);
+      mElement->FirstFrameLoaded();
     }
     UpdateReadyStateForData();
     DoNotifyOutput();
@@ -2870,8 +2871,7 @@ void HTMLMediaElement::SetupSrcMediaStreamPlayback(DOMMediaStream* aStream)
   DispatchAsyncEvent(NS_LITERAL_STRING("suspend"));
   mNetworkState = nsIDOMHTMLMediaElement::NETWORK_IDLE;
   AddRemoveSelfReference();
-  // FirstFrameLoaded(false) will be called when the stream has current data,
-  // to complete the setup by entering the HAVE_CURRENT_DATA state.
+  // FirstFrameLoaded() will be called when the stream has current data.
 }
 
 void HTMLMediaElement::EndSrcMediaStreamPlayback()
@@ -2921,6 +2921,7 @@ void HTMLMediaElement::MetadataLoaded(const MediaInfo* aInfo,
 {
   mHasAudio = aInfo->HasAudio();
   mTags = aTags;
+  mLoadedDataFired = false;
   ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_METADATA);
   DispatchAsyncEvent(NS_LITERAL_STRING("durationchange"));
   DispatchAsyncEvent(NS_LITERAL_STRING("loadedmetadata"));
@@ -2940,25 +2941,19 @@ void HTMLMediaElement::MetadataLoaded(const MediaInfo* aInfo,
   }
 }
 
-void HTMLMediaElement::FirstFrameLoaded(bool aResourceFullyLoaded)
+void HTMLMediaElement::FirstFrameLoaded()
 {
-  ChangeReadyState(aResourceFullyLoaded ?
-    nsIDOMHTMLMediaElement::HAVE_ENOUGH_DATA :
-    nsIDOMHTMLMediaElement::HAVE_CURRENT_DATA);
-  ChangeDelayLoadStatus(false);
-
   NS_ASSERTION(!mSuspendedAfterFirstFrame, "Should not have already suspended");
 
+  ChangeDelayLoadStatus(false);
+
   if (mDecoder && mAllowSuspendAfterFirstFrame && mPaused &&
-      !aResourceFullyLoaded &&
       !HasAttr(kNameSpaceID_None, nsGkAtoms::autoplay) &&
       mPreloadAction == HTMLMediaElement::PRELOAD_METADATA) {
     mSuspendedAfterFirstFrame = true;
     mDecoder->Suspend();
-  } else if (mLoadedFirstFrame &&
-             mDownloadSuspendedByCache &&
-             mDecoder &&
-             !mDecoder->IsEnded()) {
+  } else if (mDownloadSuspendedByCache &&
+             mDecoder && !mDecoder->IsEnded()) {
     // We've already loaded the first frame, and the decoder has signalled
     // that the download has been suspended by the media cache. So move
     // readyState into HAVE_ENOUGH_DATA, in case there's script waiting
@@ -2972,26 +2967,6 @@ void HTMLMediaElement::FirstFrameLoaded(bool aResourceFullyLoaded)
   }
 }
 
-void HTMLMediaElement::ResourceLoaded()
-{
-  NS_ASSERTION(!mSrcStream, "Don't call this for streams");
-
-  mBegun = false;
-  mNetworkState = nsIDOMHTMLMediaElement::NETWORK_IDLE;
-  AddRemoveSelfReference();
-  if (mReadyState >= nsIDOMHTMLMediaElement::HAVE_METADATA) {
-    // MediaStream sources are put into HAVE_CURRENT_DATA state here on setup. If the
-    // stream is not blocked, we will receive a notification that will put it
-    // into HAVE_ENOUGH_DATA state.
-    ChangeReadyState(mSrcStream ? nsIDOMHTMLMediaElement::HAVE_CURRENT_DATA
-                     : nsIDOMHTMLMediaElement::HAVE_ENOUGH_DATA);
-  }
-  // Ensure a progress event is dispatched at the end of download.
-  DispatchAsyncEvent(NS_LITERAL_STRING("progress"));
-  // The download has stopped.
-  DispatchAsyncEvent(NS_LITERAL_STRING("suspend"));
-}
-
 void HTMLMediaElement::NetworkError()
 {
   Error(nsIDOMMediaError::MEDIA_ERR_NETWORK);
@@ -3146,8 +3121,7 @@ void HTMLMediaElement::UpdateReadyStateForData(MediaDecoderOwner::NextFrameStatu
 {
   if (mReadyState < nsIDOMHTMLMediaElement::HAVE_METADATA) {
     // aNextFrame might have a next frame because the decoder can advance
-    // on its own thread before ResourceLoaded or MetadataLoaded gets
-    // a chance to run.
+    // on its own thread before MetadataLoaded gets a chance to run.
     // The arrival of more data can't change us out of this readyState.
     return;
   }
@@ -3160,17 +3134,16 @@ void HTMLMediaElement::UpdateReadyStateForData(MediaDecoderOwner::NextFrameStatu
     return;
   }
 
-  if (mReadyState > nsIDOMHTMLMediaElement::HAVE_METADATA &&
-      mDownloadSuspendedByCache &&
-      mDecoder &&
-      !mDecoder->IsEnded()) {
-    // The decoder has signalled that the download has been suspended by the
+  if (mDownloadSuspendedByCache && mDecoder && !mDecoder->IsEnded()) {
+    // The decoder has signaled that the download has been suspended by the
     // media cache. So move readyState into HAVE_ENOUGH_DATA, in case there's
     // script waiting for a "canplaythrough" event; without this forced
     // transition, we will never fire the "canplaythrough" event if the
     // media cache is too small, and scripts are bound to fail. Don't force
     // this transition if the decoder is in ended state; the readyState
     // should remain at HAVE_CURRENT_DATA in this case.
+    // Note that this state transition includes the case where we finished
+    // downloaded the whole data stream.
     ChangeReadyState(nsIDOMHTMLMediaElement::HAVE_ENOUGH_DATA);
     return;
   }
@@ -3241,10 +3214,9 @@ void HTMLMediaElement::ChangeReadyState(nsMediaReadyState aState)
 
   if (oldState < nsIDOMHTMLMediaElement::HAVE_CURRENT_DATA &&
       mReadyState >= nsIDOMHTMLMediaElement::HAVE_CURRENT_DATA &&
-      !mLoadedFirstFrame)
-  {
+      !mLoadedDataFired) {
     DispatchAsyncEvent(NS_LITERAL_STRING("loadeddata"));
-    mLoadedFirstFrame = true;
+    mLoadedDataFired = true;
   }
 
   if (mReadyState == nsIDOMHTMLMediaElement::HAVE_CURRENT_DATA) {

content/media/MediaCache.cpp

@@ -1857,7 +1857,11 @@ MediaCacheStream::NotifyDataEnded(nsresult aStatus)
     mResourceID = gMediaCache->AllocateResourceID();
   }
 
+  // It is prudent to update channel/cache status before calling
+  // CacheClientNotifyDataEnded() which will read |mChannelEnded|.
   FlushPartialBlockInternal(true);
+  mChannelEnded = true;
+  gMediaCache->QueueUpdate();
 
   MediaCache::ResourceStreamIterator iter(mResourceID);
   while (MediaCacheStream* stream = iter.Next()) {
@@ -1871,9 +1875,6 @@ MediaCacheStream::NotifyDataEnded(nsresult aStatus)
       stream->mClient->CacheClientNotifyDataEnded(aStatus);
     }
   }
-
-  mChannelEnded = true;
-  gMediaCache->QueueUpdate();
 }
 
 void

content/media/MediaDecoder.cpp

@@ -440,7 +440,6 @@ MediaDecoder::MediaDecoder() :
   mIsExitingDormant(false),
   mPlayState(PLAY_STATE_PAUSED),
   mNextState(PLAY_STATE_PAUSED),
-  mCalledResourceLoaded(false),
   mIgnoreProgressData(false),
   mInfiniteStream(false),
   mOwner(nullptr),
@@ -724,20 +723,8 @@ void MediaDecoder::MetadataLoaded(MediaInfo* aInfo, MetadataTags* aTags)
     mOwner->MetadataLoaded(aInfo, aTags);
   }
 
-  if (!mCalledResourceLoaded) {
-    StartProgress();
-  } else if (mOwner) {
-    // Resource was loaded during metadata loading, when progress
-    // events are being ignored. Fire the final progress event.
-    mOwner->DispatchAsyncEvent(NS_LITERAL_STRING("progress"));
-  }
-
-  // Only inform the element of FirstFrameLoaded if not doing a load() in order
-  // to fulfill a seek, otherwise we'll get multiple loadedfirstframe events.
-  bool notifyResourceIsLoaded = !mCalledResourceLoaded &&
-                                IsDataCachedToEndOfResource();
   if (mOwner) {
-    mOwner->FirstFrameLoaded(notifyResourceIsLoaded);
+    mOwner->FirstFrameLoaded();
   }
 
   // This can run cache callbacks.
@@ -756,45 +743,11 @@ void MediaDecoder::MetadataLoaded(MediaInfo* aInfo, MetadataTags* aTags)
     }
   }
 
-  if (notifyResourceIsLoaded) {
-    ResourceLoaded();
-  }
-
   // Run NotifySuspendedStatusChanged now to give us a chance to notice
   // that autoplay should run.
   NotifySuspendedStatusChanged();
 }
 
-void MediaDecoder::ResourceLoaded()
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  // Don't handle ResourceLoaded if we are shutting down, or if
-  // we need to ignore progress data due to seeking (in the case
-  // that the seek results in reaching end of file, we get a bogus call
-  // to ResourceLoaded).
-  if (mShuttingDown)
-    return;
-
-  {
-    // If we are seeking or loading then the resource loaded notification we get
-    // should be ignored, since it represents the end of the seek request.
-    ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
-    if (mIgnoreProgressData || mCalledResourceLoaded || mPlayState == PLAY_STATE_LOADING)
-      return;
-
-    Progress(false);
-
-    mCalledResourceLoaded = true;
-    StopProgress();
-  }
-
-  // Ensure the final progress event gets fired
-  if (mOwner) {
-    mOwner->ResourceLoaded();
-  }
-}
-
 void MediaDecoder::ResetConnectionState()
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -1029,12 +982,12 @@ void MediaDecoder::NotifyDownloadEnded(nsresult aStatus)
   }
 
   if (NS_SUCCEEDED(aStatus)) {
-    ResourceLoaded();
-  }
-  else if (aStatus != NS_BASE_STREAM_CLOSED) {
+    UpdateReadyStateForData();
+    // A final progress event will be fired by the MediaResource calling
+    // DownloadSuspended on the element.
+  } else if (aStatus != NS_BASE_STREAM_CLOSED) {
     NetworkError();
   }
-  UpdateReadyStateForData();
 }
 
 void MediaDecoder::NotifyPrincipalChanged()

content/media/MediaDecoder.h

@@ -308,9 +308,6 @@ public:
   // Called in |Load| to open mResource.
   nsresult OpenResource(nsIStreamListener** aStreamListener);
 
-  // Called when the video file has completed downloading.
-  virtual void ResourceLoaded();
-
   // Called if the media file encounters a network error.
   virtual void NetworkError();
 
@@ -1159,11 +1156,6 @@ protected:
   // been requested. When a seek is started this is reset to invalid.
   SeekTarget mRequestedSeekTarget;
 
-  // True when we have fully loaded the resource and reported that
-  // to the element (i.e. reached NETWORK_LOADED state).
-  // Accessed on the main thread only.
-  bool mCalledResourceLoaded;
-
   // True when seeking or otherwise moving the play position around in
   // such a manner that progress event data is inaccurate. This is set
   // during seek and duration operations to prevent the progress indicator

content/media/MediaDecoderOwner.h

@@ -52,15 +52,9 @@ public:
   virtual void MetadataLoaded(const MediaInfo* aInfo,
                               const MetadataTags* aTags) = 0;
 
-  // Called by the video decoder object, on the main thread,
-  // when it has read the first frame of the video
-  // aResourceFullyLoaded should be true if the resource has been
-  // fully loaded and the caller will call ResourceLoaded next.
-  virtual void FirstFrameLoaded(bool aResourceFullyLoaded) = 0;
-
-  // Called by the video decoder object, on the main thread,
-  // when the resource has completed downloading.
-  virtual void ResourceLoaded() = 0;
+  // Called by the decoder object, on the main thread,
+  // when it has read the first frame of the video or audio.
+  virtual void FirstFrameLoaded() = 0;
 
   // Called by the video decoder object, on the main thread,
   // when the resource has a network error during loading.

content/media/MediaResource.cpp

@@ -984,11 +984,24 @@ public:
     mDecoder(aDecoder), mStatus(aStatus) {}
   NS_IMETHOD Run() {
     mDecoder->NotifyDownloadEnded(mStatus);
+    if (NS_SUCCEEDED(mStatus)) {
+      MediaDecoderOwner* owner = mDecoder->GetMediaOwner();
+      if (owner) {
+        dom::HTMLMediaElement* element = owner->GetMediaElement();
+        if (element) {
+          element->DownloadSuspended();
+        }
+      }
+      // NotifySuspendedStatusChanged will tell the element that download
+      // has been suspended "by the cache", which is true since we never download
+      // anything. The element can then transition to HAVE_ENOUGH_DATA.
+      mDecoder->NotifySuspendedStatusChanged();
+    }
     return NS_OK;
   }
 private:
   nsRefPtr<MediaDecoder> mDecoder;
-  nsresult                 mStatus;
+  nsresult               mStatus;
 };
 
 void
@@ -1248,8 +1261,8 @@ public:
     return std::max(aOffset, mSize);
   }
   virtual bool    IsDataCachedToEndOfResource(int64_t aOffset) { return true; }
-  virtual bool    IsSuspendedByCache() { return false; }
-  virtual bool    IsSuspended() { return false; }
+  virtual bool    IsSuspendedByCache() { return true; }
+  virtual bool    IsSuspended() { return true; }
   virtual bool    IsTransportSeekable() MOZ_OVERRIDE { return true; }
 
   nsresult GetCachedRanges(nsTArray<MediaByteRange>& aRanges);