Bug 1364812 - ensure OnStartRequest is called after nsHttpChannel::CallOnStartRequest. r=dragana.
authorShih-Chiang Chien <schien@mozilla.com>
Mon, 22 May 2017 16:26:22 +0800
changeset 360056 90d9d225e54f33ee8d9ae50b43c070ebafce6782
parent 360055 3241038a0214844fe761d7aa08a7244e77347d58
child 360057 621439cb44b1bcc9cc8415ee076deee48ef8431c
push id43196
push userschien@mozilla.com
push dateTue, 23 May 2017 03:59:18 +0000
treeherderautoland@90d9d225e54f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana
bugs1364812
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.