diff --git a/netwerk/base/nsLoadGroup.cpp b/netwerk/base/nsLoadGroup.cpp index 2ea2398c747d..ae522bb9d883 100644 --- a/netwerk/base/nsLoadGroup.cpp +++ b/netwerk/base/nsLoadGroup.cpp @@ -194,14 +194,18 @@ nsLoadGroup::Cancel(nsresult status) { mIsCanceling = true; nsresult firstError = NS_OK; - while (count > 0) { - nsCOMPtr request = dont_AddRef(requests.ElementAt(--count)); + nsCOMPtr request = requests.ElementAt(--count); NS_ASSERTION(request, "NULL request found in list."); if (!mRequests.Search(request)) { // |request| was removed already + // We need to null out the entry in the request array so we don't try + // to notify the observers for this request. + nsCOMPtr request = dont_AddRef(requests.ElementAt(count)); + requests.ElementAt(count) = nullptr; + continue; } @@ -215,16 +219,24 @@ nsLoadGroup::Cancel(nsresult status) { // Cancel the request... rv = request->Cancel(status); - // - // Remove the request from the load group... This may cause - // the OnStopRequest notification to fire... - // - // XXX: What should the context be? - // - (void)RemoveRequest(request, nullptr, status); - // Remember the first failure and return it... if (NS_FAILED(rv) && NS_SUCCEEDED(firstError)) firstError = rv; + + if (NS_FAILED(RemoveRequestFromHashtable(request, status))) { + // It's possible that request->Cancel causes the request to be removed + // from the loadgroup causing RemoveRequestFromHashtable to fail. + // In that case we shouldn't call NotifyRemovalObservers or decrement + // mForegroundCount since that has already happened. + nsCOMPtr request = dont_AddRef(requests.ElementAt(count)); + requests.ElementAt(count) = nullptr; + + continue; + } + } + + for (count = requests.Length(); count > 0;) { + nsCOMPtr request = dont_AddRef(requests.ElementAt(--count)); + (void)NotifyRemovalObservers(request, status); } if (mRequestContext) { @@ -478,6 +490,20 @@ nsLoadGroup::AddRequest(nsIRequest* request, nsISupports* ctxt) { NS_IMETHODIMP nsLoadGroup::RemoveRequest(nsIRequest* request, nsISupports* ctxt, nsresult aStatus) { + // Make sure we have a owning reference to the request we're about + // to remove. + nsCOMPtr kungFuDeathGrip(request); + + nsresult rv = RemoveRequestFromHashtable(request, aStatus); + if (NS_FAILED(rv)) { + return rv; + } + + return NotifyRemovalObservers(request, aStatus); +} + +nsresult nsLoadGroup::RemoveRequestFromHashtable(nsIRequest* request, + nsresult aStatus) { NS_ENSURE_ARG_POINTER(request); nsresult rv; @@ -490,11 +516,6 @@ nsLoadGroup::RemoveRequest(nsIRequest* request, nsISupports* ctxt, mRequests.EntryCount() - 1)); } - // Make sure we have a owning reference to the request we're about - // to remove. - - nsCOMPtr kungFuDeathGrip(request); - // // Remove the request from the group. If this fails, it means that // the request was *not* in the group so do not update the foreground @@ -546,11 +567,17 @@ nsLoadGroup::RemoveRequest(nsIRequest* request, nsISupports* ctxt, TelemetryReport(); } + return NS_OK; +} + +nsresult nsLoadGroup::NotifyRemovalObservers(nsIRequest* request, + nsresult aStatus) { + NS_ENSURE_ARG_POINTER(request); // Undo any group priority delta... if (mPriority != 0) RescheduleRequest(request, -mPriority); nsLoadFlags flags; - rv = request->GetLoadFlags(&flags); + nsresult rv = request->GetLoadFlags(&flags); if (NS_FAILED(rv)) return rv; if (!(flags & nsIRequest::LOAD_BACKGROUND)) { diff --git a/netwerk/base/nsLoadGroup.h b/netwerk/base/nsLoadGroup.h index 41caa414babc..baa2c8801635 100644 --- a/netwerk/base/nsLoadGroup.h +++ b/netwerk/base/nsLoadGroup.h @@ -62,6 +62,9 @@ class nsLoadGroup : public nsILoadGroup, void TelemetryReportChannel(nsITimedChannel* timedChannel, bool defaultRequest); + nsresult RemoveRequestFromHashtable(nsIRequest* aRequest, nsresult aStatus); + nsresult NotifyRemovalObservers(nsIRequest* aRequest, nsresult aStatus); + protected: uint32_t mForegroundCount; uint32_t mLoadFlags;