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

@@ -194,18 +194,14 @@ nsLoadGroup::Cancel(nsresult status) {
   mIsCanceling = true;
 
   nsresult firstError = NS_OK;
+
   while (count > 0) {
-    nsCOMPtr<nsIRequest> request = requests.ElementAt(--count);
+    nsCOMPtr<nsIRequest> 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<nsIRequest> 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<nsIRequest> 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<nsIRequest> 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<nsIRequest> 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)) {

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;

netwerk/test/unit/test_loadgroup_cancel.js

@@ -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);
-  });
-});

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