From b043f6d72dbaa6a932bd867c25970388bc0ae390 Mon Sep 17 00:00:00 2001 From: Jason Duell Date: Mon, 31 Mar 2014 14:43:04 -0700 Subject: [PATCH] Bug 985230 - mStatus is not correct in HttpChannelChild::OnStartRequest r=sworkman --- netwerk/protocol/http/HttpChannelChild.cpp | 103 ++++++++++++-------- netwerk/protocol/http/HttpChannelChild.h | 14 ++- netwerk/protocol/http/HttpChannelParent.cpp | 14 ++- netwerk/protocol/http/PHttpChannel.ipdl | 8 +- 4 files changed, 88 insertions(+), 51 deletions(-) diff --git a/netwerk/protocol/http/HttpChannelChild.cpp b/netwerk/protocol/http/HttpChannelChild.cpp index 22c6fd65e5b..cf1ca53340f 100644 --- a/netwerk/protocol/http/HttpChannelChild.cpp +++ b/netwerk/protocol/http/HttpChannelChild.cpp @@ -178,6 +178,7 @@ class StartRequestEvent : public ChannelEvent { public: StartRequestEvent(HttpChannelChild* child, + const nsresult& channelStatus, const nsHttpResponseHead& responseHead, const bool& useResponseHead, const nsHttpHeaderArray& requestHeaders, @@ -189,6 +190,7 @@ class StartRequestEvent : public ChannelEvent const NetAddr& selfAddr, const NetAddr& peerAddr) : mChild(child) + , mChannelStatus(channelStatus) , mResponseHead(responseHead) , mRequestHeaders(requestHeaders) , mUseResponseHead(useResponseHead) @@ -203,13 +205,14 @@ class StartRequestEvent : public ChannelEvent void Run() { - mChild->OnStartRequest(mResponseHead, mUseResponseHead, mRequestHeaders, - mIsFromCache, mCacheEntryAvailable, + mChild->OnStartRequest(mChannelStatus, mResponseHead, mUseResponseHead, + mRequestHeaders, mIsFromCache, mCacheEntryAvailable, mCacheExpirationTime, mCachedCharset, mSecurityInfoSerialization, mSelfAddr, mPeerAddr); } private: HttpChannelChild* mChild; + nsresult mChannelStatus; nsHttpResponseHead mResponseHead; nsHttpHeaderArray mRequestHeaders; bool mUseResponseHead; @@ -223,7 +226,8 @@ class StartRequestEvent : public ChannelEvent }; bool -HttpChannelChild::RecvOnStartRequest(const nsHttpResponseHead& responseHead, +HttpChannelChild::RecvOnStartRequest(const nsresult& channelStatus, + const nsHttpResponseHead& responseHead, const bool& useResponseHead, const nsHttpHeaderArray& requestHeaders, const bool& isFromCache, @@ -242,22 +246,24 @@ HttpChannelChild::RecvOnStartRequest(const nsHttpResponseHead& responseHead, "mDivertingToParent should be unset before OnStartRequest!"); if (mEventQ->ShouldEnqueue()) { - mEventQ->Enqueue(new StartRequestEvent(this, responseHead, useResponseHead, - requestHeaders, isFromCache, - cacheEntryAvailable, - cacheExpirationTime, cachedCharset, - securityInfoSerialization, selfAddr, - peerAddr)); + mEventQ->Enqueue(new StartRequestEvent(this, channelStatus, responseHead, + useResponseHead, requestHeaders, + isFromCache, cacheEntryAvailable, + cacheExpirationTime, cachedCharset, + securityInfoSerialization, selfAddr, + peerAddr)); } else { - OnStartRequest(responseHead, useResponseHead, requestHeaders, isFromCache, - cacheEntryAvailable, cacheExpirationTime, cachedCharset, - securityInfoSerialization, selfAddr, peerAddr); + OnStartRequest(channelStatus, responseHead, useResponseHead, requestHeaders, + isFromCache, cacheEntryAvailable, cacheExpirationTime, + cachedCharset, securityInfoSerialization, selfAddr, + peerAddr); } return true; } void -HttpChannelChild::OnStartRequest(const nsHttpResponseHead& responseHead, +HttpChannelChild::OnStartRequest(const nsresult& channelStatus, + const nsHttpResponseHead& responseHead, const bool& useResponseHead, const nsHttpHeaderArray& requestHeaders, const bool& isFromCache, @@ -277,6 +283,10 @@ HttpChannelChild::OnStartRequest(const nsHttpResponseHead& responseHead, MOZ_RELEASE_ASSERT(!mDivertingToParent, "mDivertingToParent should be unset before OnStartRequest!"); + if (!mCanceled && NS_SUCCEEDED(mStatus)) { + mStatus = channelStatus; + } + if (useResponseHead && !mCanceled) mResponseHead = new nsHttpResponseHead(responseHead); @@ -331,25 +341,31 @@ class TransportAndDataEvent : public ChannelEvent { public: TransportAndDataEvent(HttpChannelChild* child, - const nsresult& status, + const nsresult& channelStatus, + const nsresult& transportStatus, const uint64_t& progress, const uint64_t& progressMax, const nsCString& data, const uint64_t& offset, const uint32_t& count) : mChild(child) - , mStatus(status) + , mChannelStatus(channelStatus) + , mTransportStatus(transportStatus) , mProgress(progress) , mProgressMax(progressMax) , mData(data) , mOffset(offset) , mCount(count) {} - void Run() { mChild->OnTransportAndData(mStatus, mProgress, mProgressMax, - mData, mOffset, mCount); } + void Run() + { + mChild->OnTransportAndData(mChannelStatus, mTransportStatus, mProgress, + mProgressMax, mData, mOffset, mCount); + } private: HttpChannelChild* mChild; - nsresult mStatus; + nsresult mChannelStatus; + nsresult mTransportStatus; uint64_t mProgress; uint64_t mProgressMax; nsCString mData; @@ -358,7 +374,8 @@ class TransportAndDataEvent : public ChannelEvent }; bool -HttpChannelChild::RecvOnTransportAndData(const nsresult& status, +HttpChannelChild::RecvOnTransportAndData(const nsresult& channelStatus, + const nsresult& transportStatus, const uint64_t& progress, const uint64_t& progressMax, const nsCString& data, @@ -369,20 +386,23 @@ HttpChannelChild::RecvOnTransportAndData(const nsresult& status, "Should not be receiving any more callbacks from parent!"); if (mEventQ->ShouldEnqueue()) { - mEventQ->Enqueue(new TransportAndDataEvent(this, status, progress, - progressMax, data, offset, - count)); + mEventQ->Enqueue(new TransportAndDataEvent(this, channelStatus, + transportStatus, progress, + progressMax, data, offset, + count)); } else { MOZ_RELEASE_ASSERT(!mDivertingToParent, "ShouldEnqueue when diverting to parent!"); - OnTransportAndData(status, progress, progressMax, data, offset, count); + OnTransportAndData(channelStatus, transportStatus, progress, progressMax, + data, offset, count); } return true; } void -HttpChannelChild::OnTransportAndData(const nsresult& status, +HttpChannelChild::OnTransportAndData(const nsresult& channelStatus, + const nsresult& transportStatus, const uint64_t progress, const uint64_t& progressMax, const nsCString& data, @@ -391,6 +411,10 @@ HttpChannelChild::OnTransportAndData(const nsresult& status, { LOG(("HttpChannelChild::OnTransportAndData [this=%p]\n", this)); + if (!mCanceled && NS_SUCCEEDED(mStatus)) { + mStatus = channelStatus; + } + // For diversion to parent, just SendDivertOnDataAvailable. if (mDivertingToParent) { MOZ_RELEASE_ASSERT(!mFlushedForDiversion, @@ -421,12 +445,12 @@ HttpChannelChild::OnTransportAndData(const nsresult& status, { // OnStatus // - MOZ_ASSERT(status == NS_NET_STATUS_RECEIVING_FROM || - status == NS_NET_STATUS_READING); + MOZ_ASSERT(transportStatus == NS_NET_STATUS_RECEIVING_FROM || + transportStatus == NS_NET_STATUS_READING); nsAutoCString host; mURI->GetHost(host); - mProgressSink->OnStatus(this, nullptr, status, + mProgressSink->OnStatus(this, nullptr, transportStatus, NS_ConvertUTF8toUTF16(host).get()); // OnProgress // @@ -463,50 +487,51 @@ class StopRequestEvent : public ChannelEvent { public: StopRequestEvent(HttpChannelChild* child, - const nsresult& statusCode) + const nsresult& channelStatus) : mChild(child) - , mStatusCode(statusCode) {} + , mChannelStatus(channelStatus) {} - void Run() { mChild->OnStopRequest(mStatusCode); } + void Run() { mChild->OnStopRequest(mChannelStatus); } private: HttpChannelChild* mChild; - nsresult mStatusCode; + nsresult mChannelStatus; }; bool -HttpChannelChild::RecvOnStopRequest(const nsresult& statusCode) +HttpChannelChild::RecvOnStopRequest(const nsresult& channelStatus) { MOZ_RELEASE_ASSERT(!mFlushedForDiversion, "Should not be receiving any more callbacks from parent!"); if (mEventQ->ShouldEnqueue()) { - mEventQ->Enqueue(new StopRequestEvent(this, statusCode)); + mEventQ->Enqueue(new StopRequestEvent(this, channelStatus)); } else { MOZ_ASSERT(!mDivertingToParent, "ShouldEnqueue when diverting to parent!"); - OnStopRequest(statusCode); + OnStopRequest(channelStatus); } return true; } void -HttpChannelChild::OnStopRequest(const nsresult& statusCode) +HttpChannelChild::OnStopRequest(const nsresult& channelStatus) { LOG(("HttpChannelChild::OnStopRequest [this=%p status=%x]\n", - this, statusCode)); + this, channelStatus)); if (mDivertingToParent) { MOZ_RELEASE_ASSERT(!mFlushedForDiversion, "Should not be processing any more callbacks from parent!"); - SendDivertOnStopRequest(statusCode); + SendDivertOnStopRequest(channelStatus); return; } mIsPending = false; - if (!mCanceled && NS_SUCCEEDED(mStatus)) - mStatus = statusCode; + if (!mCanceled && NS_SUCCEEDED(mStatus)) { + mStatus = channelStatus; + } { // We must flush the queue before we Send__delete__ // (although we really shouldn't receive any msgs after OnStop), diff --git a/netwerk/protocol/http/HttpChannelChild.h b/netwerk/protocol/http/HttpChannelChild.h index 0836e2a68c3..504a2dbb827 100644 --- a/netwerk/protocol/http/HttpChannelChild.h +++ b/netwerk/protocol/http/HttpChannelChild.h @@ -96,7 +96,8 @@ public: void FlushedForDiversion(); protected: - bool RecvOnStartRequest(const nsHttpResponseHead& responseHead, + bool RecvOnStartRequest(const nsresult& channelStatus, + const nsHttpResponseHead& responseHead, const bool& useResponseHead, const nsHttpHeaderArray& requestHeaders, const bool& isFromCache, @@ -106,7 +107,8 @@ protected: const nsCString& securityInfoSerialization, const NetAddr& selfAddr, const NetAddr& peerAddr) MOZ_OVERRIDE; - bool RecvOnTransportAndData(const nsresult& status, + bool RecvOnTransportAndData(const nsresult& channelStatus, + const nsresult& status, const uint64_t& progress, const uint64_t& progressMax, const nsCString& data, @@ -161,7 +163,8 @@ private: void AssociateApplicationCache(const nsCString &groupID, const nsCString &clientID); - void OnStartRequest(const nsHttpResponseHead& responseHead, + void OnStartRequest(const nsresult& channelStatus, + const nsHttpResponseHead& responseHead, const bool& useResponseHead, const nsHttpHeaderArray& requestHeaders, const bool& isFromCache, @@ -171,13 +174,14 @@ private: const nsCString& securityInfoSerialization, const NetAddr& selfAddr, const NetAddr& peerAddr); - void OnTransportAndData(const nsresult& status, + void OnTransportAndData(const nsresult& channelStatus, + const nsresult& status, const uint64_t progress, const uint64_t& progressMax, const nsCString& data, const uint64_t& offset, const uint32_t& count); - void OnStopRequest(const nsresult& statusCode); + void OnStopRequest(const nsresult& channelStatus); void OnProgress(const uint64_t& progress, const uint64_t& progressMax); void OnStatus(const nsresult& status); void FailedAsyncOpen(const nsresult& status); diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index a50f33f7709..2fd63b7917f 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -588,6 +588,8 @@ HttpChannelParent::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext) chan->GetCacheToken(getter_AddRefs(cacheEntry)); mCacheEntry = do_QueryInterface(cacheEntry); + nsresult channelStatus = NS_OK; + chan->GetStatus(&channelStatus); nsCString secInfoSerialization; nsCOMPtr secInfoSupp; @@ -600,7 +602,8 @@ HttpChannelParent::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext) } if (mIPCClosed || - !SendOnStartRequest(responseHead ? *responseHead : nsHttpResponseHead(), + !SendOnStartRequest(channelStatus, + responseHead ? *responseHead : nsHttpResponseHead(), !!responseHead, requestHead->Headers(), isFromCache, @@ -650,13 +653,16 @@ HttpChannelParent::OnDataAvailable(nsIRequest *aRequest, if (NS_FAILED(rv)) return rv; + nsresult channelStatus = NS_OK; + mChannel->GetStatus(&channelStatus); + // OnDataAvailable is always preceded by OnStatus/OnProgress calls that set // mStoredStatus/mStoredProgress(Max) to appropriate values, unless // LOAD_BACKGROUND set. In that case, they'll have garbage values, but // child doesn't use them. - if (mIPCClosed || !SendOnTransportAndData(mStoredStatus, mStoredProgress, - mStoredProgressMax, data, aOffset, - aCount)) { + if (mIPCClosed || !SendOnTransportAndData(channelStatus, mStoredStatus, + mStoredProgress, mStoredProgressMax, + data, aOffset, aCount)) { return NS_ERROR_UNEXPECTED; } return NS_OK; diff --git a/netwerk/protocol/http/PHttpChannel.ipdl b/netwerk/protocol/http/PHttpChannel.ipdl index 820daf3e208..1b1c698d08c 100644 --- a/netwerk/protocol/http/PHttpChannel.ipdl +++ b/netwerk/protocol/http/PHttpChannel.ipdl @@ -80,7 +80,8 @@ parent: __delete__(); child: - OnStartRequest(nsHttpResponseHead responseHead, + OnStartRequest(nsresult channelStatus, + nsHttpResponseHead responseHead, bool useResponseHead, nsHttpHeaderArray requestHeaders, bool isFromCache, @@ -93,14 +94,15 @@ child: // Combines a single OnDataAvailable and its associated OnProgress & // OnStatus calls into one IPDL message - OnTransportAndData(nsresult status, + OnTransportAndData(nsresult channelStatus, + nsresult transportStatus, uint64_t progress, uint64_t progressMax, nsCString data, uint64_t offset, uint32_t count); - OnStopRequest(nsresult statusCode); + OnStopRequest(nsresult channelStatus); OnProgress(uint64_t progress, uint64_t progressMax);