Backed out changeset 4efa832fc852 (bug 1305217) for frequent timeouts in test_ext_webrequest_upload.html. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sat, 12 Nov 2016 13:45:03 +0100
changeset 352378 217ae1d5285328611de99e2e48042b19751dbb54
parent 352377 6cbde9c5e058999ba95319391b59a7d67649ca53
child 352379 31b6c03c69f18df000420d218b7b233f739af584
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1305217
milestone52.0a1
backs out4efa832fc852bb0499f8518c458ff052bede560e
Backed out changeset 4efa832fc852 (bug 1305217) for frequent timeouts in test_ext_webrequest_upload.html. r=backout
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/ext-webRequest.js
toolkit/components/extensions/test/mochitest/head_webrequest.js
toolkit/components/extensions/test/mochitest/test_ext_webrequest_background_events.html
toolkit/components/extensions/test/mochitest/test_ext_webrequest_basic.html
toolkit/modules/addons/WebRequest.jsm
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -37,17 +37,16 @@ XPCOMUtils.defineLazyGetter(this, "Paren
                             () => ExtensionParent.ParentAPIManager);
 
 const CATEGORY_EXTENSION_SCRIPTS_ADDON = "webextension-scripts-addon";
 
 Cu.import("resource://gre/modules/ExtensionCommon.jsm");
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 const {
-  DefaultMap,
   EventManager,
   SingletonEventManager,
   SpreadArgs,
   defineLazyGetter,
   findPathInObject,
   getInnerWindowID,
   getMessageManager,
   getUniqueId,
@@ -489,105 +488,96 @@ class ProxyAPIImplementation extends Sch
     this.childApiManager.callParentFunctionNoReturn(this.path, args);
   }
 
   callAsyncFunction(args, callback) {
     return this.childApiManager.callParentAsyncFunction(this.path, args, callback);
   }
 
   addListener(listener, args) {
-    let map = this.childApiManager.listeners.get(this.path);
-
-    if (map.listeners.has(listener)) {
-      // TODO: Called with different args?
-      return;
+    let set = this.childApiManager.listeners.get(this.path);
+    if (!set) {
+      set = new Set();
+      this.childApiManager.listeners.set(this.path, set);
     }
 
-    let id = getUniqueId();
+    set.add(listener);
 
-    map.ids.set(id, listener);
-    map.listeners.set(listener, id);
+    if (set.size == 1) {
+      args = args.slice(1);
 
-    this.childApiManager.messageManager.sendAsyncMessage("API:AddListener", {
-      childId: this.childApiManager.id,
-      listenerId: id,
-      path: this.path,
-      args,
-    });
+      this.childApiManager.messageManager.sendAsyncMessage("API:AddListener", {
+        childId: this.childApiManager.id,
+        path: this.path,
+        args,
+      });
+    }
   }
 
   removeListener(listener) {
-    let map = this.childApiManager.listeners.get(this.path);
-
-    if (!map.listeners.has(listener)) {
+    let set = this.childApiManager.listeners.get(this.path);
+    if (!set) {
       return;
     }
-
-    let id = map.listeners.get(listener);
-    map.listeners.delete(listener);
-    map.ids.delete(id);
+    set.delete(listener);
 
-    this.childApiManager.messageManager.sendAsyncMessage("API:RemoveListener", {
-      childId: this.childApiManager.id,
-      listenerId: id,
-      path: this.path,
-    });
+    if (set.size == 0) {
+      this.childApiManager.messageManager.sendAsyncMessage("API:RemoveListener", {
+        childId: this.childApiManager.id,
+        path: this.path,
+      });
+    }
   }
 
   hasListener(listener) {
-    let map = this.childApiManager.listeners.get(this.path);
-    return map.listeners.has(listener);
+    let set = this.childApiManager.listeners.get(this.path);
+    return set ? set.has(listener) : false;
   }
 }
 
+let nextId = 1;
+
 // We create one instance of this class for every extension context that
 // needs to use remote APIs. It uses the message manager to communicate
 // with the ParentAPIManager singleton in ExtensionParent.jsm. It
 // handles asynchronous function calls as well as event listeners.
 class ChildAPIManagerBase {
   constructor(context, messageManager, localApis, contextData) {
     this.context = context;
     this.messageManager = messageManager;
 
     // The root namespace of all locally implemented APIs. If an extension calls
     // an API that does not exist in this object, then the implementation is
     // delegated to the ParentAPIManager.
     this.localApis = localApis;
 
     this.id = `${context.extension.id}.${context.contextId}`;
 
-    MessageChannel.addListener(messageManager, "API:RunListener", this);
+    messageManager.addMessageListener("API:RunListener", this);
     messageManager.addMessageListener("API:CallResult", this);
 
-    this.messageFilterStrict = {childId: this.id};
-
-    this.listeners = new DefaultMap(() => ({
-      ids: new Map(),
-      listeners: new Map(),
-    }));
+    // Map[path -> Set[listener]]
+    // path is, e.g., "runtime.onMessage".
+    this.listeners = new Map();
 
     // Map[callId -> Deferred]
     this.callPromises = new Map();
   }
 
-  receiveMessage({name, messageName, data}) {
+  receiveMessage({name, data}) {
     if (data.childId != this.id) {
       return;
     }
 
-    switch (name || messageName) {
+    switch (name) {
       case "API:RunListener":
-        let map = this.listeners.get(data.path);
-        let listener = map.ids.get(data.listenerId);
-
-        if (listener) {
-          return this.context.runSafe(listener, ...data.args);
+        let listeners = this.listeners.get(data.path);
+        for (let callback of listeners) {
+          this.context.runSafe(callback, ...data.args);
         }
-
-        Cu.reportError(`Unknown listener at childId=${data.childId} path=${data.path} listenerId=${data.listenerId}\n`);
         break;
 
       case "API:CallResult":
         let deferred = this.callPromises.get(data.callId);
         if ("error" in data) {
           deferred.reject(data.error);
         } else {
           deferred.resolve(new SpreadArgs(data.result));
@@ -759,17 +749,18 @@ class PseudoChildAPIManager extends Chil
     // Synchronously unload the ProxyContext because we synchronously create it.
     this.context.callOnClose(this.parentContext);
   }
 
   getFallbackImplementation(namespace, name) {
     // This is gross and should be removed ASAP.
     let useDirectParentAPI = (
       // Incompatible APIs are listed here.
-      namespace == "webNavigation" // ChildAPIManager is oblivious to filters.
+      namespace == "webNavigation" || // ChildAPIManager is oblivious to filters.
+      namespace == "webRequest" // Incompatible by design (synchronous).
     );
 
     if (useDirectParentAPI) {
       let apiObj = findPathInObject(this.parentContext.apiObj, namespace, false);
 
       if (apiObj && name in apiObj) {
         return new LocalAPIImplementation(apiObj, name, this.context);
       }
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -502,31 +502,22 @@ ParentAPIManager = {
   },
 
   addListener(data, target) {
     let context = this.getContextById(data.childId);
     if (context.parentMessageManager !== target.messageManager) {
       Cu.reportError("WebExtension warning: Message manager unexpectedly changed");
     }
 
-    let {childId} = data;
-
     function listener(...listenerArgs) {
-      return context.sendMessage(
-        context.parentMessageManager,
-        "API:RunListener",
-        {
-          childId,
-          listenerId: data.listenerId,
-          path: data.path,
-          args: listenerArgs,
-        },
-        {
-          recipient: {childId},
-        });
+      context.parentMessageManager.sendAsyncMessage("API:RunListener", {
+        childId: data.childId,
+        path: data.path,
+        args: listenerArgs,
+      });
     }
 
     context.listenerProxies.set(data.path, listener);
 
     let args = Cu.cloneInto(data.args, context.sandbox);
     findPathInObject(context.apiObj, data.path).addListener(listener, ...args);
   },
 
--- a/toolkit/components/extensions/ext-webRequest.js
+++ b/toolkit/components/extensions/ext-webRequest.js
@@ -8,16 +8,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/MatchPattern.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "WebRequest",
                                   "resource://gre/modules/WebRequest.jsm");
 
 Cu.import("resource://gre/modules/ExtensionManagement.jsm");
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 var {
   SingletonEventManager,
+  runSafeSync,
 } = ExtensionUtils;
 
 // EventManager-like class specifically for WebRequest. Inherits from
 // SingletonEventManager. Takes care of converting |details| parameter
 // when invoking listeners.
 function WebRequestEventManager(context, eventName) {
   let name = `webRequest.${eventName}`;
   let register = (callback, filter, info) => {
@@ -53,17 +54,17 @@ function WebRequestEventManager(context,
       let optional = ["requestHeaders", "responseHeaders", "statusCode", "statusLine", "error", "redirectUrl",
                       "requestBody"];
       for (let opt of optional) {
         if (opt in data) {
           data2[opt] = data[opt];
         }
       }
 
-      return context.runSafe(callback, data2);
+      return runSafeSync(context, callback, data2);
     };
 
     let filter2 = {};
     filter2.urls = new MatchPattern(filter.urls);
     if (filter.types) {
       filter2.types = filter.types;
     }
     if (filter.tabId) {
--- a/toolkit/components/extensions/test/mochitest/head_webrequest.js
+++ b/toolkit/components/extensions/test/mochitest/head_webrequest.js
@@ -30,19 +30,16 @@ function background(events) {
       // last event for that entry.  This will either be onCompleted, or the
       // last entry if an events list was provided.
       promises.push(new Promise(resolve => { entry.test.resolve = resolve; }));
       // If events was left undefined, we're expecting all normal events we're
       // listening for, exclude onBeforeRedirect and onErrorOccurred
       if (entry.events === undefined) {
         entry.events = Object.keys(events).filter(name => name != "onErrorOccurred" && name != "onBeforeRedirect");
       }
-      if (entry.optional_events === undefined) {
-        entry.optional_events = [];
-      }
     }
     // When every expected entry has finished our test is done.
     Promise.all(promises).then(() => {
       browser.test.sendMessage("done");
     });
     browser.test.sendMessage("continue");
   });
 
@@ -151,25 +148,20 @@ function background(events) {
     return details => {
       let result = {};
       browser.test.log(`${name} ${details.requestId} ${details.url}`);
       let expected = getExpected(details);
       if (!expected) {
         return result;
       }
       let expectedEvent = expected.events[0] == name;
+      browser.test.assertTrue(expectedEvent, `recieved ${name}`);
       if (expectedEvent) {
         expected.events.shift();
-      } else {
-        expectedEvent = expected.optional_events[0] == name;
-        if (expectedEvent) {
-          expected.optional_events.shift();
-        }
       }
-      browser.test.assertTrue(expectedEvent, `received ${name}`);
       browser.test.assertEq(expected.type, details.type, "resource type is correct");
       browser.test.assertEq(expected.origin || defaultOrigin, details.originUrl, "origin is correct");
 
       if (name == "onBeforeRequest") {
         // Save some values to test request consistency in later events.
         browser.test.assertTrue(details.tabId !== undefined, `tabId ${details.tabId}`);
         browser.test.assertTrue(details.requestId !== undefined, `requestId ${details.requestId}`);
         // Validate requestId if it's already set, this happens with redirects.
@@ -202,17 +194,17 @@ function background(events) {
       }
       if (name == "onSendHeaders") {
         if (expected.headers && expected.headers.request) {
           checkHeaders("request", expected, details);
         }
       }
       if (name == "onHeadersReceived") {
         browser.test.assertEq(expected.status || 200, details.statusCode,
-                              `expected HTTP status received for ${details.url}`);
+                              `expected HTTP status recieved for ${details.url}`);
         if (expected.headers && expected.headers.response) {
           result.responseHeaders = processHeaders("response", expected, details);
         }
       }
       if (name == "onCompleted") {
         // If we have already completed a GET request for this url,
         // and it was found, we expect for the response to come fromCache.
         // expected.cached may be undefined, force boolean.
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest_background_events.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_background_events.html
@@ -75,32 +75,28 @@ add_task(function* test_webRequest_backg
         "onHeadersReceived",
         "onResponseStarted",
         "onCompleted",
       ]);
 
       function listener(name, details) {
         browser.test.assertTrue(eventNames.has(name), `recieved ${name}`);
         eventNames.delete(name);
-
-        if (eventNames.size === 0) {
-          browser.test.assertEq(0, eventNames.size, "messages recieved");
-          browser.test.sendMessage("done");
-        }
       }
 
       for (let name of eventNames) {
         browser.webRequest[name].addListener(
           listener.bind(null, name),
           {urls: ["https://example.com/*"]}
         );
       }
 
       fetch("https://example.com/example.txt").then(() => {
-        browser.test.pass("Fetch succeeded.");
+        browser.test.assertEq(0, eventNames.size, "messages recieved");
+        browser.test.sendMessage("done");
       }, () => {
         browser.test.fail("fetch recieved");
         browser.test.sendMessage("done");
       });
     },
   });
 
   yield extension.startup();
--- a/toolkit/components/extensions/test/mochitest/test_ext_webrequest_basic.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_basic.html
@@ -38,19 +38,19 @@ add_task(function* setup() {
 add_task(function* test_webRequest_links() {
   let expect = {
     "file_style_bad.css": {
       type: "stylesheet",
       events: ["onBeforeRequest", "onErrorOccurred"],
       cancel: "onBeforeRequest",
     },
     "file_style_redirect.css": {
+      status: 302,
       type: "stylesheet",
       events: ["onBeforeRequest", "onBeforeSendHeaders", "onBeforeRedirect"],
-      optional_events: ["onHeadersReceived"],
       redirect: "file_style_good.css",
     },
     "file_style_good.css": {
       type: "stylesheet",
     },
   };
   extension.sendMessage("set-expected", {expect, origin: location.href});
   yield extension.awaitMessage("continue");
@@ -67,19 +67,19 @@ add_task(function* test_webRequest_links
 add_task(function* test_webRequest_images() {
   let expect = {
     "file_image_bad.png": {
       type: "image",
       events: ["onBeforeRequest", "onErrorOccurred"],
       cancel: "onBeforeRequest",
     },
     "file_image_redirect.png": {
+      status: 302,
       type: "image",
       events: ["onBeforeRequest", "onBeforeSendHeaders", "onBeforeRedirect"],
-      optional_events: ["onHeadersReceived"],
       redirect: "file_image_good.png",
     },
     "file_image_good.png": {
       type: "image",
     },
   };
   extension.sendMessage("set-expected", {expect, origin: location.href});
   yield extension.awaitMessage("continue");
@@ -93,19 +93,19 @@ add_task(function* test_webRequest_image
 add_task(function* test_webRequest_scripts() {
   let expect = {
     "file_script_bad.js": {
       type: "script",
       events: ["onBeforeRequest", "onErrorOccurred"],
       cancel: "onBeforeRequest",
     },
     "file_script_redirect.js": {
+      status: 302,
       type: "script",
       events: ["onBeforeRequest", "onBeforeSendHeaders", "onBeforeRedirect"],
-      optional_events: ["onHeadersReceived"],
       redirect: "file_script_good.js",
     },
     "file_script_good.js": {
       type: "script",
     },
   };
   extension.sendMessage("set-expected", {expect, origin: location.href});
   yield extension.awaitMessage("continue");
@@ -237,17 +237,17 @@ add_task(function* test_webRequest_tabId
 add_task(function* test_webRequest_frames() {
   let expect = {
     "text/plain,webRequestTest": {
       type: "sub_frame",
       events: ["onBeforeRequest", "onCompleted"],
     },
     "text/plain,webRequestTest_bad": {
       type: "sub_frame",
-      events: ["onBeforeRequest", "onCompleted"],
+      events: ["onBeforeRequest", "onErrorOccurred"],
       cancel: "onBeforeRequest",
     },
     "redirection.sjs": {
       status: 302,
       type: "sub_frame",
       events: ["onBeforeRequest", "onBeforeSendHeaders", "onSendHeaders", "onHeadersReceived", "onBeforeRedirect"],
     },
     "dummy_page.html": {
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -326,30 +326,28 @@ ContentPolicyManager.init();
 function StartStopListener(manager, loadContext) {
   this.manager = manager;
   this.loadContext = loadContext;
   this.orig = null;
 }
 
 StartStopListener.prototype = {
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIRequestObserver,
-                                         Ci.nsIStreamListener]),
+                                         Ci.nsIStreamListener,
+                                         Ci.nsISupports]),
 
   onStartRequest: function(request, context) {
     this.manager.onStartRequest(request, this.loadContext);
-    this.orig.onStartRequest(request, context);
+    return this.orig.onStartRequest(request, context);
   },
 
   onStopRequest(request, context, statusCode) {
-    try {
-      this.orig.onStopRequest(request, context, statusCode);
-    } catch (e) {
-      Cu.reportError(e);
-    }
+    let result = this.orig.onStopRequest(request, context, statusCode);
     this.manager.onStopRequest(request, this.loadContext);
+    return result;
   },
 
   onDataAvailable(...args) {
     return this.orig.onDataAvailable(...args);
   },
 };
 
 var ChannelEventSink = {
@@ -561,263 +559,209 @@ HttpObserverManager = {
   errorCheck(channel, loadContext, channelData = null) {
     let errorData = this.maybeError(channel, null, channelData);
     if (errorData) {
       this.runChannelListener(channel, loadContext, "onError", errorData);
     }
     return errorData;
   },
 
-  /**
-   * Resumes the channel if it is currently suspended due to this
-   * listener.
-   *
-   * @param {nsIChannel} channel
-   *        The channel to possibly suspend.
-   */
-  maybeResume(channel) {
-    let data = getData(channel);
-    if (data.suspended) {
-      channel.resume();
-      data.suspended = false;
-    }
-  },
-
-  /**
-   * Suspends the channel if it is not currently suspended due to this
-   * listener. Returns true if the channel was suspended as a result of
-   * this call.
-   *
-   * @param {nsIChannel} channel
-   *        The channel to possibly suspend.
-   * @returns {boolean}
-   *        True if this call resulted in the channel being suspended.
-   */
-  maybeSuspend(channel) {
-    let data = getData(channel);
-    if (!data.suspended) {
-      channel.suspend();
-      data.suspended = true;
-      return true;
-    }
-  },
-
   runChannelListener(channel, loadContext = null, kind, extraData = null) {
-    let handlerResults = [];
-    let requestHeaders;
-    let responseHeaders;
-
-    try {
-      if (this.activityInitialized) {
-        let channelData = getData(channel);
-        if (kind === "onError") {
-          if (channelData.errorNotified) {
-            return;
-          }
-          channelData.errorNotified = true;
-        } else if (this.errorCheck(channel, loadContext, channelData)) {
+    if (this.activityInitialized) {
+      let channelData = getData(channel);
+      if (kind === "onError") {
+        if (channelData.errorNotified) {
           return;
         }
+        channelData.errorNotified = true;
+      } else if (this.errorCheck(channel, loadContext, channelData)) {
+        return;
       }
-      let listeners = this.listeners[kind];
-      let browser = loadContext && loadContext.topFrameElement;
-      let loadInfo = channel.loadInfo;
-      let policyType = (loadInfo ? loadInfo.externalContentPolicyType
-                                 : Ci.nsIContentPolicy.TYPE_OTHER);
+    }
+    let listeners = this.listeners[kind];
+    let browser = loadContext && loadContext.topFrameElement;
+    let loadInfo = channel.loadInfo;
+    let policyType = (loadInfo ? loadInfo.externalContentPolicyType
+                               : Ci.nsIContentPolicy.TYPE_OTHER);
 
-      let includeStatus = (["headersReceived", "onRedirect", "onStart", "onStop"].includes(kind) &&
-                           channel instanceof Ci.nsIHttpChannel);
+    let includeStatus = (["headersReceived", "onRedirect", "onStart", "onStop"].includes(kind) &&
+                         channel instanceof Ci.nsIHttpChannel);
 
-      let blockable = ["headersReceived", "opening", "modify"].includes(kind);
+    let requestHeaders = new RequestHeaderChanger(channel);
+    let responseHeaders;
+    try {
+      responseHeaders = new ResponseHeaderChanger(channel);
+    } catch (e) {
+      // Just ignore this for the request phases where response headers
+      // aren't yet available.
+    }
 
-      if (channel instanceof Ci.nsIHttpChannel) {
-        requestHeaders = new RequestHeaderChanger(channel);
-        try {
-          responseHeaders = new ResponseHeaderChanger(channel);
-        } catch (e) {
-          // Just ignore this for the request phases where response headers
-          // aren't yet available.
-        }
+    let commonData = null;
+    let uri = channel.URI;
+    let handlerResults = [];
+    let requestBody;
+    for (let [callback, opts] of listeners.entries()) {
+      if (!this.shouldRunListener(policyType, uri, opts.filter)) {
+        continue;
       }
 
-      let commonData = null;
-      let uri = channel.URI;
-      let requestBody;
-      for (let [callback, opts] of listeners.entries()) {
-        if (!this.shouldRunListener(policyType, uri, opts.filter)) {
-          continue;
+      if (!commonData) {
+        commonData = {
+          requestId: RequestId.get(channel),
+          url: uri.spec,
+          method: channel.requestMethod,
+          browser: browser,
+          type: WebRequestCommon.typeForPolicyType(policyType),
+          fromCache: getData(channel).fromCache,
+          windowId: 0,
+          parentWindowId: 0,
+        };
+
+        if (loadInfo) {
+          let originPrincipal = loadInfo.triggeringPrincipal;
+          if (originPrincipal.URI) {
+            commonData.originUrl = originPrincipal.URI.spec;
+          }
+
+          let {isSystemPrincipal} = Services.scriptSecurityManager;
+
+          commonData.isSystemPrincipal = (isSystemPrincipal(loadInfo.triggeringPrincipal) ||
+                                          isSystemPrincipal(loadInfo.loadingPrincipal));
+
+          if (loadInfo.frameOuterWindowID) {
+            Object.assign(commonData, {
+              windowId: loadInfo.frameOuterWindowID,
+              parentWindowId: loadInfo.outerWindowID,
+            });
+          } else {
+            Object.assign(commonData, {
+              windowId: loadInfo.outerWindowID,
+              parentWindowId: loadInfo.parentOuterWindowID,
+            });
+          }
         }
 
-        if (!commonData) {
-          commonData = {
-            requestId: RequestId.get(channel),
-            url: uri.spec,
-            method: channel.requestMethod,
-            browser: browser,
-            type: WebRequestCommon.typeForPolicyType(policyType),
-            fromCache: getData(channel).fromCache,
-            windowId: 0,
-            parentWindowId: 0,
-          };
-
-          if (loadInfo) {
-            let originPrincipal = loadInfo.triggeringPrincipal;
-            if (originPrincipal.URI) {
-              commonData.originUrl = originPrincipal.URI.spec;
-            }
-
-            let {isSystemPrincipal} = Services.scriptSecurityManager;
-
-            commonData.isSystemPrincipal = (isSystemPrincipal(loadInfo.triggeringPrincipal) ||
-                                            isSystemPrincipal(loadInfo.loadingPrincipal));
-
-            if (loadInfo.frameOuterWindowID) {
-              Object.assign(commonData, {
-                windowId: loadInfo.frameOuterWindowID,
-                parentWindowId: loadInfo.outerWindowID,
-              });
-            } else {
-              Object.assign(commonData, {
-                windowId: loadInfo.outerWindowID,
-                parentWindowId: loadInfo.parentOuterWindowID,
-              });
-            }
+        if (channel instanceof Ci.nsIHttpChannelInternal) {
+          try {
+            commonData.ip = channel.remoteAddress;
+          } catch (e) {
+            // The remoteAddress getter throws if the address is unavailable,
+            // but ip is an optional property so just ignore the exception.
           }
-
-          if (channel instanceof Ci.nsIHttpChannelInternal) {
-            try {
-              commonData.ip = channel.remoteAddress;
-            } catch (e) {
-              // The remoteAddress getter throws if the address is unavailable,
-              // but ip is an optional property so just ignore the exception.
-            }
-          }
-
-          Object.assign(commonData, extraData);
         }
 
-        let data = Object.assign({}, commonData);
+        Object.assign(commonData, extraData);
+      }
 
-        if (opts.requestHeaders) {
-          data.requestHeaders = requestHeaders.toArray();
-        }
+      let data = Object.assign({}, commonData);
 
-        if (opts.responseHeaders && responseHeaders) {
-          data.responseHeaders = responseHeaders.toArray();
-        }
+      if (opts.requestHeaders) {
+        data.requestHeaders = requestHeaders.toArray();
+      }
 
-        if (opts.requestBody) {
-          requestBody = requestBody || WebRequestUpload.createRequestBody(channel);
-          data.requestBody = requestBody;
-        }
+      if (opts.responseHeaders && responseHeaders) {
+        data.responseHeaders = responseHeaders.toArray();
+      }
 
-        if (includeStatus) {
-          mergeStatus(data, channel, kind);
-        }
+      if (opts.requestBody) {
+        requestBody = requestBody || WebRequestUpload.createRequestBody(channel);
+        data.requestBody = requestBody;
+      }
 
-        try {
-          let result = callback(data);
+      if (includeStatus) {
+        mergeStatus(data, channel, kind);
+      }
 
-          if (result && typeof result === "object" && blockable && opts.blocking) {
-            handlerResults.push({opts, result});
-          }
-        } catch (e) {
-          Cu.reportError(e);
+      try {
+        let result = callback(data);
+
+        if (result && typeof result === "object" && opts.blocking) {
+          handlerResults.push({opts, result});
         }
+      } catch (e) {
+        Cu.reportError(e);
       }
-    } catch (e) {
-      Cu.reportError(e);
     }
 
-    return this.applyChanges(kind, channel, loadContext, handlerResults, requestHeaders, responseHeaders);
+    this.applyChanges(kind, channel, loadContext, handlerResults, requestHeaders, responseHeaders);
   },
 
   applyChanges: Task.async(function* (kind, channel, loadContext, handlerResults, requestHeaders, responseHeaders) {
     let asyncHandlers = handlerResults.filter(({result}) => isThenable(result));
     let isAsync = asyncHandlers.length > 0;
-    let shouldResume = false;
 
     try {
       if (isAsync) {
-        shouldResume = this.maybeSuspend(channel);
+        channel.suspend();
 
         for (let value of asyncHandlers) {
           try {
             value.result = yield value.result;
           } catch (e) {
             Cu.reportError(e);
             value.result = {};
           }
         }
       }
 
       for (let {opts, result} of handlerResults) {
         if (result.cancel) {
-          this.maybeResume(channel);
           channel.cancel(Cr.NS_ERROR_ABORT);
 
           this.errorCheck(channel, loadContext);
           return;
         }
 
         if (result.redirectUrl) {
           try {
-            this.maybeResume(channel);
+            if (isAsync) {
+              channel.resume();
+            }
 
             channel.redirectTo(BrowserUtils.makeURI(result.redirectUrl));
             return;
           } catch (e) {
             Cu.reportError(e);
           }
         }
 
-        if (opts.requestHeaders && result.requestHeaders && requestHeaders) {
+        if (opts.requestHeaders && result.requestHeaders) {
           requestHeaders.applyChanges(result.requestHeaders);
         }
 
-        if (opts.responseHeaders && result.responseHeaders && responseHeaders) {
+        if (opts.responseHeaders && result.responseHeaders) {
           responseHeaders.applyChanges(result.responseHeaders);
         }
       }
     } catch (e) {
       Cu.reportError(e);
     }
 
+    if (isAsync) {
+      channel.resume();
+    }
 
     if (kind === "opening") {
       return this.runChannelListener(channel, loadContext, "modify");
     } else if (kind === "modify") {
       return this.runChannelListener(channel, loadContext, "afterModify");
     }
-
-    // Only resume the channel if either it was suspended by this call,
-    // and this callback is not part of the opening-modify-afterModify
-    // chain, or if this is an afterModify callback. This allows us to
-    // chain the aforementioned handlers without repeatedly suspending
-    // and resuming the request.
-    if (shouldResume || kind === "afterModify") {
-      this.maybeResume(channel);
-    }
   }),
 
   examine(channel, topic, data) {
     let loadContext = this.getLoadContext(channel);
 
     if (this.needTracing) {
-      // Check whether we've already added a listener to this channel,
-      // so we don't wind up chaining multiple listeners.
-      let channelData = getData(channel);
-      if (!channelData.listener && channel instanceof Ci.nsITraceableChannel) {
+      if (channel instanceof Ci.nsITraceableChannel) {
         let responseStatus = channel.responseStatus;
         // skip redirections, https://bugzilla.mozilla.org/show_bug.cgi?id=728901#c8
         if (responseStatus < 300 || responseStatus >= 400) {
           let listener = new StartStopListener(this, loadContext);
           let orig = channel.setNewListener(listener);
           listener.orig = orig;
-          channelData.listener = listener;
         }
       }
     }
 
     this.runChannelListener(channel, loadContext, "headersReceived");
   },
 
   onChannelReplaced(oldChannel, newChannel) {