From 66724802d89cfb76b8e7b7fb91ab9874069dae91 Mon Sep 17 00:00:00 2001 From: Cosmin Sabou Date: Thu, 3 Oct 2019 21:38:39 +0300 Subject: [PATCH] Backed out 3 changesets (bug 1533957) for linux fission failures. CLOSED TREE Backed out changeset 93dc3aae76fe (bug 1533957) Backed out changeset aa6ab3f678cc (bug 1533957) Backed out changeset 19d46cc8b80f (bug 1533957) --- netwerk/base/nsLoadGroup.cpp | 50 ++++-------- netwerk/base/nsLoadGroup.h | 3 - netwerk/test/unit/test_loadgroup_cancel.js | 94 ---------------------- netwerk/test/unit/xpcshell.ini | 1 - 4 files changed, 16 insertions(+), 132 deletions(-) delete mode 100644 netwerk/test/unit/test_loadgroup_cancel.js diff --git a/netwerk/base/nsLoadGroup.cpp b/netwerk/base/nsLoadGroup.cpp index c483b485ece6..f8b5452e73a9 100644 --- a/netwerk/base/nsLoadGroup.cpp +++ b/netwerk/base/nsLoadGroup.cpp @@ -194,18 +194,14 @@ nsLoadGroup::Cancel(nsresult status) { mIsCanceling = true; nsresult firstError = NS_OK; + while (count > 0) { - nsCOMPtr request = requests.ElementAt(--count); + nsCOMPtr request = dont_AddRef(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; } @@ -216,20 +212,21 @@ nsLoadGroup::Cancel(nsresult status) { nameStr.get())); } + // + // 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); + // Cancel the request... rv = request->Cancel(status); - (void)RemoveRequestFromHashtable(request, status); - // Remember the first failure and return it... if (NS_FAILED(rv) && NS_SUCCEEDED(firstError)) firstError = rv; } - for (count = requests.Length(); count > 0;) { - nsCOMPtr request = dont_AddRef(requests.ElementAt(--count)); - (void)NotifyRemovalObservers(request, status); - } - if (mRequestContext) { Unused << mRequestContext->CancelTailPendingRequests(status); } @@ -481,20 +478,6 @@ 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; @@ -507,6 +490,11 @@ nsresult nsLoadGroup::RemoveRequestFromHashtable(nsIRequest* request, 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 @@ -558,17 +546,11 @@ nsresult nsLoadGroup::RemoveRequestFromHashtable(nsIRequest* request, 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; - nsresult rv = request->GetLoadFlags(&flags); + 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 baa2c8801635..41caa414babc 100644 --- a/netwerk/base/nsLoadGroup.h +++ b/netwerk/base/nsLoadGroup.h @@ -62,9 +62,6 @@ 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; diff --git a/netwerk/test/unit/test_loadgroup_cancel.js b/netwerk/test/unit/test_loadgroup_cancel.js deleted file mode 100644 index accfede36a86..000000000000 --- a/netwerk/test/unit/test_loadgroup_cancel.js +++ /dev/null @@ -1,94 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -"use strict"; - -function makeChan(url) { - return NetUtil.newChannel({ - uri: url, - loadUsingSystemPrincipal: true, - }).QueryInterface(Ci.nsIHttpChannel); -} - -const { HttpServer } = ChromeUtils.import("resource://testing-common/httpd.js"); - -function request_handler(metadata, response) { - response.processAsync(); - do_timeout(500, () => { - const body = "some body once told me..."; - response.setStatusLine(metadata.httpVersion, 200, "Ok"); - response.setHeader("Content-Type", "text/plain", false); - response.setHeader("Content-Length", "" + body.length, false); - response.bodyOutputStream.write(body, body.length); - response.finish(); - }); -} - -// This test checks that when canceling a loadgroup by the time the loadgroup's -// groupObserver is sent OnStopRequest for a request, that request has been -// canceled. -add_task(async function test_cancelledInOnStop() { - let http_server = new HttpServer(); - http_server.registerPathHandler("/test1", request_handler); - http_server.registerPathHandler("/test2", request_handler); - http_server.registerPathHandler("/test3", request_handler); - http_server.start(-1); - const port = http_server.identity.primaryPort; - - let loadGroup = Cc["@mozilla.org/network/load-group;1"].createInstance( - Ci.nsILoadGroup - ); - - let loadListener = { - onStartRequest: aRequest => { - info("onStartRequest"); - }, - onStopRequest: (aRequest, aStatusCode) => { - equal( - aStatusCode, - Cr.NS_ERROR_ABORT, - "aStatusCode must be the cancellation code" - ); - equal( - aRequest.status, - Cr.NS_ERROR_ABORT, - "aRequest.status must be the cancellation code" - ); - }, - QueryInterface: ChromeUtils.generateQI([ - "nsIRequestObserver", - "nsISupportsWeakReference", - ]), - }; - loadGroup.groupObserver = loadListener; - - let chan1 = makeChan(`http://localhost:${port}/test1`); - chan1.loadGroup = loadGroup; - let chan2 = makeChan(`http://localhost:${port}/test2`); - chan2.loadGroup = loadGroup; - let chan3 = makeChan(`http://localhost:${port}/test3`); - chan3.loadGroup = loadGroup; - - await new Promise(resolve => do_timeout(500, resolve)); - - let promises = [ - new Promise(resolve => { - chan1.asyncOpen(new ChannelListener(resolve, null, CL_EXPECT_FAILURE)); - }), - new Promise(resolve => { - chan2.asyncOpen(new ChannelListener(resolve, null, CL_EXPECT_FAILURE)); - }), - new Promise(resolve => { - chan3.asyncOpen(new ChannelListener(resolve, null, CL_EXPECT_FAILURE)); - }), - ]; - - loadGroup.cancel(Cr.NS_ERROR_ABORT); - - await Promise.all(promises); - - await new Promise(resolve => { - http_server.stop(resolve); - }); -}); diff --git a/netwerk/test/unit/xpcshell.ini b/netwerk/test/unit/xpcshell.ini index 48db86b505d6..1bbfbf9dbd83 100644 --- a/netwerk/test/unit/xpcshell.ini +++ b/netwerk/test/unit/xpcshell.ini @@ -442,6 +442,5 @@ skip-if = os == "android" [test_head_request_no_response_body.js] [test_disabled_ftp.js] [test_cache_204_response.js] -[test_loadgroup_cancel.js] [test_node_execute.js] skip-if = os == "android" # node server doesn't run on android