From b59a6cfc2afc346e86ff9811e4762283c2986219 Mon Sep 17 00:00:00 2001 From: Osei Fortune Date: Mon, 28 Sep 2026 20:07:48 -0400 Subject: [PATCH] feat: null and numbers for IReference parameters and results IReference parameters didn't accept null: null and undefined weren't passed as a null reference, and numbers were passed as the boxed IPropertyValue rather than its IReference interface. APIs that treat a missing value as "leave unchanged", such as ScrollViewer.ChangeView, got 0 instead. null and undefined now pass a null reference. Numbers are boxed with the typed Create* call and queried for the IReference IID, and an existing boxed value (NSWinRT.interop.reference) is passed through the same way. IReference results and property values come back as the number (or struct) they hold, or null. Both the classic and the Node-API engines handle it. --- runtime/src/interop_test.rs | 31 ++++++++++++++ runtime/src/lib.rs | 40 ++++++++++++++++++ runtime/src/method_call.rs | 39 +++++++++++------ runtime/src/napi_engine/invoke.rs | 13 ++++++ runtime/src/napi_engine/value.rs | 31 ++++++++++++-- runtime/src/ns_proxy.rs | 17 ++++++++ runtime/src/property_call.rs | 24 ++++++----- runtime/src/value.rs | 70 +++++++++++++++++++++++++++++-- 8 files changed, 234 insertions(+), 31 deletions(-) diff --git a/runtime/src/interop_test.rs b/runtime/src/interop_test.rs index 1f7419b..3648dbd 100644 --- a/runtime/src/interop_test.rs +++ b/runtime/src/interop_test.rs @@ -2577,3 +2577,34 @@ fn js_delegate_without_value_types_keeps_the_shared_invoke() { assert!(std::ptr::eq(unsafe { (*delegate).vtable }, &crate::JS_DELEGATE_VTBL)); unsafe { crate::js_delegate_release(delegate) }; } + +/// `IReference` parameters take a JS number or null, and `IReference` results come +/// back as the number or null. +#[test] +fn ireference_parameters_and_results() { + run_js_assert( + "ireference_parameters_and_results", + r#" + const formatter = new Windows.Globalization.NumberFormatting.DecimalFormatter(); + const parsed = formatter.ParseDouble('1.5'); + if (parsed !== 1.5) throw new Error(`ParseDouble('1.5') returned ${parsed}`); + const invalid = formatter.ParseDouble('not a number'); + if (invalid !== null) throw new Error(`ParseDouble of junk returned ${invalid}`); + const int = formatter.ParseInt('42'); + if (int !== 42) throw new Error(`ParseInt('42') returned ${int}`); + + const date = new Windows.ApplicationModel.Contacts.ContactDate(); + if (date.Day !== null) throw new Error(`unset Day was ${date.Day}`); + date.Day = 12; + date.Year = -5; + if (date.Day !== 12) throw new Error(`Day was ${date.Day}`); + if (date.Year !== -5) throw new Error(`Year was ${date.Year}`); + date.Day = null; + if (date.Day !== null) throw new Error(`cleared Day was ${date.Day}`); + date.Year = undefined; + if (date.Year !== null) throw new Error(`cleared Year was ${date.Year}`); + date.Month = NSWinRT.interop.reference('UInt32', 7); + if (date.Month !== 7) throw new Error(`boxed Month was ${date.Month}`); + "#, + ); +} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 671479c..c263d66 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -992,12 +992,44 @@ pub(crate) enum ReturnKind { }, /// Return type is `Object`/IInspectable: concrete type only known at runtime. DynamicObject, + /// `IReference` return: `get_Value` is read into a `size`-byte buffer and converted as + /// `value`; a null reference is JS `null`. + Reference { + value: Box, + size: usize, + }, +} + +fn classify_reference_return(inner: &str) -> Option { + use crate::value::NativeType; + let value = classify_return(inner, false); + let size = match &value { + ReturnKind::Primitive( + NativeType::Void + | NativeType::Pointer + | NativeType::Buffer + | NativeType::Function + | NativeType::Struct(_), + ) => return None, + ReturnKind::Primitive(nt) => nt.size(), + ReturnKind::Struct(_) => crate::helpers::struct_native_type_for_sig(inner)?.size(), + _ => return None, + }; + Some(ReturnKind::Reference { + value: Box::new(value), + size, + }) } pub(crate) fn classify_return(return_type: &str, is_void: bool) -> ReturnKind { if is_void { return ReturnKind::Void; } + if let Some(kind) = + crate::helpers::ireference_inner_type(return_type).and_then(classify_reference_return) + { + return kind; + } if return_type == "Guid" { return ReturnKind::Guid; } @@ -1094,6 +1126,14 @@ pub(crate) fn return_value_from_kind<'a>( } } } + ReturnKind::Reference { value, size } => { + match unsafe { crate::value::read_reference_value(result, *size) } { + Some(mut buf) => { + return_value_from_kind(value, buf.as_mut_ptr() as *mut c_void, None, scope) + } + None => v8::null(scope).into(), + } + } ReturnKind::Primitive(nt) => match nt { NativeType::Pointer => { if result.is_null() { diff --git a/runtime/src/method_call.rs b/runtime/src/method_call.rs index 6462494..d2be21b 100644 --- a/runtime/src/method_call.rs +++ b/runtime/src/method_call.rs @@ -49,8 +49,9 @@ pub(crate) enum PointerPlan { /// Plain pointer parse: non-WinRT signature, unresolvable type, or a resolvable kind /// that takes no special handling. Plain, - /// `IReference` parameter — box primitives via the typed Create* call (inner type name). - IReference(String), + /// `IReference` parameter — box primitives via the typed Create* call (inner type name) + /// and pass the `IReference` interface (IID). + IReference(String, GUID), /// `Windows.UI.Xaml.Interop.TypeName` struct — synthesize {Name, Kind} from a class ctor. TypeName, /// Other struct parameter — serialize field-by-field (declaration pre-resolved). @@ -78,7 +79,15 @@ impl PointerPlan { typename_special: bool, ) -> Self { if let Some(inner) = crate::helpers::ireference_inner_type(signature) { - return PointerPlan::IReference(inner.to_string()); + let name = signature.trim(); + let name = name.strip_prefix("ByRef ").unwrap_or(name); + let iid = GenericInstanceIdBuilder::generate_id_from_name( + &crate::property_call::substitute_type_vars(name, type_args), + ); + return PointerPlan::IReference( + crate::property_call::substitute_type_vars(inner, type_args), + iid, + ); } if !signature.contains('.') { return PointerPlan::Plain; @@ -993,11 +1002,13 @@ impl MethodCall { PointerPlan::Plain => ffi_parse_pointer_arg(scope, value), // IReference parameters: box JS primitives with the correct Create* call // so XAML receives the right typed IPropertyValue (e.g. IReference). - PointerPlan::IReference(inner) => { - if let Some(nv) = crate::value::box_as_ireference(scope, value, inner) { - Ok(nv) - } else { - ffi_parse_pointer_arg(scope, value) + PointerPlan::IReference(inner, iid) => { + match crate::value::box_as_ireference(scope, value, inner, iid) { + Some((nv, guard)) => { + queried_interfaces.extend(guard); + Ok(nv) + } + None => ffi_parse_pointer_arg(scope, value), } } PointerPlan::TypeName => { @@ -1625,12 +1636,14 @@ impl MethodCall { // resolved once into the per-parameter plan when the static info was built. match &self.si.param_plans[i] { PointerPlan::Plain => nv::napi_parse_pointer(env, &value), - PointerPlan::IReference(inner) => { + PointerPlan::IReference(inner, iid) => { // IReference: box primitives with the correct typed Create* call. - if let Some(nvv) = nv::box_as_ireference(env, &value, inner) { - Ok(nvv) - } else { - nv::napi_parse_pointer(env, &value) + match nv::box_as_ireference(env, &value, inner, iid) { + Some((nvv, guard)) => { + queried_interfaces.extend(guard); + Ok(nvv) + } + None => nv::napi_parse_pointer(env, &value), } } PointerPlan::TypeName => { diff --git a/runtime/src/napi_engine/invoke.rs b/runtime/src/napi_engine/invoke.rs index 3a3c3ca..3cc1106 100644 --- a/runtime/src/napi_engine/invoke.rs +++ b/runtime/src/napi_engine/invoke.rs @@ -103,6 +103,19 @@ pub(crate) fn convert_call_result( .map_err(|e| generic_error(e.to_string()))?; return Ok(nv::as_unknown(env, proxy)); } + ReturnKind::Reference { value, size } => { + let inner = crate::helpers::ireference_inner_type(return_type).unwrap_or(return_type); + return match unsafe { crate::value::read_reference_value(result, *size) } { + Some(mut buf) => { + let ptr = buf.as_mut_ptr() as *mut c_void; + convert_call_result(env, value, false, inner, hr, ptr) + } + None => { + let n = env.get_null().map_err(|e| type_error(e.to_string()))?; + Ok(nv::as_unknown(env, n)) + } + }; + } ReturnKind::Primitive(nt) => match nt { NativeType::Pointer | NativeType::Buffer diff --git a/runtime/src/napi_engine/value.rs b/runtime/src/napi_engine/value.rs index 654090e..b451e43 100644 --- a/runtime/src/napi_engine/value.rs +++ b/runtime/src/napi_engine/value.rs @@ -851,10 +851,33 @@ pub fn box_as_typed_value(env: &Env, arg: &JsUnknown, type_name: &str) -> Option } } -/// Alias used by method_call / property_call for IReference params. -#[inline] -pub fn box_as_ireference(env: &Env, arg: &JsUnknown, inner_type: &str) -> Option { - box_as_typed_value(env, arg, inner_type) +/// Marshal a JS value for an `IReference` parameter. See `crate::value::box_as_ireference`. +pub fn box_as_ireference( + env: &Env, + arg: &JsUnknown, + inner_type: &str, + iid: &GUID, +) -> Option<(NativeValue, Option)> { + let vt = arg.get_type().ok()?; + if vt == ValueType::Null || vt == ValueType::Undefined { + return Some(( + NativeValue { + pointer: std::ptr::null_mut(), + }, + None, + )); + } + let wrapped = if vt == ValueType::Object || vt == ValueType::Function { + let obj: JsObject = unsafe { arg.cast() }; + try_get_external_handle(env, &obj).filter(|ptr| !ptr.is_null()) + } else { + None + }; + let boxed = match wrapped { + Some(ptr) => (*ManuallyDrop::new(unsafe { IUnknown::from_raw(ptr) })).clone(), + None => unsafe { IUnknown::from_raw(box_as_typed_value(env, arg, inner_type)?.pointer) }, + }; + Some(crate::value::query_reference(boxed, iid)) } // diff --git a/runtime/src/ns_proxy.rs b/runtime/src/ns_proxy.rs index e8e857c..c4c5137 100644 --- a/runtime/src/ns_proxy.rs +++ b/runtime/src/ns_proxy.rs @@ -698,6 +698,11 @@ pub(crate) fn handle_named_property_getter( .return_kind() { ReturnKind::Void => None, + kind @ ReturnKind::Reference { .. } => { + Some(crate::return_value_from_kind( + kind, result, None, scope, + )) + } ReturnKind::Guid => { let obj = unsafe { crate::guid_ptr_to_js_object(result, scope) @@ -1023,6 +1028,9 @@ fn instance_method_dispatch( let return_value_opt: Option> = match method.return_kind() { ReturnKind::Void => None, + kind @ ReturnKind::Reference { .. } => { + Some(crate::return_value_from_kind(kind, result, None, scope)) + } ReturnKind::Guid => { let obj = unsafe { crate::guid_ptr_to_js_object(result, scope) }; Some(obj.into()) @@ -1308,6 +1316,9 @@ pub(crate) fn handle_instance_property_getter( let ret_val: Option> = match property_call.return_kind() { ReturnKind::Void => None, + kind @ ReturnKind::Reference { .. } => { + Some(crate::return_value_from_kind(kind, result, None, scope)) + } ReturnKind::Guid => { let obj = unsafe { crate::guid_ptr_to_js_object(result, scope) }; Some(obj.into()) @@ -2120,6 +2131,9 @@ pub(crate) fn create_ns_ctor_instance_object<'a>( } else if !method.is_void() { let ret_v: Option> = match method.return_kind() { ReturnKind::Void => None, + kind @ ReturnKind::Reference { .. } => { + Some(crate::return_value_from_kind(kind, result, None, scope)) + } ReturnKind::Guid => { let obj = unsafe { guid_ptr_to_js_object(result, scope) }; Some(obj.into()) @@ -2268,6 +2282,9 @@ pub(crate) fn create_ns_ctor_instance_object<'a>( } else if !method.is_void() { let ret_v: Option> = match method.return_kind() { ReturnKind::Void => None, + kind @ ReturnKind::Reference { .. } => Some( + crate::return_value_from_kind(kind, result, None, scope), + ), ReturnKind::Guid => { let obj = unsafe { guid_ptr_to_js_object(result, scope) }; Some(obj.into()) diff --git a/runtime/src/property_call.rs b/runtime/src/property_call.rs index ebd7916..b06131f 100644 --- a/runtime/src/property_call.rs +++ b/runtime/src/property_call.rs @@ -1259,11 +1259,13 @@ impl PropertyCall { } // IReference parameters: box JS primitives with the correct Create* call // so XAML receives the right typed IPropertyValue (e.g. IReference). - PointerPlan::IReference(inner) => { - if let Some(nv) = crate::value::box_as_ireference(scope, value, inner) { - Ok(nv) - } else { - ffi_parse_pointer_arg(scope, value) + PointerPlan::IReference(inner, iid) => { + match crate::value::box_as_ireference(scope, value, inner, iid) { + Some((nv, guard)) => { + queried_interfaces.extend(guard); + Ok(nv) + } + None => ffi_parse_pointer_arg(scope, value), } } PointerPlan::Struct(declaration) => { @@ -1651,11 +1653,13 @@ impl PropertyCall { PointerPlan::Plain | PointerPlan::TypeName => { nv::napi_parse_pointer(env, &value) } - PointerPlan::IReference(inner) => { - if let Some(nvv) = nv::box_as_ireference(env, &value, inner) { - Ok(nvv) - } else { - nv::napi_parse_pointer(env, &value) + PointerPlan::IReference(inner, iid) => { + match nv::box_as_ireference(env, &value, inner, iid) { + Some((nvv, guard)) => { + queried_interfaces.extend(guard); + Ok(nvv) + } + None => nv::napi_parse_pointer(env, &value), } } PointerPlan::Struct(declaration) => { diff --git a/runtime/src/value.rs b/runtime/src/value.rs index a7ebcd4..696f1eb 100644 --- a/runtime/src/value.rs +++ b/runtime/src/value.rs @@ -1084,15 +1084,77 @@ pub fn box_as_typed_value( } } -/// Keep the old name as an alias — used by method_call / property_call for IReference params. +/// Marshal a JS value for an `IReference` parameter: `null`/`undefined` is a null reference; +/// anything else is boxed as `inner_type` (or taken from its wrapper) and queried for `iid`. +/// The returned guard keeps the reference alive for the call. #[cfg(feature = "classic")] -#[inline] pub fn box_as_ireference( scope: &mut v8::PinScope<'_, '_>, arg: v8::Local, inner_type: &str, -) -> Option { - box_as_typed_value(scope, arg, inner_type) + iid: &GUID, +) -> Option<(NativeValue, Option)> { + if arg.is_null_or_undefined() { + return Some(( + NativeValue { + pointer: std::ptr::null_mut(), + }, + None, + )); + } + let wrapped = if arg.is_object() { + arg.to_object(scope) + .and_then(|obj| try_get_external_handle(scope, obj)) + .filter(|ptr| !ptr.is_null()) + } else { + None + }; + let boxed = match wrapped { + Some(ptr) => (*ManuallyDrop::new(unsafe { IUnknown::from_raw(ptr) })).clone(), + None => unsafe { IUnknown::from_raw(box_as_typed_value(scope, arg, inner_type)?.pointer) }, + }; + Some(query_reference(boxed, iid)) +} + +/// QI an owned boxed value for an `IReference` IID, falling back to the value itself. +pub(crate) fn query_reference( + boxed: windows::core::IUnknown, + iid: &windows::core::GUID, +) -> (NativeValue, Option) { + use windows::core::Interface; + let mut queried: *mut c_void = std::ptr::null_mut(); + if unsafe { boxed.query(iid, &mut queried) }.is_ok() && !queried.is_null() { + let queried = unsafe { windows::core::IUnknown::from_raw(queried) }; + return ( + NativeValue { + pointer: queried.as_raw(), + }, + Some(queried), + ); + } + ( + NativeValue { + pointer: boxed.as_raw(), + }, + Some(boxed), + ) +} + +/// Read an `IReference` return through its `get_Value` into a buffer of at least `size` +/// bytes, releasing the reference. `None` for a null reference or a failed read. +pub(crate) unsafe fn read_reference_value(reference: *mut c_void, size: usize) -> Option> { + use windows::core::Interface; + if reference.is_null() { + return None; + } + let reference = windows::core::IUnknown::from_raw(reference); + type GetValue = unsafe extern "system" fn(*mut c_void, *mut c_void) -> windows::core::HRESULT; + let vtable = *(reference.as_raw() as *const *const usize); + let get_value: GetValue = std::mem::transmute(*vtable.add(6)); + let mut buf = vec![0u64; size.div_ceil(8).max(2)]; + get_value(reference.as_raw(), buf.as_mut_ptr() as *mut c_void) + .is_ok() + .then_some(buf) } /// Parse "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" into a GUID.