Bug 1938471 - Backed out changeset dcfa4149aaf3. r=necko-reviewers,kershaw, a=dmeehan

Please see the explaination for the fix in the bug [[ https://bugzilla.mozilla.org/show_bug.cgi?id=1938471#c16 | comment ]]

Differential Revision: https://phabricator.services.mozilla.com/D235761
This commit is contained in:
smayya 2025-02-04 15:48:43 +00:00
parent 8b6f2a74f2
commit f05fe128f1
2 changed files with 3 additions and 26 deletions

View file

@ -367,9 +367,6 @@ FetchDriver::~FetchDriver() {
// We assert this since even on failures, we should call
// FailWithNetworkError().
MOZ_ASSERT(mResponseAvailableCalled);
if (mObserver) {
mObserver = nullptr;
}
}
already_AddRefed<PreloaderBase> FetchDriver::FindPreload(nsIURI* aURI) {
@ -1013,9 +1010,7 @@ FetchDriver::OnStartRequest(nsIRequest* aRequest) {
}
if (!mChannel) {
// if the request is aborted, we remove the mObserver reference in
// OnStopRequest or ~FetchDriver()
MOZ_ASSERT_IF(!mAborted, !mObserver);
MOZ_ASSERT(!mObserver);
return NS_BINDING_ABORTED;
}
@ -1389,11 +1384,7 @@ FetchDriver::OnDataAvailable(nsIRequest* aRequest, nsIInputStream* aInputStream,
uint64_t aOffset, uint32_t aCount) {
// NB: This can be called on any thread! But we're guaranteed that it is
// called between OnStartRequest and OnStopRequest, so we don't need to worry
// about races for accesses in OnStartRequest, OnStopRequest and
// member functions accessed before opening the channel.
// However, we have a possibility of a race from FetchDriverAbortActions.
// Hence, we need to ensure that we are not modifying any members accessed by
// FetchDriver::FetchDriverAbortActions
// about races.
if (!mPipeOutputStream) {
// We ignore the body for HEAD/CONNECT requests.
@ -1468,14 +1459,6 @@ FetchDriver::OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) {
MOZ_DIAGNOSTIC_ASSERT(!mOnStopRequestCalled);
mOnStopRequestCalled = true;
if (mObserver && mAborted) {
// fetch request was aborted.
// We have already sent the observer
// notification that request has been aborted in FetchDriverAbortActions.
// Remove the observer reference and don't push anymore notifications.
mObserver = nullptr;
}
// main data loading is going to finish, breaking the reference cycle.
RefPtr<AlternativeDataStreamListener> altDataListener =
std::move(mAltDataListener);
@ -1823,10 +1806,7 @@ void FetchDriver::FetchDriverAbortActions(AbortSignalImpl* aSignalImpl) {
reason.set(aSignalImpl->RawReason());
}
mObserver->OnResponseEnd(FetchDriverObserver::eAborted, reason);
// As a part of cleanup, we are not removing the mObserver reference as it
// could race with mObserver access in OnDataAvailable when it runs OMT.
// We will be removing the reference in the OnStopRequest which guaranteed
// to run after cancelling the channel.
mObserver = nullptr;
}
if (mChannel) {

View file

@ -155,9 +155,6 @@ class FetchDriver final : public nsIChannelEventSink,
SafeRefPtr<InternalRequest> mRequest;
SafeRefPtr<InternalResponse> mResponse;
nsCOMPtr<nsIOutputStream> mPipeOutputStream;
// Access to mObserver can be racy from OnDataAvailable and
// FetchAbortActions. This must not be modified
// in either of these functions.
RefPtr<FetchDriverObserver> mObserver;
RefPtr<Document> mDocument;
nsCOMPtr<nsICSPEventListener> mCSPEventListener;