Bug 513144. Move ownership of the Ogg decoder's nsMediaStream out from nsChannelReader to nsOggDecoder. Also remove unnecessary mNotifyOnShutdown flag from nsOggDecoder and nsWaveDecoder. Since Load can only be called once after Init, remove unnecessary initialization of variables in Load and move some code up from Load to Init in both backends to simplify Load. r=doublec

--HG--
extra : rebase_source : 65cd9f01424f5b8ab8507e4788f2b8228ce1e2b0

content/media/nsMediaDecoder.h

@@ -111,6 +111,7 @@ public:
   // point of the first frame of data.
   // Exactly one of aURI and aChannel must be null. aListener must be
   // null if and only if aChannel is.
+  // This is called at most once per decoder, after Init().
   virtual nsresult Load(nsIURI* aURI,
                         nsIChannel* aChannel,
                         nsIStreamListener **aListener) = 0;

content/media/ogg/nsChannelReader.cpp

@@ -131,12 +131,9 @@ static ogg_int64_t oggplay_channel_reader_duration(struct _OggPlayReader *aReade
   return me->duration();
 }
 
-nsresult nsChannelReader::Init(nsMediaDecoder* aDecoder, nsIURI* aURI,
-                               nsIChannel* aChannel,
-                               nsIStreamListener** aStreamListener)
+void nsChannelReader::Init(nsMediaStream* aStream)
 {
-  return nsMediaStream::Open(aDecoder, aURI, aChannel,
-                             getter_Transfers(mStream), aStreamListener);
+  mStream = aStream;
 }
 
 nsChannelReader::~nsChannelReader()

content/media/ogg/nsChannelReader.h

@@ -54,14 +54,10 @@ public:
   ~nsChannelReader();
 
   /**
-   * Initialize the reader with the given decoder, URI, and
-   * optional channel.
-   * @param aChannel may be null
-   * @param aStreamListener if aChannel is non-null, this will return
-   * a stream listener which should be attached to the channel.
+   * Initialize the reader with the edia stream.
+   * This takes ownership of aStream.
    */
-  nsresult Init(nsMediaDecoder* aDecoder, nsIURI* aURI, nsIChannel* aChannel,
-                nsIStreamListener** aStreamListener);
+  void Init(nsMediaStream* aStream);
 
   nsMediaStream* Stream() { return mStream; }
 

content/media/ogg/nsOggDecoder.cpp

@@ -1878,7 +1878,6 @@ nsOggDecoder::nsOggDecoder() :
   mInitialVolume(0.0),
   mRequestedSeekTime(-1.0),
   mDuration(-1),
-  mNotifyOnShutdown(PR_FALSE),
   mSeekable(PR_TRUE),
   mReader(nsnull),
   mMonitor(nsnull),
@@ -1892,8 +1891,19 @@ nsOggDecoder::nsOggDecoder() :
 
 PRBool nsOggDecoder::Init(nsHTMLMediaElement* aElement)
 {
+  if (!nsMediaDecoder::Init(aElement))
+    return PR_FALSE;
+
   mMonitor = nsAutoMonitor::NewMonitor("media.decoder");
-  return mMonitor && nsMediaDecoder::Init(aElement);
+  if (!mMonitor)
+    return PR_FALSE;
+
+  RegisterShutdownObserver();
+
+  mReader = new nsChannelReader();
+  NS_ENSURE_TRUE(mReader, PR_FALSE);
+
+  return PR_TRUE;
 }
 
 void nsOggDecoder::Stop()
