Bug 788275 - Part 0: Make nsCORSListenerProxy's constructor not cause the object to be destroyed before it is constructed; r=bzbarsky
authorEhsan Akhgari <ehsan@mozilla.com>
Tue, 18 Sep 2012 22:16:23 -0400
changeset 107562 64c78f239c471565e60c6e29502e15e6c3eda382
parent 107561 a04dc1a6fe29474adf15829b98b15afdaf1f36fe
child 107563 1c7689d7feb60b761a0dddcca71638e695b63329
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersbzbarsky
bugs788275
milestone18.0a1
Bug 788275 - Part 0: Make nsCORSListenerProxy's constructor not cause the object to be destroyed before it is constructed; r=bzbarsky That is always a nice property to have!
content/base/src/nsCrossSiteListenerProxy.cpp
content/base/src/nsCrossSiteListenerProxy.h
content/base/src/nsEventSource.cpp
content/base/src/nsScriptLoader.cpp
content/base/src/nsSyncLoadService.cpp
content/base/src/nsXMLHttpRequest.cpp
content/html/content/src/nsHTMLMediaElement.cpp
content/media/MediaResource.cpp
content/xslt/src/xslt/txMozillaStylesheetCompiler.cpp
image/src/imgLoader.cpp
layout/style/Loader.cpp
layout/style/nsFontFaceLoader.cpp
--- a/content/base/src/nsCrossSiteListenerProxy.cpp
+++ b/content/base/src/nsCrossSiteListenerProxy.cpp
@@ -341,91 +341,59 @@ void
 nsCORSListenerProxy::Shutdown()
 {
   delete sPreflightCache;
   sPreflightCache = nullptr;
 }
 
 nsCORSListenerProxy::nsCORSListenerProxy(nsIStreamListener* aOuter,
                                          nsIPrincipal* aRequestingPrincipal,
-                                         nsIChannel* aChannel,
-                                         bool aWithCredentials,
-                                         nsresult* aResult)
+                                         bool aWithCredentials)
   : mOuterListener(aOuter),
     mRequestingPrincipal(aRequestingPrincipal),
     mWithCredentials(aWithCredentials && !gDisableCORSPrivateData),
     mRequestApproved(false),
     mHasBeenCrossSite(false),
     mIsPreflight(false)
 {
-  aChannel->GetNotificationCallbacks(getter_AddRefs(mOuterNotificationCallbacks));
-  aChannel->SetNotificationCallbacks(this);
-
-  *aResult = UpdateChannel(aChannel);
-  if (NS_FAILED(*aResult)) {
-    mOuterListener = nullptr;
-    mRequestingPrincipal = nullptr;
-    mOuterNotificationCallbacks = nullptr;
-  }
 }
 
 nsCORSListenerProxy::nsCORSListenerProxy(nsIStreamListener* aOuter,
                                          nsIPrincipal* aRequestingPrincipal,
-                                         nsIChannel* aChannel,
-                                         bool aWithCredentials,
-                                         bool aAllowDataURI,
-                                         nsresult* aResult)
-  : mOuterListener(aOuter),
-    mRequestingPrincipal(aRequestingPrincipal),
-    mWithCredentials(aWithCredentials && !gDisableCORSPrivateData),
-    mRequestApproved(false),
-    mHasBeenCrossSite(false),
-    mIsPreflight(false)
-{
-  aChannel->GetNotificationCallbacks(getter_AddRefs(mOuterNotificationCallbacks));
-  aChannel->SetNotificationCallbacks(this);
-
-  *aResult = UpdateChannel(aChannel, aAllowDataURI);
-  if (NS_FAILED(*aResult)) {
-    mOuterListener = nullptr;
-    mRequestingPrincipal = nullptr;
-    mOuterNotificationCallbacks = nullptr;
-  }
-}
-
-nsCORSListenerProxy::nsCORSListenerProxy(nsIStreamListener* aOuter,
-                                         nsIPrincipal* aRequestingPrincipal,
-                                         nsIChannel* aChannel,
                                          bool aWithCredentials,
                                          const nsCString& aPreflightMethod,
-                                         const nsTArray<nsCString>& aPreflightHeaders,
-                                         nsresult* aResult)
+                                         const nsTArray<nsCString>& aPreflightHeaders)
   : mOuterListener(aOuter),
     mRequestingPrincipal(aRequestingPrincipal),
     mWithCredentials(aWithCredentials && !gDisableCORSPrivateData),
     mRequestApproved(false),
     mHasBeenCrossSite(false),
     mIsPreflight(true),
     mPreflightMethod(aPreflightMethod),
     mPreflightHeaders(aPreflightHeaders)
 {
   for (uint32_t i = 0; i < mPreflightHeaders.Length(); ++i) {
     ToLowerCase(mPreflightHeaders[i]);
   }
   mPreflightHeaders.Sort();
+}
 
