bug 795305 - http channel can drop callback references sooner on asyncopen failures r=jduell
authorPatrick McManus <mcmanus@ducksong.com>
Tue, 02 Oct 2012 20:18:34 -0400
changeset 109075 5a159a90e13f9769b805d916951242d5eef7fa02
parent 109074 7e0bf7bccc187467808f0ac4d324f4e374d8ccda
child 109076 ad81145cad9d29ead4f8d2b16333f162ddc0b457
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersjduell
bugs795305
milestone18.0a1
bug 795305 - http channel can drop callback references sooner on asyncopen failures r=jduell
netwerk/protocol/http/HttpBaseChannel.cpp
netwerk/protocol/http/HttpBaseChannel.h
netwerk/protocol/http/nsHttpChannel.cpp
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -1416,33 +1416,40 @@ HttpBaseChannel::SetNewListener(nsIStrea
   return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // HttpBaseChannel helpers
 //-----------------------------------------------------------------------------
 
 void
+HttpBaseChannel::ReleaseListeners()
+{
+  mListener = nullptr;
+  mListenerContext = nullptr;
+  mCallbacks = nullptr;
+  mProgressSink = nullptr;
+}
+
+void
 HttpBaseChannel::DoNotifyListener()
 {
   // Make sure mIsPending is set to false. At this moment we are done from
   // the point of view of our consumer and we have to report our self
   // as not-pending.
   if (mListener) {
     mListener->OnStartRequest(this, mListenerContext);
     mIsPending = false;
     mListener->OnStopRequest(this, mListenerContext, mStatus);
-    mListener = 0;
-    mListenerContext = 0;
   } else {
     mIsPending = false;
   }
-  // We have to make sure to drop the reference to the callbacks too
-  mCallbacks = nullptr;
-  mProgressSink = nullptr;
+  // We have to make sure to drop the references to listeners and callbacks
+  // no longer  needed
+  ReleaseListeners();
 
   DoNotifyListenerCleanup();
 }
 
 void
 HttpBaseChannel::AddCookiesToRequest()
 {
   if (mLoadFlags & LOAD_ANONYMOUS) {
--- a/netwerk/protocol/http/HttpBaseChannel.h
+++ b/netwerk/protocol/http/HttpBaseChannel.h
@@ -188,16 +188,19 @@ public:
 public: /* Necko internal use only... */
 
 protected:
 
   // Handle notifying listener, removing from loadgroup if request failed.
   void     DoNotifyListener();
   virtual void DoNotifyListenerCleanup() = 0;
 
+  // drop reference to listener, its callbacks, and the progress sink
+  void ReleaseListeners();
+
   nsresult ApplyContentConversions();
 
   void AddCookiesToRequest();
   virtual nsresult SetupReplacementChannel(nsIURI *,
                                            nsIChannel *,
                                            bool preserveMethod);
 
   // Helper function to simplify getting notification callbacks.
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -1644,23 +1644,17 @@ nsHttpChannel::OpenRedirectChannel(nsres
     if (NS_FAILED(rv)) {
         return rv;
     }
 
     mStatus = NS_BINDING_REDIRECTED;
 
     notifier.RedirectSucceeded();
 
-    // disconnect from the old listeners...
-    mListener = nullptr;
-    mListenerContext = nullptr;
-
-    // ...and the old callbacks
-    mCallbacks = nullptr;
-    mProgressSink = nullptr;
+    ReleaseListeners();
 
     return NS_OK;
 }
 
 nsresult
 nsHttpChannel::AsyncDoReplaceWithProxy(nsIProxyInfo* pi)
 {
     LOG(("nsHttpChannel::AsyncDoReplaceWithProxy [this=%p pi=%p]", this, pi));
@@ -1711,23 +1705,17 @@ nsHttpChannel::ContinueDoReplaceWithProx
     rv = mRedirectChannel->AsyncOpen(mListener, mListenerContext);
     if (NS_FAILED(rv))
         return rv;
 
     mStatus = NS_BINDING_REDIRECTED;
 
     notifier.RedirectSucceeded();
 
-    // disconnect from the old listeners...
-    mListener = nullptr;
-    mListenerContext = nullptr;
-
-    // ...and the old callbacks
-    mCallbacks = nullptr;
-    mProgressSink = nullptr;
+    ReleaseListeners();
 
     return rv;
 }
 
 nsresult
 nsHttpChannel::ResolveProxy()
 {
     LOG(("nsHttpChannel::ResolveProxy [this=%p]\n", this));
@@ -2279,23 +2267,17 @@ nsHttpChannel::ContinueProcessFallback(n
     if (NS_FAILED(rv))
         return rv;
 
     // close down this channel
     Cancel(NS_BINDING_REDIRECTED);
 
     notifier.RedirectSucceeded();
 
-    // disconnect from our listener
-    mListener = 0;
-    mListenerContext = 0;
-
-    // and from our callbacks
-    mCallbacks = nullptr;
-    mProgressSink = nullptr;
+    ReleaseListeners();
 
     mFallingBack = true;
 
     return NS_OK;
 }
 
 // Determines if a request is a byte range request for a subrange,
 // i.e. is a byte range request, but not a 0- byte range request.
@@ -4125,23 +4107,18 @@ nsHttpChannel::ContinueProcessRedirectio
     if (NS_FAILED(rv))
         return rv;
 
     // close down this channel
     Cancel(NS_BINDING_REDIRECTED);
     
     notifier.RedirectSucceeded();
 
-    // disconnect from our listener
-    mListener = 0;
-    mListenerContext = 0;
-
-    // and from our callbacks
-    mCallbacks = nullptr;
-    mProgressSink = nullptr;
+    ReleaseListeners();
+
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel <auth>
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP nsHttpChannel::OnAuthAvailable()
@@ -4314,18 +4291,20 @@ nsHttpChannel::AsyncOpen(nsIStreamListen
 
     NS_ENSURE_ARG_POINTER(listener);
     NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
     NS_ENSURE_TRUE(!mWasOpened, NS_ERROR_ALREADY_OPENED);
 
     nsresult rv;
 
     rv = NS_CheckPortSafety(mURI);
-    if (NS_FAILED(rv))
+    if (NS_FAILED(rv)) {
+        ReleaseListeners();
         return rv;
+    }
 
     // Remember the cookie header that was set, if any
     const char *cookieHeader = mRequestHead.PeekHeader(nsHttp::Cookie);
     if (cookieHeader) {
         mUserSetCookieHeader = cookieHeader;
     }
 
     AddCookiesToRequest();
@@ -4350,17 +4329,21 @@ nsHttpChannel::AsyncOpen(nsIStreamListen
     if (mTimingEnabled)
         mAsyncOpenTime = mozilla::TimeStamp::Now();
 
     // the only time we would already know the proxy information at this
     // point would be if we were proxying a non-http protocol like ftp
     if (!mProxyInfo && NS_SUCCEEDED(ResolveProxy()))
         return NS_OK;
 
-    return BeginConnect();
+    rv = BeginConnect();
+    if (NS_FAILED(rv))
+        ReleaseListeners();
+
+    return rv;
 }
 
 nsresult
 nsHttpChannel::BeginConnect()
 {
     LOG(("nsHttpChannel::BeginConnect [this=%p]\n", this));
     nsresult rv;
 
@@ -5002,18 +4985,16 @@ nsHttpChannel::OnStopRequest(nsIRequest 
     if (mCacheEntry && (mCacheAccess & nsICache::ACCESS_WRITE) &&
         mRequestTimeInitialized){
         FinalizeCacheEntry();
     }
     
     if (mListener) {
         LOG(("  calling OnStopRequest\n"));
         mListener->OnStopRequest(this, mListenerContext, status);
-        mListener = 0;
-        mListenerContext = 0;
     }
 
     if (mCacheEntry) {
         bool asFile = false;
         if (mInitedCacheEntry && !mCachedContentIsPartial &&
             (NS_SUCCEEDED(mStatus) || contentComplete) &&
             (mCacheAccess & nsICache::ACCESS_WRITE) &&
             NS_SUCCEEDED(GetCacheAsFile(&asFile)) && asFile) {
@@ -5036,18 +5017,17 @@ nsHttpChannel::OnStopRequest(nsIRequest 
         CloseOfflineCacheEntry();
 
     if (mLoadGroup)
         mLoadGroup->RemoveRequest(this, nullptr, status);
 
     // We don't need this info anymore
     CleanRedirectCacheChainIfNecessary();
 
-    mCallbacks = nullptr;
-    mProgressSink = nullptr;
+    ReleaseListeners();
     
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsIStreamListener
 //-----------------------------------------------------------------------------