Bug 1182357 - Implement support for optional size_is for arrays passed from JS. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Sat, 11 Jul 2015 00:21:16 -0400
changeset 253119 86d4a584905c36d6c4ef873cd2a03496a17adde5
parent 253118 909610f2dfc2c101a28a215ffc3e2156c4d7189a
child 253120 71069116ee281ff10618d78097abb21c8bcba3aa
push id29061
push userryanvm@gmail.com
push dateThu, 16 Jul 2015 18:53:45 +0000
treeherdermozilla-central@a0f4a688433d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs1182357
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1182357 - Implement support for optional size_is for arrays passed from JS. r=mrbkap The fact that the caller needs to pass this is just an artifact of the clunky XPIDL type system. This should let us make nicer APIs.
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/tests/components/js/xpctest_params.js
js/xpconnect/tests/components/native/xpctest_params.cpp
js/xpconnect/tests/idl/xpctest_params.idl
js/xpconnect/tests/unit/test_params.js
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1292,17 +1292,17 @@ class MOZ_STACK_CLASS CallMethodHelper
     nsAutoTArray<nsXPTCVariant, 8> mDispatchParams;
     uint8_t mJSContextIndex; // TODO make const
     uint8_t mOptArgcIndex; // TODO make const
 
     jsval* const mArgv;
     const uint32_t mArgc;
 
     MOZ_ALWAYS_INLINE bool
-    GetArraySizeFromParam(uint8_t paramIndex, uint32_t* result) const;
+    GetArraySizeFromParam(uint8_t paramIndex, HandleValue maybeArray, uint32_t* result);
 
     MOZ_ALWAYS_INLINE bool
     GetInterfaceTypeFromParam(uint8_t paramIndex,
                               const nsXPTType& datum_type,
                               nsID* result) const;
 
     MOZ_ALWAYS_INLINE bool
     GetOutParamSource(uint8_t paramIndex, MutableHandleValue srcp) const;
@@ -1441,17 +1441,17 @@ CallMethodHelper::~CallMethodHelper()
                 if (!p)
                     continue;
 
                 // Clean up the array contents if necessary.
                 if (dp->DoesValNeedCleanup()) {
                     // We need some basic information to properly destroy the array.
                     uint32_t array_count = 0;
                     nsXPTType datum_type;
-                    if (!GetArraySizeFromParam(i, &array_count) ||
+                    if (!GetArraySizeFromParam(i, UndefinedHandleValue, &array_count) ||
                         !NS_SUCCEEDED(mIFaceInfo->GetTypeForParam(mVTableIndex,
                                                                   &paramInfo,
                                                                   1, &datum_type))) {
                         // XXXbholley - I'm not convinced that the above calls will
                         // ever fail.
                         NS_ERROR("failed to get array information, we'll leak here");
                         continue;
                     }
@@ -1475,27 +1475,44 @@ CallMethodHelper::~CallMethodHelper()
         }
     }
 
     mCallContext.GetXPCContext()->SetLastResult(mInvokeResult);
 }
 
 bool
 CallMethodHelper::GetArraySizeFromParam(uint8_t paramIndex,
-                                        uint32_t* result) const
+                                        HandleValue maybeArray,
+                                        uint32_t* result)
 {
     nsresult rv;
     const nsXPTParamInfo& paramInfo = mMethodInfo->GetParam(paramIndex);
 
     // TODO fixup the various exceptions that are thrown
 
     rv = mIFaceInfo->GetSizeIsArgNumberForParam(mVTableIndex, &paramInfo, 0, &paramIndex);
     if (NS_FAILED(rv))
         return Throw(NS_ERROR_XPC_CANT_GET_ARRAY_INFO, mCallContext);
 
+    // If the array length wasn't passed, it might have been listed as optional.
+    // When converting arguments from JS to C++, we pass the array as |maybeArray|,
+    // and give ourselves the chance to infer the length. Once we have it, we stick
+    // it in the right slot so that we can find it again when cleaning up the params.
+    // from the array.
+    if (paramIndex >= mArgc && maybeArray.isObject()) {
+        MOZ_ASSERT(mMethodInfo->GetParam(paramIndex).IsOptional());
+        RootedObject arrayOrNull(mCallContext, maybeArray.isObject() ? &maybeArray.toObject()
+                                                                     : nullptr);
+        if (!JS_IsArrayObject(mCallContext, maybeArray) ||
+            !JS_GetArrayLength(mCallContext, arrayOrNull, &GetDispatchParam(paramIndex)->val.u32))
+        {
+            return Throw(NS_ERROR_XPC_CANT_CONVERT_OBJECT_TO_ARRAY, mCallContext);
+        }
+    }
+
     *result = GetDispatchParam(paramIndex)->val.u32;
 
     return true;
 }
 
 bool
 CallMethodHelper::GetInterfaceTypeFromParam(uint8_t paramIndex,
                                             const nsXPTType& datum_type,
@@ -1581,17 +1598,17 @@ CallMethodHelper::GatherAndConvertResult
                                                       &datum_type))) {
                 Throw(NS_ERROR_XPC_CANT_GET_ARRAY_INFO, mCallContext);
                 return false;
             }
         } else
             datum_type = type;
 
         if (isArray || isSizedString) {
-            if (!GetArraySizeFromParam(i, &array_count))
+            if (!GetArraySizeFromParam(i, UndefinedHandleValue, &array_count))
                 return false;
         }
 
         nsID param_iid;
         if (datum_type.IsInterfacePointer() &&
             !GetInterfaceTypeFromParam(i, datum_type, &param_iid))
             return false;
 
