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
--- 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]