Bug 1166133 - If proxy is available and an error happens, onStart/onStopRequest can be called twice. r=mcmanus, a=sledru
authorDragana Damjanovic <dd.mozilla@gmail.com>
Tue, 19 May 2015 11:28:00 -0400
changeset 274889 63fb6ce91b3c48fb007889e2ca8b98af91251cac
parent 274888 dce71770604036433ef75b373935d89658984baa
child 274890 63b95ebfb9793aa8237210fd384f19bc367c2199
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus, sledru
bugs1166133
milestone40.0a2
Bug 1166133 - If proxy is available and an error happens, onStart/onStopRequest can be called twice. r=mcmanus, a=sledru
netwerk/protocol/http/nsHttpChannel.cpp
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -4817,16 +4817,18 @@ nsHttpChannel::AsyncOpen(nsIStreamListen
 
     rv = BeginConnect();
     if (NS_FAILED(rv))
         ReleaseListeners();
 
     return rv;
 }
 
+// On error BeginConnect() should call AsyncAbort() before exiting until
+// ContineBeginConnect after that it should not call it.
 nsresult
 nsHttpChannel::BeginConnect()
 {
     LOG(("nsHttpChannel::BeginConnect [this=%p]\n", this));
     nsresult rv;
 
     // Construct connection info object
     nsAutoCString host;
@@ -4840,22 +4842,27 @@ nsHttpChannel::BeginConnect()
     if (NS_SUCCEEDED(rv))
         rv = mURI->GetAsciiHost(host);
     if (NS_SUCCEEDED(rv))
         rv = mURI->GetPort(&port);
     if (NS_SUCCEEDED(rv))
         mURI->GetUsername(mUsername);
     if (NS_SUCCEEDED(rv))
         rv = mURI->GetAsciiSpec(mSpec);
-    if (NS_FAILED(rv))
+    if (NS_FAILED(rv)) {
+        AsyncAbort(rv);
         return rv;
+    }
 
     // Reject the URL if it doesn't specify a host
-    if (host.IsEmpty())
-        return NS_ERROR_MALFORMED_URI;
+    if (host.IsEmpty()) {
+        rv = NS_ERROR_MALFORMED_URI;
+        AsyncAbort(rv);
+        return rv;
+    }
     LOG(("host=%s port=%d\n", host.get(), port));
     LOG(("uri=%s\n", mSpec.get()));
 
     nsCOMPtr<nsProxyInfo> proxyInfo;
     if (mProxyInfo)
         proxyInfo = do_QueryInterface(mProxyInfo);
 
     mRequestHead.SetHTTPS(isHttps);
@@ -4918,29 +4925,35 @@ nsHttpChannel::BeginConnect()
         mConnectionInfo->SetNetworkInterfaceId(mNetworkInterfaceId);
     }
 
     mAuthProvider =
         do_CreateInstance("@mozilla.org/network/http-channel-auth-provider;1",
                           &rv);
     if (NS_SUCCEEDED(rv))
         rv = mAuthProvider->Init(this);
-    if (NS_FAILED(rv))
+    if (NS_FAILED(rv)) {
+        AsyncAbort(rv);
         return rv;
+    }
 
     // check to see if authorization headers should be included
     mAuthProvider->AddAuthorizationHeaders();
 
     // notify "http-on-modify-request" observers
     CallOnModifyRequestObservers();
 
     // Check to see if we should redirect this channel elsewhere by
     // nsIHttpChannel.redirectTo API request
     if (mAPIRedirectToURI) {
-        return AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect);
+        rv = AsyncCall(&nsHttpChannel::HandleAsyncAPIRedirect);
+        if (NS_FAILED(rv)) {
+            AsyncAbort(rv);
+        }
+        return rv;
     }
     // Check to see if this principal exists on local blocklists.
     nsRefPtr<nsChannelClassifier> channelClassifier = new nsChannelClassifier();
     if (mLoadFlags & LOAD_CLASSIFY_URI) {
         nsCOMPtr<nsIURIClassifier> classifier = do_GetService(NS_URICLASSIFIERSERVICE_CONTRACTID);
         if (classifier) {
             bool tp = false;
             channelClassifier->ShouldEnableTrackingProtection(this, &tp);
@@ -5023,26 +5036,30 @@ nsHttpChannel::BeginConnect()
         // just the initial document resets the whole pool
         if (mLoadFlags & LOAD_INITIAL_DOCUMENT_URI) {
             gHttpHandler->ConnMgr()->ClearAltServiceMappings();
             gHttpHandler->ConnMgr()->DoShiftReloadConnectionCleanup(mConnectionInfo);
         }
         mCaps &= ~NS_HTTP_ALLOW_PIPELINING;
     }
     if (!(mLoadFlags & LOAD_CLASSIFY_URI)) {
+        // On error ContinueBeginConnect() will call AsyncAbort so do not do it
+        // here
         return ContinueBeginConnect();
     }
     // mLocalBlocklist is true only if tracking protection is enabled and the
     // URI is a tracking domain, it makes no guarantees about phishing or
     // malware, so if LOAD_CLASSIFY_URI is true we must call
     // nsChannelClassifier to catch phishing and malware URIs.
     bool callContinueBeginConnect = true;
     if (mCanceled || !mLocalBlocklist) {
        rv = ContinueBeginConnect();
        if (NS_FAILED(rv)) {
+           // On error ContinueBeginConnect() will call AsyncAbort so do not do
+           // it here
            return rv;
        }
        callContinueBeginConnect = false;
     }
     // nsChannelClassifier calls ContinueBeginConnect if it has not already
     // been called, after optionally cancelling the channel once we have a
     // remote verdict. We call a concrete class instead of an nsI* that might
     // be overridden.
@@ -5156,31 +5173,25 @@ nsHttpChannel::OnProxyAvailable(nsICance
 
     if (NS_SUCCEEDED(status))
         mProxyInfo = pi;
 
     if (!gHttpHandler->Active()) {
         LOG(("nsHttpChannel::OnProxyAvailable [this=%p] "
              "Handler no longer active.\n", this));
         rv = NS_ERROR_NOT_AVAILABLE;
+        AsyncAbort(rv);
     }
     else {
+        // On error BeginConnect() will call AsyncAbort.
         rv = BeginConnect();
     }
 
     if (NS_FAILED(rv)) {
         Cancel(rv);
-        // Calling OnStart/OnStop synchronously here would mean doing it before
-        // returning from AsyncOpen which is a contract violation. Do it async.
-        nsRefPtr<nsRunnableMethod<HttpBaseChannel> > event =
-            NS_NewRunnableMethod(this, &nsHttpChannel::DoNotifyListener);
-        rv = NS_DispatchToCurrentThread(event);
-        if (NS_FAILED(rv)) {
-            NS_WARNING("Failed To Dispatch DoNotifyListener");
-        }
     }
     return rv;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsIProxiedChannel
 //-----------------------------------------------------------------------------