@@ -1963,17 +1980,17 @@ CallMethodHelper::ConvertDependentParam(
     nsID param_iid;
     if (datum_type.IsInterfacePointer() &&
         !GetInterfaceTypeFromParam(i, datum_type, &param_iid))
         return false;
 
     nsresult err;
 
     if (isArray || isSizedString) {
-        if (!GetArraySizeFromParam(i, &array_count))
+        if (!GetArraySizeFromParam(i, src, &array_count))
             return false;
 
         if (isArray) {
             if (array_count &&
                 !XPCConvert::JSArray2Native((void**)&dp->val, src,
                                             array_count, datum_type, &param_iid,
                                             &err)) {
                 // XXX need exception scheme for arrays to indicate bad element
--- a/js/xpconnect/tests/components/js/xpctest_params.js
+++ b/js/xpconnect/tests/components/js/xpctest_params.js
@@ -68,12 +68,18 @@ TestParams.prototype = {
   testDoubleArray: f_is,
   testStringArray: f_is,
   testWstringArray: f_is,
   testInterfaceArray: f_is,
   testSizedString: f_is,
   testSizedWstring: f_is,
   testInterfaceIs: f_is,
   testInterfaceIsArray: f_size_and_iid,
-  testOutAString: function(o) { o.value = "out"; }
+  testOutAString: function(o) { o.value = "out"; },
+  testStringArrayOptionalSize: function(arr, size) {
+    if (arr.length != size) { throw "bad size passed to test method"; }
+    var rv = "";
+    arr.forEach((x) => rv += x);
+    return rv;
+  }
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([TestParams]);
--- a/js/xpconnect/tests/components/native/xpctest_params.cpp
+++ b/js/xpconnect/tests/components/native/xpctest_params.cpp
@@ -341,8 +341,21 @@ NS_IMETHODIMP nsXPCTestParams::TestInter
 }
 
 /* void testOutAString (out AString o); */
 NS_IMETHODIMP nsXPCTestParams::TestOutAString(nsAString & o)
 {
     o.AssignLiteral("out");
     return NS_OK;
 }
+
+/*
+ * ACString testStringArrayOptionalSize([array, size_is(aLength)] in string a, [optional] in unsigned long aLength);
+ */
+NS_IMETHODIMP nsXPCTestParams::TestStringArrayOptionalSize(const char * *a, uint32_t length, nsACString& out)
+{
+  out.Truncate();
+  for (uint32_t i = 0; i < length; ++i) {
+    out.Append(a[i]);
+  }
+
+  return NS_OK;
+}
--- a/js/xpconnect/tests/idl/xpctest_params.idl
+++ b/js/xpconnect/tests/idl/xpctest_params.idl
@@ -10,17 +10,17 @@
  * covered by the intersection of return values and inout).
  */
 
 #include "nsISupports.idl"
 
 interface nsIXPCTestInterfaceA;
 interface nsIXPCTestInterfaceB;
 
-[scriptable, uuid(fe2b7433-ac3b-49ef-9344-b67228bfdd46)]
+[scriptable, uuid(812145c7-9fcc-425e-a878-36ad1b7730b7)]
 interface nsIXPCTestParams : nsISupports {
 
   // These types correspond to the ones in typelib.py
   boolean               testBoolean(in boolean a, inout boolean b);
   octet                 testOctet(in octet a, inout octet b);
   short                 testShort(in short a, inout short b);
   long                  testLong(in long a, inout long b);
   long long             testLongLong(in long long a, inout long long b);
@@ -79,9 +79,12 @@ interface nsIXPCTestParams : nsISupports
                                              [array, size_is(aLength), iid_is(aIID)] in nsQIResult a,
                                              inout unsigned long bLength, inout nsIIDPtr bIID,
                                              [array, size_is(bLength), iid_is(bIID)] inout nsQIResult b,
                                              out unsigned long rvLength, out nsIIDPtr rvIID,
                                              [retval, array, size_is(rvLength), iid_is(rvIID)] out nsQIResult rv);
 
   // Test for out dipper parameters
   void                 testOutAString(out AString o);
+
+  // Test for optional array size_is.
+  ACString             testStringArrayOptionalSize([array, size_is(aLength)] in string a, [optional] in unsigned long aLength);
 };
--- a/js/xpconnect/tests/unit/test_params.js
+++ b/js/xpconnect/tests/unit/test_params.js
@@ -183,16 +183,19 @@ function test_component(contractid) {
   doIsTest("testInterfaceIs", makeA(), Ci['nsIXPCTestInterfaceA'],
                               makeB(), Ci['nsIXPCTestInterfaceB'],
                               interfaceComparator, dotEqualsComparator);
 
   // Test arrays of iids.
   doIs2Test("testInterfaceIsArray", [makeA(), makeA(), makeA(), makeA(), makeA()], 5, Ci['nsIXPCTestInterfaceA'],
                                     [makeB(), makeB(), makeB()], 3, Ci['nsIXPCTestInterfaceB']);
 
+  // Test optional array size.
+  do_check_eq(o.testStringArrayOptionalSize(["some", "string", "array"]), "somestringarray");
+
   // Test incorrect (too big) array size parameter; this should throw NOT_ENOUGH_ELEMENTS.
   doTypedArrayMismatchTest("testShortArray", new Int16Array([-3, 7, 4]), 4,
                                              new Int16Array([1, -32, 6]), 3);
 
   // Test type mismatch (int16 <-> uint16); this should throw BAD_CONVERT_JS.
   doTypedArrayMismatchTest("testShortArray", new Uint16Array([0, 7, 4, 3]), 4,
                                              new Uint16Array([1, 5, 6]), 3);
 }