@@ -1958,15 +1968,6 @@ nsOggDecoder::~nsOggDecoder()
 nsresult nsOggDecoder::Load(nsIURI* aURI, nsIChannel* aChannel,
                             nsIStreamListener** aStreamListener)
 {
-  // Reset progress member variables
-  mDecoderPosition = 0;
-  mPlaybackPosition = 0;
-  mResourceLoaded = PR_FALSE;
-
-  NS_ASSERTION(!mReader, "Didn't shutdown properly!");
-  NS_ASSERTION(!mDecodeStateMachine, "Didn't shutdown properly!");
-  NS_ASSERTION(!mDecodeThread, "Didn't shutdown properly!");
-
   if (aStreamListener) {
     *aStreamListener = nsnull;
   }
@@ -1985,22 +1986,20 @@ nsresult nsOggDecoder::Load(nsIURI* aURI, nsIChannel* aChannel,
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  RegisterShutdownObserver();
-
-  mReader = new nsChannelReader();
-  NS_ENSURE_TRUE(mReader, NS_ERROR_OUT_OF_MEMORY);
-
   {
-    nsAutoMonitor mon(mMonitor);
     // Hold the lock while we do this to set proper lock ordering
     // expectations for dynamic deadlock detectors: decoder lock(s)
     // should be grabbed before the cache lock
-    nsresult rv = mReader->Init(this, mURI, aChannel, aStreamListener);
+    nsAutoMonitor mon(mMonitor);
+
+    nsAutoPtr<nsMediaStream> stream;
+    nsresult rv = nsMediaStream::Open(this, mURI, aChannel,
+                                      getter_Transfers(stream),
+                                      aStreamListener);
     if (NS_FAILED(rv)) {
-      // Free the failed-to-initialize reader so we don't try to use it.
-      mReader = nsnull;
       return rv;
     }
+    mReader->Init(stream.forget());
   }
 
   nsresult rv = NS_NewThread(getter_AddRefs(mDecodeThread));
@@ -2398,30 +2397,23 @@ void nsOggDecoder::SeekingStarted()
 
 void nsOggDecoder::RegisterShutdownObserver()
 {
-  if (!mNotifyOnShutdown) {
-    nsCOMPtr<nsIObserverService> observerService =
-      do_GetService("@mozilla.org/observer-service;1");
-    if (observerService) {
-      mNotifyOnShutdown = 
-        NS_SUCCEEDED(observerService->AddObserver(this, 
-                                                  NS_XPCOM_SHUTDOWN_OBSERVER_ID, 
-                                                  PR_FALSE));
-    }
-    else {
-      NS_WARNING("Could not get an observer service. Video decoding events may not shutdown cleanly.");
-    }
+  nsCOMPtr<nsIObserverService> observerService =
+    do_GetService("@mozilla.org/observer-service;1");
+  if (observerService) {
+    observerService->AddObserver(this, 
+                                 NS_XPCOM_SHUTDOWN_OBSERVER_ID, 
+                                 PR_FALSE);
+  } else {
+    NS_WARNING("Could not get an observer service. Video decoding events may not shutdown cleanly.");
   }
 }
 
 void nsOggDecoder::UnregisterShutdownObserver()
 {
-  if (mNotifyOnShutdown) {
-    nsCOMPtr<nsIObserverService> observerService =
-      do_GetService("@mozilla.org/observer-service;1");
-    if (observerService) {
-      mNotifyOnShutdown = PR_FALSE;
-      observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
-    }
+  nsCOMPtr<nsIObserverService> observerService =
+    do_GetService("@mozilla.org/observer-service;1");
+  if (observerService) {
+    observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
   }
 }
 

content/media/ogg/nsOggDecoder.h

@@ -519,9 +519,6 @@ private:
   // only.
   PRInt64 mDuration;
 
-  // True if we are registered with the observer service for shutdown.
-  PRPackedBool mNotifyOnShutdown;
-
   // True if the media resource is seekable (server supports byte range
   // requests).
   PRPackedBool mSeekable;

content/media/wave/nsWaveDecoder.cpp

@@ -1151,7 +1151,6 @@ nsWaveDecoder::nsWaveDecoder()
     mCurrentTime(0.0),
     mEndedDuration(std::numeric_limits<float>::quiet_NaN()),
     mEnded(PR_FALSE),
-    mNotifyOnShutdown(PR_FALSE),
     mSeekable(PR_TRUE),
     mResourceLoaded(PR_FALSE),
     mMetadataLoadedReported(PR_FALSE),
@@ -1165,6 +1164,21 @@ nsWaveDecoder::~nsWaveDecoder()
   MOZ_COUNT_DTOR(nsWaveDecoder);
 }
 
+PRBool
+nsWaveDecoder::Init(nsHTMLMediaElement* aElement)
+{
+  nsMediaDecoder::Init(aElement);
+
+  RegisterShutdownObserver();
+
+  mPlaybackStateMachine = new nsWaveStateMachine(this,
+    TimeDuration::FromMilliseconds(BUFFERING_TIMEOUT),
+    mInitialVolume);
+  NS_ENSURE_TRUE(mPlaybackStateMachine, PR_FALSE);
+
+  return PR_TRUE;
+}
+
 void
 nsWaveDecoder::GetCurrentURI(nsIURI** aURI)
 {
@@ -1270,11 +1284,6 @@ nsWaveDecoder::Stop()
 nsresult
 nsWaveDecoder::Load(nsIURI* aURI, nsIChannel* aChannel, nsIStreamListener** aStreamListener)
 {
-  // Reset progress member variables
-  mResourceLoaded = PR_FALSE;
-  mResourceLoadedReported = PR_FALSE;
-  mMetadataLoadedReported = PR_FALSE;
-
   if (aStreamListener) {
     *aStreamListener = nsnull;
   }
@@ -1290,15 +1299,6 @@ nsWaveDecoder::Load(nsIURI* aURI, nsIChannel* aChannel, nsIStreamListener** aStr
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  RegisterShutdownObserver();
-
-  mPlaybackStateMachine = new nsWaveStateMachine(this,
-    TimeDuration::FromMilliseconds(BUFFERING_TIMEOUT),
-    mInitialVolume);
-  NS_ENSURE_TRUE(mPlaybackStateMachine, NS_ERROR_OUT_OF_MEMORY);
-
-  // Open the stream *after* setting mPlaybackStateMachine, to ensure
-  // that callbacks (e.g. setting stream size) will actually work
   nsresult rv = nsMediaStream::Open(this, mURI, aChannel, getter_Transfers(mStream),
                                     aStreamListener);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -1512,30 +1512,24 @@ nsWaveDecoder::SeekingStopped()
 void
 nsWaveDecoder::RegisterShutdownObserver()
 {
-  if (!mNotifyOnShutdown) {
-    nsCOMPtr<nsIObserverService> observerService =
-      do_GetService("@mozilla.org/observer-service;1");
-    if (observerService) {
-      mNotifyOnShutdown =
-        NS_SUCCEEDED(observerService->AddObserver(this,
-                                                  NS_XPCOM_SHUTDOWN_OBSERVER_ID,
-                                                  PR_FALSE));
-    } else {
-      NS_WARNING("Could not get an observer service. Audio playback may not shutdown cleanly.");
-    }
+  nsCOMPtr<nsIObserverService> observerService =
+    do_GetService("@mozilla.org/observer-service;1");
+  if (observerService) {
+    observerService->AddObserver(this,
+                                 NS_XPCOM_SHUTDOWN_OBSERVER_ID,
+                                 PR_FALSE);
+  } else {
+    NS_WARNING("Could not get an observer service. Audio playback may not shutdown cleanly.");
   }
 }
 
 void
 nsWaveDecoder::UnregisterShutdownObserver()
 {
-  if (mNotifyOnShutdown) {
-    nsCOMPtr<nsIObserverService> observerService =
-      do_GetService("@mozilla.org/observer-service;1");
-    if (observerService) {
-      mNotifyOnShutdown = PR_FALSE;
-      observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
-    }
+  nsCOMPtr<nsIObserverService> observerService =
+    do_GetService("@mozilla.org/observer-service;1");
+  if (observerService) {
+    observerService->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
   }
 }
 

content/media/wave/nsWaveDecoder.h

@@ -146,6 +146,8 @@ class nsWaveDecoder : public nsMediaDecoder
   nsWaveDecoder();
   ~nsWaveDecoder();
 
+  virtual PRBool Init(nsHTMLMediaElement* aElement);
+
   virtual void GetCurrentURI(nsIURI** aURI);
   virtual already_AddRefed<nsIPrincipal> GetCurrentPrincipal();
 
@@ -274,9 +276,6 @@ private:
   float mEndedDuration;
   PRPackedBool mEnded;
 
-  // True if we have registered a shutdown observer.
-  PRPackedBool mNotifyOnShutdown;
-
   // True if the media resource is seekable.
   PRPackedBool mSeekable;