Bug 1622887 - Properly support js components implementing an [optional] out nsISupports. r=nika

Differential Revision: https://phabricator.services.mozilla.com/D67024

--HG--
extra : moz-landing-system : lando

js/xpconnect/src/XPCWrappedJSClass.cpp

@@ -558,6 +558,11 @@ void nsXPCWrappedJS::CleanupOutparams(const nsXPTMethodInfo* info,
 
     MOZ_ASSERT(param.IsIndirect(), "Outparams are always indirect");
 
+    // Don't try to clear optional out params that are not set.
+    if (param.IsOptional() && !nativeParams[i].val.p) {
+      continue;
+    }
+
     // Call 'CleanupValue' on parameters which we know to be initialized:
     //  1. Complex parameters (initialized by caller)
     //  2. 'inout' parameters (initialized by caller)
@@ -890,8 +895,8 @@ nsXPCWrappedJS::CallMethod(uint16_t methodIndex, const nsXPTMethodInfo* info,
     uint32_t array_count;
     RootedValue val(cx, NullValue());
 
-    // verify that null was not passed for 'out' param
-    if (param.IsOut() && !nativeParams[i].val.p) {
+    // Verify that null was not passed for a non-optional 'out' param.
+    if (param.IsOut() && !nativeParams[i].val.p && !param.IsOptional()) {
       retval = NS_ERROR_INVALID_ARG;
       goto pre_call_clean_up;
     }
@@ -996,7 +1001,7 @@ pre_call_clean_up:
   for (i = 0; i < paramCount; i++) {
     const nsXPTParamInfo& param = info->GetParam(i);
     MOZ_ASSERT(!param.IsShared(), "[shared] implies [noscript]!");
-    if (!param.IsOut()) {
+    if (!param.IsOut() || !nativeParams[i].val.p) {
       continue;
     }
 

js/xpconnect/tests/components/js/xpctest_params.js

@@ -88,6 +88,17 @@ TestParams.prototype = {
     arr.forEach((x) => rv += x);
     return rv;
   },
+  testOmittedOptionalOut(o) {
+    if (typeof o != "object" || o.value !== undefined) {
+      throw new Components.Exception(
+        "unexpected value",
+        Cr.NS_ERROR_ILLEGAL_VALUE
+      );
+    }
+    o.value = Cc["@mozilla.org/network/io-service;1"]
+      .getService(Ci.nsIIOService)
+      .newURI("http://example.com/");
+  }
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([TestParams]);

js/xpconnect/tests/components/native/xpctest_params.cpp

@@ -6,6 +6,10 @@
 #include "xpctest_interfaces.h"
 #include "js/Value.h"
 
+#include "nsCOMPtr.h"
+#include "nsComponentManagerUtils.h"
+#include "nsIURI.h"
+
 NS_IMPL_ISUPPORTS(nsXPCTestParams, nsIXPCTestParams)
 
 nsXPCTestParams::nsXPCTestParams() = default;
@@ -367,3 +371,28 @@ nsXPCTestParams::TestOptionalSequence(const nsTArray<uint8_t>& aInArr,
   aReturnArr = aInArr;
   return NS_OK;
 }
+
+NS_IMETHODIMP
+nsXPCTestParams::TestOmittedOptionalOut(nsIURI** aOut) {
+  MOZ_ASSERT(!(*aOut), "Unexpected value received");
+  // Call the js component, to check XPConnect won't crash when passing nullptr
+  // as the optional out parameter, and that the out object is built regardless.
+  nsresult rv;
+  nsCOMPtr<nsIXPCTestParams> jsComponent =
+      do_CreateInstance("@mozilla.org/js/xpc/test/js/Params;1", &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+  // Invoke it directly passing nullptr.
+  rv = jsComponent->TestOmittedOptionalOut(nullptr);
+  NS_ENSURE_SUCCESS(rv, rv);
+  // Also invoke it with a ref pointer.
+  nsCOMPtr<nsIURI> someURI;
+  rv = jsComponent->TestOmittedOptionalOut(getter_AddRefs(someURI));
+  NS_ENSURE_SUCCESS(rv, rv);
+  nsAutoCString spec;
+  rv = someURI->GetSpec(spec);
+  if (!spec.EqualsLiteral("http://example.com/")) {
+    return NS_ERROR_UNEXPECTED;
+  }
+  someURI.forget(aOut);
+  return NS_OK;
+}

js/xpconnect/tests/idl/xpctest_params.idl

@@ -12,6 +12,7 @@
 
 #include "nsISupports.idl"
 
+interface nsIURI;
 interface nsIXPCTestInterfaceA;
 interface nsIXPCTestInterfaceB;
 
@@ -108,4 +109,7 @@ interface nsIXPCTestParams : nsISupports {
 
   // Test for optional array size_is.
   ACString             testStringArrayOptionalSize([array, size_is(aLength)] in string a, [optional] in unsigned long aLength);
+
+  // Test for omitted optional out parameter.
+  void                 testOmittedOptionalOut([optional] out nsIURI aOut);
 };

js/xpconnect/tests/unit/test_params.js

@@ -16,6 +16,7 @@ function run_test() {
 function test_component(contractid) {
 
   // Instantiate the object.
+  info("Testing " + contractid);
   var o = Cc[contractid].createInstance(Ci["nsIXPCTestParams"]);
 
   // Possible comparator functions.
@@ -228,4 +229,9 @@ function test_component(contractid) {
   ret = o.testOptionalSequence([1, 2, 3]);
   Assert.ok(Array.isArray(ret));
   Assert.equal(ret.length, 3);
+
+  o.testOmittedOptionalOut();
+  ret = {};
+  o.testOmittedOptionalOut(ret);
+  Assert.equal(ret.value.spec, "http://example.com/")
 }