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
--- 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.