Bug 1351443 - Only fall back to NewChannel if implementation of NewChannel2 is missing r=mcmanus draft
authorValentin Gosu <valentin.gosu@gmail.com>
Fri, 07 Apr 2017 15:25:19 +0300
changeset 558253 5d41df330a15ad7b679d9bed52e6fbf90bdc4ed8
parent 557546 422bd63b18bc5b11482255aaaef1826285309233
child 623177 0510adcab5b68a08453fcfbeec535bf19dada58e
push id52851
push uservalentin.gosu@gmail.com
push dateFri, 07 Apr 2017 12:25:46 +0000
reviewersmcmanus
bugs1351443
milestone55.0a1
Bug 1351443 - Only fall back to NewChannel if implementation of NewChannel2 is missing r=mcmanus If the call to NewChannel2 returns NS_ERROR_NOT_IMPLEMENTED or NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED that means the implementation is actually missing, so it is OK to fall back to NewChannel. If it fails with any other error code, we just return it. MozReview-Commit-ID: JmgEmPqu6zJ
netwerk/base/nsIOService.cpp
netwerk/test/unit/test_1351443-missing-NewChannel2.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/base/nsIOService.cpp
+++ b/netwerk/base/nsIOService.cpp
@@ -786,27 +786,32 @@ nsIOService::NewChannelFromURIWithProxyF
             // The protocol handler does not implement NewProxiedChannel2, so
             // maybe we need to wrap the channel (see comment in MaybeWrap
             // function).
             channel = nsSecCheckWrapChannel::MaybeWrap(channel, aLoadInfo);
         }
     }
     else {
         rv = handler->NewChannel2(aURI, aLoadInfo, getter_AddRefs(channel));
-        // if calling newChannel2() fails we try to fall back to
+        // if an implementation of NewChannel2() is missing we try to fall back to
         // creating a new channel by calling NewChannel().
-        if (NS_FAILED(rv)) {
+        if (rv == NS_ERROR_NOT_IMPLEMENTED ||
+            rv == NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED) {
+            LOG(("NewChannel2 not implemented rv=%" PRIx32
+                 ". Falling back to NewChannel\n", static_cast<uint32_t>(rv)));
             rv = handler->NewChannel(aURI, getter_AddRefs(channel));
             if (NS_FAILED(rv)) {
                 return rv;
             }
             // The protocol handler does not implement NewChannel2, so
             // maybe we need to wrap the channel (see comment in MaybeWrap
             // function).
             channel = nsSecCheckWrapChannel::MaybeWrap(channel, aLoadInfo);
+        } else if (NS_FAILED(rv)) {
+            return rv;
         }
     }
 
     // Make sure that all the individual protocolhandlers attach a loadInfo.
     if (aLoadInfo) {
       // make sure we have the same instance of loadInfo on the newly created channel
       nsCOMPtr<nsILoadInfo> loadInfo = channel->GetLoadInfo();
       if (aLoadInfo != loadInfo) {
copy from netwerk/test/unit/test_bug894586.js
copy to netwerk/test/unit/test_1351443-missing-NewChannel2.js
--- a/netwerk/test/unit/test_bug894586.js
+++ b/netwerk/test/unit/test_1351443-missing-NewChannel2.js
@@ -1,13 +1,15 @@
 /*
- * Tests for bug 894586: nsSyncLoadService::PushSyncStreamToListener
- * should not fail for channels of unknown size
+ * Tests for bug 1351443: NewChannel2 should only fallback to NewChannel if
+ * NewChannel2() is unimplemented.
  */
 
+"use strict";
+
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/NetUtil.jsm");
 
 var contentSecManager = Cc["@mozilla.org/contentsecuritymanager;1"]
                           .getService(Ci.nsIContentSecurityManager);
 
 function ProtocolHandler() {
@@ -15,17 +17,17 @@ function ProtocolHandler() {
                createInstance(Ci.nsIURI);
   this.uri.spec = this.scheme + ":dummy";
   this.uri.QueryInterface(Ci.nsIMutable).mutable = false;
 }
 
 ProtocolHandler.prototype = {
   /** nsIProtocolHandler */
   get scheme() {
-    return "x-bug894586";
+    return "x-1351443";
   },
   get defaultPort() {
     return -1;
   },
   get protocolFlags() {
     return Ci.nsIProtocolHandler.URI_NORELATIVE |
            Ci.nsIProtocolHandler.URI_NOAUTH |
            Ci.nsIProtocolHandler.URI_IS_UI_RESOURCE |
@@ -35,16 +37,22 @@ ProtocolHandler.prototype = {
   },
   newURI: function(aSpec, aOriginCharset, aBaseURI) {
     return this.uri;
   },
   newChannel2: function(aURI, aLoadInfo) {
     this.loadInfo = aLoadInfo;
     return this;
   },
+  newChannel2_not_implemented: function(aURI, aLoadInfo) {
+    throw Cr.NS_ERROR_NOT_IMPLEMENTED;
+  },
+  newChannel2_failure: function(aURI, aLoadInfo) {
+    throw Cr.NS_ERROR_FAILURE;
+  },
   newChannel: function(aURI) {
     return this;
   },
   allowPort: function(port, scheme) {
     return port != -1;
   },
 
   /** nsIChannel */
@@ -124,35 +132,62 @@ ProtocolHandler.prototype = {
   },
   lockFactory: function() {},
 
   /** nsISupports */
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIProtocolHandler,
                                          Ci.nsIRequest,
                                          Ci.nsIChannel,
                                          Ci.nsIFactory]),
-  classID: Components.ID("{16d594bc-d9d8-47ae-a139-ea714dc0c35c}")
+  classID: Components.ID("{accbaf4a-2fd9-47e7-8aad-8c19fc5265b5}")
 };
 
 /**
  * Attempt a sync load; we use the stylesheet service to do this for us,
  * based on the knowledge that it forces a sync load under the hood.
  */
 function run_test()
 {
   var handler = new ProtocolHandler();
   var registrar = Components.manager.
                              QueryInterface(Ci.nsIComponentRegistrar);
   registrar.registerFactory(handler.classID, "",
                             "@mozilla.org/network/protocol;1?name=" + handler.scheme,
                             handler);
-  try {
-    var ss = Cc["@mozilla.org/content/style-sheet-service;1"].
-               getService(Ci.nsIStyleSheetService);
-    ss.loadAndRegisterSheet(handler.uri, Ci.nsIStyleSheetService.AGENT_SHEET);
-    do_check_true(ss.sheetRegistered(handler.uri, Ci.nsIStyleSheetService.AGENT_SHEET));
-  } finally {
-    registrar.unregisterFactory(handler.classID, handler);
-  }
+
+  // The default implementation of NewChannel2 should work.
+  var channel = NetUtil.newChannel({
+    uri: handler.uri,
+    loadUsingSystemPrincipal: true
+  });
+  ok(channel, "channel exists");
+  channel = null;
+
+  // If the method throws NS_ERROR_NOT_IMPLEMENTED, it should fall back on newChannel()
+  handler.newChannel2 = handler.newChannel2_not_implemented;
+  channel = NetUtil.newChannel({
+    uri: handler.uri,
+    loadUsingSystemPrincipal: true
+  });
+  ok(channel, "channel exists");
+  channel = null;
+
+  // If the method is not defined (the error code will be
+  // NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED) it should fall back on newChannel()
+  handler.newChannel2 = undefined;
+  channel = NetUtil.newChannel({
+    uri: handler.uri,
+    loadUsingSystemPrincipal: true
+  });
+  ok(channel, "channel exists");
+  channel = null;
+
+  // If the method simply throws an error code, it should not fall back on newChannel()
+  // so we make sure it throws.
+  handler.newChannel2 = handler.newChannel2_failure;
+  Assert.throws(() => {
+    channel = NetUtil.newChannel({
+      uri: handler.uri,
+      loadUsingSystemPrincipal: true
+    });
+  }, /Failure/, "If the channel returns an error code, it should throw");
+  ok(!channel, "channel should not exist");
 }
-
-// vim: set et ts=2 :
-
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -382,9 +382,10 @@ skip-if = os == "android"
 [test_throttlechannel.js]
 [test_throttling.js]
 [test_separate_connections.js]
 [test_rusturl.js]
 [test_trackingProtection_annotateChannels.js]
 [test_race_cache_network.js]
 [test_channel_priority.js]
 [test_bug1312774_http1.js]
+[test_1351443-missing-NewChannel2.js]
 [test_bug1312782_http1.js]