Bug 1391170 - lessen the assertion in AddMediaElementToURITable() so we can move MediaDecoder::GetResource() to private. draft
authorJW Wang <jwwang@mozilla.com>
Thu, 17 Aug 2017 15:41:22 +0800
changeset 650288 212104d817ed451336ae20ee5c2cc7da6a1ca59e
parent 650287 0ad7f637206268ea6fc20cda18f3d3c329dbac8f
child 650291 73765ef7cc368d44b57889eab962cf75f35545ea
push id75333
push userjwwang@mozilla.com
push dateTue, 22 Aug 2017 06:01:48 +0000
bugs1391170
milestone57.0a1
Bug 1391170 - lessen the assertion in AddMediaElementToURITable() so we can move MediaDecoder::GetResource() to private. If AddMediaElementToURITable() is called after the decoder Load failed, mDecoder will be reset and it is sufficient to assert mDecoder only. MozReview-Commit-ID: 58WT8zFeiFj
dom/html/HTMLMediaElement.cpp
dom/media/ChannelMediaDecoder.h
dom/media/MediaDecoder.h
dom/media/hls/HLSDecoder.h
dom/media/mediasource/MediaSourceDecoder.h
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -3688,17 +3688,17 @@ MediaElementTableCount(HTMLMediaElement*
   NS_ASSERTION(otherCount == 0, "Should not have entries for unknown URIs");
   return uriCount;
 }
 #endif
 
 void
 HTMLMediaElement::AddMediaElementToURITable()
 {
-  NS_ASSERTION(mDecoder && mDecoder->GetResource(), "Call this only with decoder Load called");
+  NS_ASSERTION(mDecoder, "Call this only with decoder Load called");
   NS_ASSERTION(MediaElementTableCount(this, mLoadingSrc) == 0,
     "Should not have entry for element in element table before addition");
   if (!gElementTable) {
     gElementTable = new MediaElementURITable();
   }
   MediaElementSetForURI* entry = gElementTable->PutEntry(mLoadingSrc);
   entry->mElements.AppendElement(this);
   NS_ASSERTION(MediaElementTableCount(this, mLoadingSrc) == 1,
--- a/dom/media/ChannelMediaDecoder.h
+++ b/dom/media/ChannelMediaDecoder.h
@@ -62,34 +62,34 @@ protected:
                       MediaDecoderEventVisibility aEventVisibility) override;
 
   RefPtr<ResourceCallback> mResourceCallback;
   RefPtr<BaseMediaResource> mResource;
 
 public:
   explicit ChannelMediaDecoder(MediaDecoderInit& aInit);
 
-  MediaResource* GetResource() const override final;
-
   void Shutdown() override;
 
   bool CanClone();
 
   // Create a new decoder of the same type as this one.
   already_AddRefed<ChannelMediaDecoder> Clone(MediaDecoderInit& aInit);
 
   nsresult Load(nsIChannel* aChannel,
                 bool aIsPrivateBrowsing,
                 nsIStreamListener** aStreamListener);
 
   void SetLoadInBackground(bool aLoadInBackground) override;
   void Suspend() override;
   void Resume() override;
 
 private:
+  MediaResource* GetResource() const override final;
+
   // Create a new state machine to run this decoder.
   MediaDecoderStateMachine* CreateStateMachine();
 
   nsresult OpenResource(nsIStreamListener** aStreamListener);
   nsresult Load(BaseMediaResource* aOriginal);
 
   // Called by MediaResource when the download has ended.
   // Called on the main thread only. aStatus is the result from OnStopRequest.
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -115,25 +115,16 @@ public:
   // Notified by the shutdown manager that XPCOM shutdown has begun.
   // The decoder should notify its owner to drop the reference to the decoder
   // to prevent further calls into the decoder.
   void NotifyXPCOMShutdown();
 
   // Called if the media file encounters a network error.
   void NetworkError();
 
-  // Get the current MediaResource being used.
-  // Note: The MediaResource is refcounted, but it outlives the MediaDecoder,
-  // so it's OK to use the reference returned by this function without
-  // refcounting, *unless* you need to store and use the reference after the
-  // MediaDecoder has been destroyed. You might need to do this if you're
-  // wrapping the MediaResource in some kind of byte stream interface to be
-  // passed to a platform decoder.
-  virtual MediaResource* GetResource() const = 0;
-
   // Return the principal of the current URI being played or downloaded.
   virtual already_AddRefed<nsIPrincipal> GetCurrentPrincipal();
 
   // Return the time position in the video stream being
   // played measured in seconds.
   virtual double GetCurrentTime();
 
   // Seek to the time position in (seconds) from the start of the video.
@@ -485,16 +476,25 @@ protected:
 
   // Amount of buffered data ahead of current time required to consider that
   // the next frame is available.
   // An arbitrary value of 250ms is used.
   static constexpr auto DEFAULT_NEXT_FRAME_AVAILABLE_BUFFERED =
     media::TimeUnit::FromMicroseconds(250000);
 
 private:
+  // Get the current MediaResource being used.
+  // Note: The MediaResource is refcounted, but it outlives the MediaDecoder,
+  // so it's OK to use the reference returned by this function without
+  // refcounting, *unless* you need to store and use the reference after the
+  // MediaDecoder has been destroyed. You might need to do this if you're
+  // wrapping the MediaResource in some kind of byte stream interface to be
+  // passed to a platform decoder.
+  virtual MediaResource* GetResource() const = 0;
+
   nsCString GetDebugInfo();
 
   // Called when the owner's activity changed.
   void NotifyCompositor();
 
   void OnPlaybackErrorEvent(const MediaResult& aError);
 
   void OnDecoderDoctorEvent(DecoderDoctorEvent aEvent);
--- a/dom/media/hls/HLSDecoder.h
+++ b/dom/media/hls/HLSDecoder.h
@@ -16,18 +16,16 @@ class HLSDecoder final : public MediaDec
 {
 public:
   // MediaDecoder interface.
   explicit HLSDecoder(MediaDecoderInit& aInit)
     : MediaDecoder(aInit)
   {
   }
 
-  MediaResource* GetResource() const override final;
-
   void Shutdown() override;
 
   // Returns true if the HLS backend is pref'ed on.
   static bool IsEnabled();
 
   // Returns true if aContainerType is an HLS type that we think we can render
   // with the a platform decoder backend.
   // If provided, codecs are checked for support.
@@ -38,16 +36,18 @@ public:
   nsresult Play() override;
 
   void Pause() override;
 
   void Suspend() override;
   void Resume() override;
 
 private:
+  MediaResource* GetResource() const override final;
+
   MediaDecoderStateMachine* CreateStateMachine();
 
   bool CanPlayThroughImpl() override final
   {
     // TODO: We don't know how to estimate 'canplaythrough' for this decoder.
     // For now we just return true for 'autoplay' can work.
     return true;
   }
--- a/dom/media/mediasource/MediaSourceDecoder.h
+++ b/dom/media/mediasource/MediaSourceDecoder.h
@@ -22,18 +22,16 @@ class MediaSource;
 
 } // namespace dom
 
 class MediaSourceDecoder : public MediaDecoder
 {
 public:
   explicit MediaSourceDecoder(MediaDecoderInit& aInit);
 
-  MediaResource* GetResource() const override final;
-
   nsresult Load(nsIPrincipal* aPrincipal);
   media::TimeIntervals GetSeekable() override;
   media::TimeIntervals GetBuffered() override;
 
   void Shutdown() override;
 
   void AttachMediaSource(dom::MediaSource* aMediaSource);
   void DetachMediaSource();
@@ -59,16 +57,17 @@ public:
 
   MediaDecoderOwner::NextFrameStatus NextFrameBufferedStatus() override;
 
   bool IsMSE() const override { return true; }
 
   void NotifyInitDataArrived();
 
 private:
+  MediaResource* GetResource() const override final;
   MediaDecoderStateMachine* CreateStateMachine();
   void DoSetMediaSourceDuration(double aDuration);
   media::TimeInterval ClampIntervalToEnd(const media::TimeInterval& aInterval);
   bool CanPlayThroughImpl() override;
   bool IsLiveStream() override final { return !mEnded; }
 
   RefPtr<MediaSourceResource> mResource;