Bug 1364812 - ensure OnStartRequest is called after nsHttpChannel::CallOnStartRequest. r=dragana. draft
authorShih-Chiang Chien <schien@mozilla.com>
Mon, 22 May 2017 16:26:22 +0800
changeset 582338 ffe386bc8bea6defb216ecf149a29deacdb3737a
parent 582183 74566d5345f4cab06c5683d4b620124104801e65
child 629733 78a7a2945568ae0b9e9c9f3d32da64f0a1cc9c12
push id60043
push userbmo:schien@mozilla.com
push dateMon, 22 May 2017 08:29:59 +0000
reviewersdragana
bugs1364812
milestone55.0a1
Bug 1364812 - ensure OnStartRequest is called after nsHttpChannel::CallOnStartRequest. r=dragana. 1. Use ScopeExit to ensure mListener->OnStartRequest() is invoked before exit CallOnStartRequest. 2. Add assertion to ensure OnStartRequest called before OnStopRequest. MozReview-Commit-ID: FgONlk5HPNz
netwerk/protocol/http/nsHttpChannel.cpp
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // HttpLog.h should generally be included first
 #include "HttpLog.h"
 
 #include <inttypes.h>
 
 #include "mozilla/dom/nsCSPContext.h"
+#include "mozilla/ScopeExit.h"
 #include "mozilla/SizePrintfMacros.h"
 #include "mozilla/Sprintf.h"
 
 #include "nsHttp.h"
 #include "nsHttpChannel.h"
 #include "nsHttpHandler.h"
 #include "nsIApplicationCacheService.h"
 #include "nsIApplicationCacheContainer.h"
@@ -1383,38 +1384,53 @@ nsHttpChannel::CallOnStartRequest()
     LOG(("nsHttpChannel::CallOnStartRequest [this=%p]", this));
 
     MOZ_RELEASE_ASSERT(!(mRequireCORSPreflight &&
                          mInterceptCache != INTERCEPTED) ||
                        mIsCorsPreflightDone,
                        "CORS preflight must have been finished by the time we "
                        "call OnStartRequest");
 
-    nsresult rv = EnsureMIMEOfScript(mURI, mResponseHead, mLoadInfo);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    rv = ProcessXCTO(mURI, mResponseHead, mLoadInfo);
-    NS_ENSURE_SUCCESS(rv, rv);
-
     if (mOnStartRequestCalled) {
         // This can only happen when a range request loading rest of the data
         // after interrupted concurrent cache read asynchronously failed, e.g.
         // the response range bytes are not as expected or this channel has
         // been externally canceled.
         //
         // It's legal to bypass CallOnStartRequest for that case since we've
         // already called OnStartRequest on our listener and also added all
         // content converters before.
         MOZ_ASSERT(mConcurrentCacheAccess);
         LOG(("CallOnStartRequest already invoked before"));
         return mStatus;
     }
 
     mTracingEnabled = false;
 
+    // Ensure mListener->OnStartRequest will be invoked before exiting
+    // this function.
+    auto onStartGuard = MakeScopeExit([&] {
+        LOG(("  calling mListener->OnStartRequest by ScopeExit [this=%p, "
+             "listener=%p]\n", this, mListener.get()));
+        MOZ_ASSERT(!mOnStartRequestCalled);
+
+        if (mListener) {
+            nsCOMPtr<nsIStreamListener> deleteProtector(mListener);
+            deleteProtector->OnStartRequest(this, mListenerContext);
+        }
+
+        mOnStartRequestCalled = true;
+    });
+
+    nsresult rv = EnsureMIMEOfScript(mURI, mResponseHead, mLoadInfo);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = ProcessXCTO(mURI, mResponseHead, mLoadInfo);
+    NS_ENSURE_SUCCESS(rv, rv);
+
     // Allow consumers to override our content type
     if (mLoadFlags & LOAD_CALL_CONTENT_SNIFFERS) {
         // NOTE: We can have both a txn pump and a cache pump when the cache
         // content is partial. In that case, we need to read from the cache,
         // because that's the one that has the initial contents. If that fails
         // then give the transaction pump a shot.
 
         nsIChannel* thisChannel = static_cast<nsIChannel*>(this);
@@ -1471,16 +1487,20 @@ nsHttpChannel::CallOnStartRequest()
           // Don't throw the entry away, we will need it later.
           LOG(("  entry too big"));
         } else {
           NS_ENSURE_SUCCESS(rv, rv);
         }
     }
 
     LOG(("  calling mListener->OnStartRequest [this=%p, listener=%p]\n", this, mListener.get()));
+
+    // About to call OnStartRequest, dismiss the guard object.
+    onStartGuard.release();
+
     if (mListener) {
         MOZ_ASSERT(!mOnStartRequestCalled,
                    "We should not call OsStartRequest twice");
         nsCOMPtr<nsIStreamListener> deleteProtector(mListener);
         rv = deleteProtector->OnStartRequest(this, mListenerContext);
         mOnStartRequestCalled = true;
         if (NS_FAILED(rv))
             return rv;
@@ -7349,16 +7369,18 @@ nsHttpChannel::OnStopRequest(nsIRequest 
     // Register entry to the Performance resource timing
     mozilla::dom::Performance* documentPerformance = GetPerformance();
     if (documentPerformance) {
         documentPerformance->AddEntry(this, this);
     }
 
     if (mListener) {
         LOG(("  calling OnStopRequest\n"));
+        MOZ_ASSERT(mOnStartRequestCalled,
+                   "OnStartRequest should be called before OnStopRequest");
         MOZ_ASSERT(!mOnStopRequestCalled,
                    "We should not call OnStopRequest twice");
         mListener->OnStopRequest(this, mListenerContext, status);
         mOnStopRequestCalled = true;
     }
 
     // If a preferred alt-data type was set, this signals the consumer is
     // interested in reading and/or writing the alt-data representation.