+nsresult
+nsCORSListenerProxy::Init(nsIChannel* aChannel, bool aAllowDataURI)
+{
   aChannel->GetNotificationCallbacks(getter_AddRefs(mOuterNotificationCallbacks));
   aChannel->SetNotificationCallbacks(this);
 
-  *aResult = UpdateChannel(aChannel);
-  if (NS_FAILED(*aResult)) {
+  nsresult rv = UpdateChannel(aChannel, aAllowDataURI);
+  if (NS_FAILED(rv)) {
     mOuterListener = nullptr;
     mRequestingPrincipal = nullptr;
     mOuterNotificationCallbacks = nullptr;
   }
+  return rv;
 }
 
 NS_IMETHODIMP
 nsCORSListenerProxy::OnStartRequest(nsIRequest* aRequest,
                                     nsISupports* aContext)
 {
   mRequestApproved = NS_SUCCEEDED(CheckRequestApproved(aRequest));
   if (!mRequestApproved) {
@@ -1086,22 +1054,23 @@ NS_StartCORSPreflight(nsIChannel* aReque
   NS_ENSURE_SUCCESS(rv, rv);
   
   // Set up listener which will start the original channel
   nsCOMPtr<nsIStreamListener> preflightListener =
     new nsCORSPreflightListener(aRequestChannel, aListener, nullptr, aPrincipal,
                                 method, aWithCredentials);
   NS_ENSURE_TRUE(preflightListener, NS_ERROR_OUT_OF_MEMORY);
 
-  preflightListener =
+  nsRefPtr<nsCORSListenerProxy> corsListener =
     new nsCORSListenerProxy(preflightListener, aPrincipal,
-                            preflightChannel, aWithCredentials,
-                            method, aUnsafeHeaders, &rv);
-  NS_ENSURE_TRUE(preflightListener, NS_ERROR_OUT_OF_MEMORY);
+                            aWithCredentials, method,
+                            aUnsafeHeaders);
+  rv = corsListener->Init(preflightChannel);
   NS_ENSURE_SUCCESS(rv, rv);
+  preflightListener = corsListener;
 
   // Start preflight
   rv = preflightChannel->AsyncOpen(preflightListener, nullptr);
   NS_ENSURE_SUCCESS(rv, rv);
   
   // Return newly created preflight channel
   preflightChannel.forget(aPreflightChannel);
 
--- a/content/base/src/nsCrossSiteListenerProxy.h
+++ b/content/base/src/nsCrossSiteListenerProxy.h
@@ -35,45 +35,37 @@ NS_StartCORSPreflight(nsIChannel* aReque
 class nsCORSListenerProxy MOZ_FINAL : public nsIStreamListener,
                                       public nsIInterfaceRequestor,
                                       public nsIChannelEventSink,
                                       public nsIAsyncVerifyRedirectCallback
 {
 public:
   nsCORSListenerProxy(nsIStreamListener* aOuter,
                       nsIPrincipal* aRequestingPrincipal,
-                      nsIChannel* aChannel,
-                      bool aWithCredentials,
-                      nsresult* aResult);
+                      bool aWithCredentials);
   nsCORSListenerProxy(nsIStreamListener* aOuter,
                       nsIPrincipal* aRequestingPrincipal,
-                      nsIChannel* aChannel,
-                      bool aWithCredentials,
-                      bool aAllowDataURI,
-                      nsresult* aResult);
-  nsCORSListenerProxy(nsIStreamListener* aOuter,
-                      nsIPrincipal* aRequestingPrincipal,
-                      nsIChannel* aChannel,
                       bool aWithCredentials,
                       const nsCString& aPreflightMethod,
-                      const nsTArray<nsCString>& aPreflightHeaders,
-                      nsresult* aResult);
+                      const nsTArray<nsCString>& aPreflightHeaders);
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIASYNCVERIFYREDIRECTCALLBACK
 
   // Must be called at startup.
   static void Startup();
 
   static void Shutdown();
 
+  nsresult Init(nsIChannel* aChannel, bool aAllowDataURI = false);
+
 private:
   nsresult UpdateChannel(nsIChannel* aChannel, bool aAllowDataURI = false);
   nsresult CheckRequestApproved(nsIRequest* aRequest);
 
   nsCOMPtr<nsIStreamListener> mOuterListener;
   nsCOMPtr<nsIPrincipal> mRequestingPrincipal;
   nsCOMPtr<nsIInterfaceRequestor> mOuterNotificationCallbacks;
   bool mWithCredentials;
--- a/content/base/src/nsEventSource.cpp
+++ b/content/base/src/nsEventSource.cpp
@@ -898,19 +898,19 @@ nsEventSource::InitChannelAndRequestEven
 
   nsCOMPtr<nsIInterfaceRequestor> notificationCallbacks;
   mHttpChannel->GetNotificationCallbacks(getter_AddRefs(notificationCallbacks));
   if (notificationCallbacks != this) {
     mNotificationCallbacks = notificationCallbacks;
     mHttpChannel->SetNotificationCallbacks(this);
   }
 
-  nsCOMPtr<nsIStreamListener> listener =
-    new nsCORSListenerProxy(this, mPrincipal, mHttpChannel,
-                            mWithCredentials, &rv);
+  nsRefPtr<nsCORSListenerProxy> listener =
+    new nsCORSListenerProxy(this, mPrincipal, mWithCredentials);
+  rv = listener->Init(mHttpChannel);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Start reading from the channel
   rv = mHttpChannel->AsyncOpen(listener, nullptr);
   if (NS_SUCCEEDED(rv)) {
     mWaitingForOnStopRequest = true;
   }
   return rv;
--- a/content/base/src/nsScriptLoader.cpp
+++ b/content/base/src/nsScriptLoader.cpp
@@ -315,20 +315,22 @@ nsScriptLoader::StartLoad(nsScriptLoadRe
   nsCOMPtr<nsIStreamLoader> loader;
   rv = NS_NewStreamLoader(getter_AddRefs(loader), this);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIStreamListener> listener = loader.get();
 
   if (aRequest->mCORSMode != CORS_NONE) {
     bool withCredentials = (aRequest->mCORSMode == CORS_USE_CREDENTIALS);
-    listener =
-      new nsCORSListenerProxy(listener, mDocument->NodePrincipal(), channel,
-                              withCredentials, &rv);
+    nsRefPtr<nsCORSListenerProxy> corsListener =
+      new nsCORSListenerProxy(listener, mDocument->NodePrincipal(),
+                              withCredentials);
+    rv = corsListener->Init(channel);
     NS_ENSURE_SUCCESS(rv, rv);
+    listener = corsListener;
   }
 
   rv = channel->AsyncOpen(listener, aRequest);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
--- a/content/base/src/nsSyncLoadService.cpp
+++ b/content/base/src/nsSyncLoadService.cpp
@@ -175,19 +175,21 @@ nsSyncLoader::LoadDocument(nsIChannel* a
 
     if (aForceToXML) {
         nsCOMPtr<nsIStreamListener> forceListener =
             new nsForceXMLListener(listener);
         listener.swap(forceListener);
     }
 
     if (aLoaderPrincipal) {
-        listener = new nsCORSListenerProxy(listener, aLoaderPrincipal,
-                                           mChannel, false, &rv);
+        nsRefPtr<nsCORSListenerProxy> corsListener =
+          new nsCORSListenerProxy(listener, aLoaderPrincipal, false);
+        rv = corsListener->Init(mChannel);
         NS_ENSURE_SUCCESS(rv, rv);
+        listener = corsListener;
     }
 
     if (aChannelIsSync) {
         rv = PushSyncStream(listener);
     }
     else {
         rv = PushAsyncStream(listener);
     }
--- a/content/base/src/nsXMLHttpRequest.cpp
+++ b/content/base/src/nsXMLHttpRequest.cpp
@@ -2923,19 +2923,21 @@ nsXMLHttpRequest::Send(nsIVariant* aVari
 
   // Blocking gets are common enough out of XHR that we should mark
   // the channel slow by default for pipeline purposes
   AddLoadFlags(mChannel, nsIRequest::INHIBIT_PIPELINE);
 
   if (!IsSystemXHR()) {
     // Always create a nsCORSListenerProxy here even if it's
     // a same-origin request right now, since it could be redirected.
-    listener = new nsCORSListenerProxy(listener, mPrincipal, mChannel,
-                                       withCredentials, true, &rv);
+    nsRefPtr<nsCORSListenerProxy> corsListener =
+      new nsCORSListenerProxy(listener, mPrincipal, withCredentials);
+    rv = corsListener->Init(mChannel, true);
     NS_ENSURE_SUCCESS(rv, rv);
+    listener = corsListener;
   }
   else {
     // Because of bug 682305, we can't let listener be the XHR object itself
     // because JS wouldn't be able to use it. So if we haven't otherwise
     // created a listener around 'this', do so now.
 
     listener = new nsStreamListenerWrapper(listener);
   }
--- a/content/html/content/src/nsHTMLMediaElement.cpp
+++ b/content/html/content/src/nsHTMLMediaElement.cpp
@@ -1092,30 +1092,31 @@ nsresult nsHTMLMediaElement::LoadResourc
   // notification before OnStartRequest fires.  Necko guarantees that
   // OnStartRequest will eventually fire if we don't shut down first.
   nsRefPtr<MediaLoadListener> loadListener = new MediaLoadListener(this);
 
   channel->SetNotificationCallbacks(loadListener);
 
   nsCOMPtr<nsIStreamListener> listener;
   if (ShouldCheckAllowOrigin()) {
-    listener =
+    nsRefPtr<nsCORSListenerProxy> corsListener =
       new nsCORSListenerProxy(loadListener,
                               NodePrincipal(),
-                              channel,
-                              GetCORSMode() == CORS_USE_CREDENTIALS,
-                              &rv);
+                              GetCORSMode() == CORS_USE_CREDENTIALS);
+    rv = corsListener->Init(channel);
+    NS_ENSURE_SUCCESS(rv, rv);
+    listener = corsListener;
   } else {
     rv = nsContentUtils::GetSecurityManager()->
            CheckLoadURIWithPrincipal(NodePrincipal(),
                                      mLoadingSrc,
                                      nsIScriptSecurityManager::STANDARD);
     listener = loadListener;
+    NS_ENSURE_SUCCESS(rv, rv);
   }
-  NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIHttpChannel> hc = do_QueryInterface(channel);
   if (hc) {
     // Use a byte range request from the start of the resource.
     // This enables us to detect if the stream supports byte range
     // requests, and therefore seeking, early.
     hc->SetRequestHeader(NS_LITERAL_CSTRING("Range"),
                          NS_LITERAL_CSTRING("bytes=0-"),
--- a/content/media/MediaResource.cpp
+++ b/content/media/MediaResource.cpp
@@ -441,23 +441,21 @@ nsresult ChannelMediaResource::OpenChann
 
     nsCOMPtr<nsIStreamListener> listener = mListener.get();
 
     // Ensure that if we're loading cross domain, that the server is sending
     // an authorizing Access-Control header.
     nsHTMLMediaElement* element = mDecoder->GetMediaElement();
     NS_ENSURE_TRUE(element, NS_ERROR_FAILURE);
     if (element->ShouldCheckAllowOrigin()) {
-      nsresult rv;
       nsCORSListenerProxy* crossSiteListener =
         new nsCORSListenerProxy(mListener,
                                 element->NodePrincipal(),
-                                mChannel,
-                                false,
-                                &rv);
+                                false);
+      nsresult rv = crossSiteListener->Init(mChannel);
       listener = crossSiteListener;
       NS_ENSURE_TRUE(crossSiteListener, NS_ERROR_OUT_OF_MEMORY);
       NS_ENSURE_SUCCESS(rv, rv);
     } else {
       nsresult rv = nsContentUtils::GetSecurityManager()->
         CheckLoadURIWithPrincipal(element->NodePrincipal(),
                                   mURI,
                                   nsIScriptSecurityManager::STANDARD);
--- a/content/xslt/src/xslt/txMozillaStylesheetCompiler.cpp
+++ b/content/xslt/src/xslt/txMozillaStylesheetCompiler.cpp
@@ -480,20 +480,19 @@ txCompileObserver::startLoad(nsIURI* aUr
 
     channel->SetNotificationCallbacks(sink);
 
     parser->SetCommand(kLoadAsData);
     parser->SetContentSink(sink);
     parser->Parse(aUri);
 
     // Always install in case of redirects
-    nsCOMPtr<nsIStreamListener> listener =
-        new nsCORSListenerProxy(sink, aReferrerPrincipal, channel,
-                                false, &rv);
-    NS_ENSURE_TRUE(listener, NS_ERROR_OUT_OF_MEMORY);
+    nsRefPtr<nsCORSListenerProxy> listener =
+        new nsCORSListenerProxy(sink, aReferrerPrincipal, false);
+    rv = listener->Init(channel);
     NS_ENSURE_SUCCESS(rv, rv);
 
     return channel->AsyncOpen(listener, parser);
 }
 
 nsresult
 TX_LoadSheet(nsIURI* aUri, txMozillaXSLTProcessor* aProcessor,
              nsILoadGroup* aLoadGroup, nsIPrincipal* aCallerPrincipal)
--- a/image/src/imgLoader.cpp
+++ b/image/src/imgLoader.cpp
@@ -1245,18 +1245,19 @@ bool imgLoader::ValidateRequestWithNewCh
 
     // We must set the notification callbacks before setting up the
     // CORS listener, because that's also interested inthe
     // notification callbacks.
     newChannel->SetNotificationCallbacks(hvc);
 
     if (aCORSMode != imgIRequest::CORS_NONE) {
       bool withCredentials = aCORSMode == imgIRequest::CORS_USE_CREDENTIALS;
-      nsCOMPtr<nsIStreamListener> corsproxy =
-        new nsCORSListenerProxy(hvc, aLoadingPrincipal, newChannel, withCredentials, &rv);
+      nsRefPtr<nsCORSListenerProxy> corsproxy =
+        new nsCORSListenerProxy(hvc, aLoadingPrincipal, withCredentials);
+      rv = corsproxy->Init(newChannel);
       if (NS_FAILED(rv)) {
         return false;
       }
 
       listener = corsproxy;
     }
 
     request->mValidator = hvc;
@@ -1699,19 +1700,19 @@ NS_IMETHODIMP imgLoader::LoadImage(nsIUR
     // request.
     nsCOMPtr<nsIStreamListener> listener = pl;
     if (corsmode != imgIRequest::CORS_NONE) {
       PR_LOG(gImgLog, PR_LOG_DEBUG,
              ("[this=%p] imgLoader::LoadImage -- Setting up a CORS load",
               this));
       bool withCredentials = corsmode == imgIRequest::CORS_USE_CREDENTIALS;
 
-      nsCOMPtr<nsIStreamListener> corsproxy =
-        new nsCORSListenerProxy(pl, aLoadingPrincipal, newChannel,
-                                withCredentials, &rv);
+      nsRefPtr<nsCORSListenerProxy> corsproxy =
+        new nsCORSListenerProxy(pl, aLoadingPrincipal, withCredentials);
+      rv = corsproxy->Init(newChannel);
       if (NS_FAILED(rv)) {
         PR_LOG(gImgLog, PR_LOG_DEBUG,
                ("[this=%p] imgLoader::LoadImage -- nsCORSListenerProxy "
                 "creation failed: 0x%x\n", this, rv));
         request->CancelAndAbort(rv);
         return NS_ERROR_FAILURE;
       }
 
--- a/layout/style/Loader.cpp
+++ b/layout/style/Loader.cpp
@@ -1568,27 +1568,29 @@ Loader::LoadSheet(SheetLoadData* aLoadDa
     return rv;
   }
 
   nsCOMPtr<nsIStreamListener> channelListener;
   CORSMode ourCORSMode = aLoadData->mSheet->GetCORSMode();
   if (ourCORSMode != CORS_NONE) {
     bool withCredentials = (ourCORSMode == CORS_USE_CREDENTIALS);
     LOG(("  Doing CORS-enabled load; credentials %d", withCredentials));
-    channelListener =
+    nsRefPtr<nsCORSListenerProxy> corsListener =
       new nsCORSListenerProxy(streamLoader, aLoadData->mLoaderPrincipal,
-			      channel, withCredentials, &rv);
+			      withCredentials);
+    rv = corsListener->Init(channel);
     if (NS_FAILED(rv)) {
 #ifdef DEBUG
       mSyncCallback = false;
 #endif
       LOG_ERROR(("  Initial CORS check failed"));
       SheetComplete(aLoadData, rv);
       return rv;
     }
+    channelListener = corsListener;
   } else {
     channelListener = streamLoader;
   }
 
   rv = channel->AsyncOpen(channelListener, nullptr);
 
 #ifdef DEBUG
   mSyncCallback = false;
--- a/layout/style/nsFontFaceLoader.cpp
+++ b/layout/style/nsFontFaceLoader.cpp
@@ -368,23 +368,23 @@ nsUserFontSet::StartLoad(gfxProxyFontEnt
   bool inherits = false;
   rv = NS_URIChainHasFlags(aFontFaceSrc->mURI,
                            nsIProtocolHandler::URI_INHERITS_SECURITY_CONTEXT,
                            &inherits);
   if (NS_SUCCEEDED(rv) && inherits) {
     // allow data, javascript, etc URI's
     rv = channel->AsyncOpen(streamLoader, nullptr);
   } else {
-    nsCOMPtr<nsIStreamListener> listener =
-      new nsCORSListenerProxy(streamLoader, principal, channel,
-                              false, &rv);
+    nsRefPtr<nsCORSListenerProxy> listener =
+      new nsCORSListenerProxy(streamLoader, principal,
+                              false);
+    rv = listener->Init(channel);
     if (NS_FAILED(rv)) {
       fontLoader->DropChannel();  // explicitly need to break ref cycle
     }
-    NS_ENSURE_TRUE(listener, NS_ERROR_OUT_OF_MEMORY);
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = channel->AsyncOpen(listener, nullptr);
   }
 
   if (NS_SUCCEEDED(rv)) {
     mLoaders.PutEntry(fontLoader);
     fontLoader->StartedLoading(streamLoader);