From 184a8111675e89ac0349b3cfb14ba23645e4765e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lder=20Gon=C3=A7alves?= Date: Fri, 14 Aug 2026 16:49:45 +0100 Subject: [PATCH 1/5] RDEV-10097 - Bail out of unbound native object proxy calls Behind BailOutOnUnboundNativeObjectCalls, view property proxies no-op when the native object is missing or CefGlue rejects with "was not found", so teardown/remount races no longer surface as uncaught errors. Bump to 5.120.7. Co-authored-by: Cursor --- Directory.Build.props | 2 +- ReactViewControl/ReactView.cs | 2 +- ReactViewControl/ReactViewFactory.cs | 7 +++ .../ReactViewRender.LoaderModule.cs | 4 +- ReactViewControl/ReactViewRender.cs | 6 +- ReactViewResources/Loader/Internal/Flags.ts | 9 +++ .../Loader/Internal/ViewPropertiesProxy.ts | 55 ++++++++++++++++--- ReactViewResources/Loader/Loader.ts | 6 +- 8 files changed, 75 insertions(+), 16 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index e30e7bfb..ce01d0cb 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -5,7 +5,7 @@ 2.0.0.0 2.0.0.0 - 5.120.6 + 5.120.7 OutSystems ReactView Copyright © OutSystems 2023 diff --git a/ReactViewControl/ReactView.cs b/ReactViewControl/ReactView.cs index 34924c25..24eb45d8 100644 --- a/ReactViewControl/ReactView.cs +++ b/ReactViewControl/ReactView.cs @@ -19,7 +19,7 @@ public abstract partial class ReactView : IDisposable { private static ReactViewRender CreateReactViewInstance(ReactViewFactory factory) { ReactViewRender InnerCreateView() { - var view = new ReactViewRender(factory.DefaultStyleSheet, () => factory.InitializePlugins(), factory.EnableViewPreload, factory.EnableDebugMode, factory.EnsureInnerViewsAreDisposed, factory.LoadScriptsOncePerDocument, factory.EnsureViewPluginsAreDisposed); + var view = new ReactViewRender(factory.DefaultStyleSheet, () => factory.InitializePlugins(), factory.EnableViewPreload, factory.EnableDebugMode, factory.EnsureInnerViewsAreDisposed, factory.LoadScriptsOncePerDocument, factory.EnsureViewPluginsAreDisposed, factory.BailOutOnUnboundNativeObjectCalls); if (factory.ShowDeveloperTools) { view.ShowDeveloperTools(); } diff --git a/ReactViewControl/ReactViewFactory.cs b/ReactViewControl/ReactViewFactory.cs index f92db495..1bf8bfa7 100644 --- a/ReactViewControl/ReactViewFactory.cs +++ b/ReactViewControl/ReactViewFactory.cs @@ -47,5 +47,12 @@ public class ReactViewFactory { /// behaviour, where only the host released them. /// public virtual bool EnsureViewPluginsAreDisposed => true; + + /// + /// Calls through the view properties proxy bail out when the native object is missing or was + /// unregistered (typical remount / teardown races). Set to false to restore the previous behaviour, + /// where those calls surface as uncaught errors. + /// + public virtual bool BailOutOnUnboundNativeObjectCalls => true; } } diff --git a/ReactViewControl/ReactViewRender.LoaderModule.cs b/ReactViewControl/ReactViewRender.LoaderModule.cs index 56742276..d41155c1 100644 --- a/ReactViewControl/ReactViewRender.LoaderModule.cs +++ b/ReactViewControl/ReactViewRender.LoaderModule.cs @@ -22,7 +22,7 @@ public LoaderModule(ReactViewRender viewRender) { /// /// Loads the specified react component into the specified frame /// - public void LoadComponent(IViewModule component, string frameName, bool hasStyleSheet, bool hasPlugins, bool ensureDisposeInnerViews, bool loadScriptsOncePerDocument, bool ensureViewPluginsAreDisposed) { + public void LoadComponent(IViewModule component, string frameName, bool hasStyleSheet, bool hasPlugins, bool ensureDisposeInnerViews, bool loadScriptsOncePerDocument, bool ensureViewPluginsAreDisposed, bool bailOutOnUnboundNativeObjectCalls) { var mainSource = ViewRender.ToFullUrl(NormalizeUrl(component.MainJsSource)); var dependencySources = component.DependencyJsSources.Select(s => ViewRender.ToFullUrl(NormalizeUrl(s))).ToArray(); var cssSources = component.CssSources.Select(s => ViewRender.ToFullUrl(NormalizeUrl(s))).ToArray(); @@ -46,6 +46,7 @@ public void LoadComponent(IViewModule component, string frameName, bool hasStyle // ensureDisposeInnerViews: boolean // loadScriptsOncePerDocument: boolean // ensureViewPluginsAreDisposed: boolean + // bailOutOnUnboundNativeObjectCalls: boolean var loadArgs = new[] { JavascriptSerializer.Serialize(component.Name), @@ -62,6 +63,7 @@ public void LoadComponent(IViewModule component, string frameName, bool hasStyle JavascriptSerializer.Serialize(ensureDisposeInnerViews), JavascriptSerializer.Serialize(loadScriptsOncePerDocument), JavascriptSerializer.Serialize(ensureViewPluginsAreDisposed), + JavascriptSerializer.Serialize(bailOutOnUnboundNativeObjectCalls), }; ExecuteLoaderFunction("loadComponent", loadArgs); diff --git a/ReactViewControl/ReactViewRender.cs b/ReactViewControl/ReactViewRender.cs index 2d677267..ba1c21f9 100644 --- a/ReactViewControl/ReactViewRender.cs +++ b/ReactViewControl/ReactViewRender.cs @@ -39,11 +39,13 @@ internal partial class ReactViewRender : IChildViewHost, IDisposable { private readonly bool ensureDisposeInnerViews; private readonly bool loadScriptsOncePerDocument; private readonly bool ensureViewPluginsAreDisposed; + private readonly bool bailOutOnUnboundNativeObjectCalls; - public ReactViewRender(ResourceUrl defaultStyleSheet, Func initializePlugins, bool preloadWebView, bool enableDebugMode, bool ensureInnerViewsAreDisposed, bool loadScriptsOncePerDocument = true, bool ensureViewPluginsAreDisposed = true) { + public ReactViewRender(ResourceUrl defaultStyleSheet, Func initializePlugins, bool preloadWebView, bool enableDebugMode, bool ensureInnerViewsAreDisposed, bool loadScriptsOncePerDocument = true, bool ensureViewPluginsAreDisposed = true, bool bailOutOnUnboundNativeObjectCalls = true) { this.ensureDisposeInnerViews = ensureInnerViewsAreDisposed; this.loadScriptsOncePerDocument = loadScriptsOncePerDocument; this.ensureViewPluginsAreDisposed = ensureViewPluginsAreDisposed; + this.bailOutOnUnboundNativeObjectCalls = bailOutOnUnboundNativeObjectCalls; UserCallingAssembly = GetUserCallingMethod().ReflectedType.Assembly; // must useSharedDomain for the local storage to be shared @@ -278,7 +280,7 @@ private void TryLoadComponent(FrameInfo frame) { RegisterNativeObject(frame.Component, frame); - Loader.LoadComponent(frame.Component, frame.Name, DefaultStyleSheet != null, frame.Plugins.Length > 0, ensureDisposeInnerViews, loadScriptsOncePerDocument, ensureViewPluginsAreDisposed); + Loader.LoadComponent(frame.Component, frame.Name, DefaultStyleSheet != null, frame.Plugins.Length > 0, ensureDisposeInnerViews, loadScriptsOncePerDocument, ensureViewPluginsAreDisposed, bailOutOnUnboundNativeObjectCalls); if (isInputDisabled && frame.IsMain) { Loader.DisableMouseInteractions(); } diff --git a/ReactViewResources/Loader/Internal/Flags.ts b/ReactViewResources/Loader/Internal/Flags.ts index 213d9bb4..22d64f5e 100644 --- a/ReactViewResources/Loader/Internal/Flags.ts +++ b/ReactViewResources/Loader/Internal/Flags.ts @@ -2,6 +2,7 @@ // module depends on react, and bootstrap reads flags before react has been defined const LoadScriptsOncePerDocumentFlagKey = "LOAD_SCRIPTS_ONCE_PER_DOCUMENT"; const EnsureViewPluginsAreDisposedFlagKey = "ENSURE_VIEW_PLUGINS_ARE_DISPOSED"; +const BailOutOnUnboundNativeObjectCallsFlagKey = "BAIL_OUT_ON_UNBOUND_NATIVE_OBJECT_CALLS"; export function getLoadScriptsOncePerDocumentFlag(): boolean { return !!window[LoadScriptsOncePerDocumentFlagKey]; @@ -18,3 +19,11 @@ export function getEnsureViewPluginsAreDisposedFlag(): boolean { export function setEnsureViewPluginsAreDisposedFlag(ensureViewPluginsAreDisposed: boolean): void { window[EnsureViewPluginsAreDisposedFlagKey] = ensureViewPluginsAreDisposed; } + +export function getBailOutOnUnboundNativeObjectCallsFlag(): boolean { + return !!window[BailOutOnUnboundNativeObjectCallsFlagKey]; +} + +export function setBailOutOnUnboundNativeObjectCallsFlag(bailOutOnUnboundNativeObjectCalls: boolean): void { + window[BailOutOnUnboundNativeObjectCallsFlagKey] = bailOutOnUnboundNativeObjectCalls; +} diff --git a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts index e5a60ae7..9bda287f 100644 --- a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts +++ b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts @@ -1,4 +1,5 @@ import { bindNativeObject } from "./NativeAPI"; +import { getBailOutOnUnboundNativeObjectCallsFlag } from "./Flags"; import { Task } from "./Task"; export function createPropertiesProxy(rootElement: Element, objProperties: {}, nativeObjName: string, componentRenderedWaitTask?: Task | null): {} { @@ -9,18 +10,54 @@ export function createPropertiesProxy(rootElement: Element, objProperties: {}, n proxy[key] = value; } else { proxy[key] = async function () { - const nativeObject = window[nativeObjName] || await bindNativeObject(nativeObjName); - - const result = nativeObject[key].apply(window, arguments); - - if (componentRenderedWaitTask) { - // wait until component is rendered, first render should only render static data - await componentRenderedWaitTask.promise; + if (!getBailOutOnUnboundNativeObjectCallsFlag()) { + return invokeNative(nativeObjName, key, arguments, componentRenderedWaitTask, /*bailOut*/ false); } - return result; + try { + return await invokeNative(nativeObjName, key, arguments, componentRenderedWaitTask, /*bailOut*/ true); + } catch (error) { + // remount / teardown races: the view is still calling in while CefGlue has already + // unregistered the object (rejects with a plain string from NativeObjectMethodDispatcher) + if (isUnboundNativeObjectError(error)) { + return; + } + throw error; + } }; } }); return proxy; -} \ No newline at end of file +} + +async function invokeNative( + nativeObjName: string, + key: string, + args: IArguments, + componentRenderedWaitTask: Task | null | undefined, + bailOut: boolean +): Promise { + const nativeObject = window[nativeObjName] || await bindNativeObject(nativeObjName); + const method = nativeObject && nativeObject[key]; + + if (bailOut && typeof method !== "function") { + return; + } + + const result = method.apply(window, args); + + if (componentRenderedWaitTask) { + // wait until component is rendered, first render should only render static data + await componentRenderedWaitTask.promise; + } + + return result; +} + +function isUnboundNativeObjectError(error: unknown): boolean { + const message = typeof error === "string" ? error : error instanceof Error ? error.message : String(error); + // CefGlue rejects method calls with a plain string from NativeObjectMethodDispatcher; bind can fail + // the same way when the object will never come back + return (message.indexOf("was not found") >= 0 && message.indexOf("registered before") >= 0) + || message.indexOf("Failed to create native object") >= 0; +} diff --git a/ReactViewResources/Loader/Loader.ts b/ReactViewResources/Loader/Loader.ts index 5d8488d7..a8f30171 100644 --- a/ReactViewResources/Loader/Loader.ts +++ b/ReactViewResources/Loader/Loader.ts @@ -12,7 +12,7 @@ import { ViewMetadata } from "./Internal/ViewMetadata"; import { createPropertiesProxy } from "./Internal/ViewPropertiesProxy"; import { addView, getView, tryGetView } from "./Internal/ViewsCollection"; import { setEnsureDisposeInnerViewsFlag } from "./Internal/ViewMetadataContext"; -import { setEnsureViewPluginsAreDisposedFlag, setLoadScriptsOncePerDocumentFlag } from "./Internal/Flags"; +import { setBailOutOnUnboundNativeObjectCallsFlag, setEnsureViewPluginsAreDisposedFlag, setLoadScriptsOncePerDocumentFlag } from "./Internal/Flags"; export { disableMouseInteractions, enableMouseInteractions } from "./Internal/InputManager"; export { showErrorMessage } from "./Internal/MessagesProvider"; @@ -139,7 +139,8 @@ export function loadComponent( componentHash: string, ensureDisposeInnerViews: boolean, loadScriptsOncePerDocument: boolean, - ensureViewPluginsAreDisposed: boolean): void { + ensureViewPluginsAreDisposed: boolean, + bailOutOnUnboundNativeObjectCalls: boolean): void { async function innerLoad() { let view: ViewMetadata; @@ -155,6 +156,7 @@ export function loadComponent( // script or is taken down setLoadScriptsOncePerDocumentFlag(loadScriptsOncePerDocument); setEnsureViewPluginsAreDisposedFlag(ensureViewPluginsAreDisposed); + setBailOutOnUnboundNativeObjectCallsFlag(bailOutOnUnboundNativeObjectCalls); } view = tryGetView(frameName)!; From 5ed8d667fad057b3168ed4b372f08587a4988aed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lder=20Gon=C3=A7alves?= Date: Fri, 14 Aug 2026 17:14:23 +0100 Subject: [PATCH 2/5] RDEV-10097 - Address review of the unbound native object bail out Restores the previous behaviour exactly when BailOutOnUnboundNativeObjectCalls is off: the proxy calls nativeObject[key].apply again, so a call to an object that is no longer bound still fails naming the method it was trying to reach, which is what identifies the caller. With the flag on, a released view is now the primary signal. The proxy checks view.isReleased before binding anything, and the rejection text is left as the backstop for the object that is still on the window while the host has already unregistered it. "Failed to create native object" is no longer swallowed: it is raised when registration cannot enter the v8 context, which is a failure rather than a teardown race. Dropped calls are logged, since an object that was never registered looks exactly like one that went away with its view, and only one of those is a bug. Covered by Tests.ReactView/UnboundNativeObjectCallsTests.cs, for both values of the flag. Co-authored-by: Cursor --- ReactViewControl/ReactViewFactory.cs | 6 +- .../Loader/Internal/ViewPropertiesProxy.ts | 46 ++++--- ReactViewResources/Loader/Loader.ts | 2 +- Tests.ReactView/TestAppView/InnerView.tsx | 3 + Tests.ReactView/TestAppView/TestApp.tsx | 20 +++ .../UnboundNativeObjectCallsTests.cs | 117 ++++++++++++++++++ 6 files changed, 176 insertions(+), 18 deletions(-) create mode 100644 Tests.ReactView/UnboundNativeObjectCallsTests.cs diff --git a/ReactViewControl/ReactViewFactory.cs b/ReactViewControl/ReactViewFactory.cs index 1bf8bfa7..e653ff30 100644 --- a/ReactViewControl/ReactViewFactory.cs +++ b/ReactViewControl/ReactViewFactory.cs @@ -49,9 +49,9 @@ public class ReactViewFactory { public virtual bool EnsureViewPluginsAreDisposed => true; /// - /// Calls through the view properties proxy bail out when the native object is missing or was - /// unregistered (typical remount / teardown races). Set to false to restore the previous behaviour, - /// where those calls surface as uncaught errors. + /// Calls through the view properties proxy are dropped, and logged to the console, when the view was + /// destroyed or its native object is no longer bound (typical remount / teardown races). Set to false + /// to restore the previous behaviour, where those calls surface as uncaught errors. /// public virtual bool BailOutOnUnboundNativeObjectCalls => true; } diff --git a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts index 9bda287f..67ced4b8 100644 --- a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts +++ b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts @@ -1,8 +1,9 @@ import { bindNativeObject } from "./NativeAPI"; import { getBailOutOnUnboundNativeObjectCallsFlag } from "./Flags"; import { Task } from "./Task"; +import { ViewMetadata } from "./ViewMetadata"; -export function createPropertiesProxy(rootElement: Element, objProperties: {}, nativeObjName: string, componentRenderedWaitTask?: Task | null): {} { +export function createPropertiesProxy(rootElement: Element, objProperties: {}, nativeObjName: string, view: ViewMetadata, componentRenderedWaitTask?: Task | null): {} { const proxy = Object.assign({}, objProperties); Object.keys(proxy).forEach(key => { const value = objProperties[key]; @@ -10,16 +11,24 @@ export function createPropertiesProxy(rootElement: Element, objProperties: {}, n proxy[key] = value; } else { proxy[key] = async function () { - if (!getBailOutOnUnboundNativeObjectCallsFlag()) { - return invokeNative(nativeObjName, key, arguments, componentRenderedWaitTask, /*bailOut*/ false); + // read per call: the proxy outlives the view, and what it should do about a call that + // arrives after the view is gone is decided by the flag in place at that moment + const bailOut = getBailOutOnUnboundNativeObjectCallsFlag(); + + if (bailOut && view.isReleased) { + // destroying the view is what unregisters its native objects, so there is nothing + // left to call into + logUnboundCall(nativeObjName, key, "the view was destroyed"); + return; } try { - return await invokeNative(nativeObjName, key, arguments, componentRenderedWaitTask, /*bailOut*/ true); + return await invokeNative(nativeObjName, key, arguments, componentRenderedWaitTask, bailOut); } catch (error) { - // remount / teardown races: the view is still calling in while CefGlue has already - // unregistered the object (rejects with a plain string from NativeObjectMethodDispatcher) - if (isUnboundNativeObjectError(error)) { + // teardown races: the object is still on the window while the host has already + // unregistered it, and the call is rejected on arrival + if (bailOut && isUnboundNativeObjectError(error)) { + logUnboundCall(nativeObjName, key, error); return; } throw error; @@ -38,13 +47,15 @@ async function invokeNative( bailOut: boolean ): Promise { const nativeObject = window[nativeObjName] || await bindNativeObject(nativeObjName); - const method = nativeObject && nativeObject[key]; - if (bailOut && typeof method !== "function") { + if (bailOut && (!nativeObject || typeof nativeObject[key] !== "function")) { + // unregistering an object deletes it from the window but leaves its binding task behind, already + // resolved from the first registration, so binding it again succeeds and hands back nothing + logUnboundCall(nativeObjName, key, "the object is no longer bound"); return; } - const result = method.apply(window, args); + const result = nativeObject[key].apply(window, args); if (componentRenderedWaitTask) { // wait until component is rendered, first render should only render static data @@ -54,10 +65,17 @@ async function invokeNative( return result; } +/** + * A dropped call is usually a teardown race, but a native object that was never registered looks exactly + * the same from this side, and that one is a bug worth finding. + */ +function logUnboundCall(nativeObjName: string, key: string, reason: unknown): void { + window.console.debug(`Ignored call to "${nativeObjName}.${key}"`, reason); +} + function isUnboundNativeObjectError(error: unknown): boolean { + // the call is rejected with a plain string, "Object named X was not found. Make sure it was registered + // before.", raised by cef's NativeObjectMethodDispatcher when the object is no longer registered const message = typeof error === "string" ? error : error instanceof Error ? error.message : String(error); - // CefGlue rejects method calls with a plain string from NativeObjectMethodDispatcher; bind can fail - // the same way when the object will never come back - return (message.indexOf("was not found") >= 0 && message.indexOf("registered before") >= 0) - || message.indexOf("Failed to create native object") >= 0; + return message.includes("was not found") && message.includes("registered before"); } diff --git a/ReactViewResources/Loader/Loader.ts b/ReactViewResources/Loader/Loader.ts index a8f30171..3ffb4985 100644 --- a/ReactViewResources/Loader/Loader.ts +++ b/ReactViewResources/Loader/Loader.ts @@ -194,7 +194,7 @@ export function loadComponent( const renderFinishedTask = cacheEntry ? view.viewLoadTask : null; // create proxy for properties obj to delay its methods execution until native object is ready - const properties = createPropertiesProxy(rootElement, componentNativeObject, componentNativeObjectName, renderFinishedTask); + const properties = createPropertiesProxy(rootElement, componentNativeObject, componentNativeObjectName, view, renderFinishedTask); view.nativeObjectNames.push(componentNativeObjectName); // add to the native objects collection const componentClass = (getViewModule(componentName) || {}).default; diff --git a/Tests.ReactView/TestAppView/InnerView.tsx b/Tests.ReactView/TestAppView/InnerView.tsx index 85f5997c..6d83c409 100644 --- a/Tests.ReactView/TestAppView/InnerView.tsx +++ b/Tests.ReactView/TestAppView/InnerView.tsx @@ -15,6 +15,9 @@ export default class InnerView extends React.Component private sharedContextLoaded = false; componentDidMount() { + // kept around on purpose, so that a test can call into this view's native object after the view + // itself is gone + (window as any).InnerViewProperties = this.props; this.props.loaded(); } diff --git a/Tests.ReactView/TestAppView/TestApp.tsx b/Tests.ReactView/TestAppView/TestApp.tsx index 043a08e1..5e46bc67 100644 --- a/Tests.ReactView/TestAppView/TestApp.tsx +++ b/Tests.ReactView/TestAppView/TestApp.tsx @@ -119,6 +119,26 @@ class App extends React.Component { return (window as any).DisposedPluginModules; } + callInnerViewNativeMethod(nativeObjectNameToUnbind?: string) { + const innerViewProperties = (window as any).InnerViewProperties; + + if (nativeObjectNameToUnbind) { + // leaves the js side as unregistering the object does: gone from the window, while its binding + // task stays behind resolved, so binding it again succeeds and hands back nothing + delete (window as any)[nativeObjectNameToUnbind]; + + if ((window as any)[nativeObjectNameToUnbind] !== undefined) { + this.props.event("NativeObjectStillBound"); + return; + } + } + + innerViewProperties.methodCalled(true).then( + () => this.props.event("CallCompleted"), + (error: any) => this.props.event("CallFailed: " + ((error && error.message) || error)) + ); + } + loadCustomResource(url: string) { console.log(url); var img = document.createElement("img"); diff --git a/Tests.ReactView/UnboundNativeObjectCallsTests.cs b/Tests.ReactView/UnboundNativeObjectCallsTests.cs new file mode 100644 index 00000000..146937e1 --- /dev/null +++ b/Tests.ReactView/UnboundNativeObjectCallsTests.cs @@ -0,0 +1,117 @@ +using System.Threading.Tasks; +using NUnit.Framework; +using ReactViewControl; + +namespace Tests.ReactView { + + public abstract class UnboundNativeObjectCallsTestsBase : ReactViewTestBase { + + /// + /// The name the inner view native object is registered under, as built by GetNativeObjectFullName + /// for the "test" frame. + /// + protected const string InnerViewNativeObjectName = "$test$InnerViewModule"; + + protected const string CallReachedNativeObject = "CallReachedNativeObject"; + + protected override void InitializeView() { + if (TargetView != null) { + TargetView.AutoShowInnerView = true; + } + base.InitializeView(); + } + + protected async Task LoadInnerView() { + var innerViewLoaded = new TaskCompletionSource(); + TargetView.InnerView.Loaded += () => innerViewLoaded.TrySetResult(true); +#if DEBUG + TargetView.Ready += () => TargetView.InnerView.Load(); +#endif + TargetView.InnerView.Load(); + await innerViewLoaded.Task; + } + + /// + /// Calls a method of the inner view native object after unbinding it, and returns what came out of + /// the call: the result reported by the view, or CallReachedNativeObject if the call went through. + /// + protected async Task CallUnboundInnerViewNativeMethod() { + await LoadInnerView(); + + var callResult = new TaskCompletionSource(); + TargetView.Event += result => callResult.TrySetResult(result); + TargetView.InnerView.MethodCalled += _ => callResult.TrySetResult(CallReachedNativeObject); + + TargetView.ExecuteMethod("callInnerViewNativeMethod", InnerViewNativeObjectName); + + return await callResult.Task; + } + } + + public class UnboundNativeObjectCallsTests : UnboundNativeObjectCallsTestsBase { + + [Test(Description = "Tests that a call to a native object that is no longer bound is ignored")] + public async Task CallToUnboundNativeObjectIsIgnored() { + await Run(async () => { + var callResult = await CallUnboundInnerViewNativeMethod(); + + Assert.AreEqual("CallCompleted", callResult, "The call to the unbound native object was not ignored!"); + }); + } + + [Test(Description = "Tests that a call to the native object of a destroyed view is ignored")] + public async Task CallToDestroyedViewNativeObjectIsIgnored() { + await Run(async () => { + await LoadInnerView(); + + var innerViewHidden = new TaskCompletionSource(); + var callResult = new TaskCompletionSource(); + TargetView.Event += result => { + if (result == "InnerViewHidden") { + innerViewHidden.TrySetResult(true); + } else { + callResult.TrySetResult(result); + } + }; + TargetView.InnerView.MethodCalled += _ => callResult.TrySetResult(CallReachedNativeObject); + + // the notification is sent from the set state callback, which react runs after it has + // committed the removal, so the inner view is already torn down by the time it arrives + TargetView.ExecuteMethod("hideInnerView"); + await innerViewHidden.Task; + + TargetView.ExecuteMethod("callInnerViewNativeMethod"); + + Assert.AreEqual("CallCompleted", await callResult.Task, "The call to the destroyed view native object was not ignored!"); + }); + } + } + + public class UnboundNativeObjectCallsWithoutBailOutTests : UnboundNativeObjectCallsTestsBase { + + private class ViewFactoryWithoutBailOut : TestReactViewFactory { + + public override bool BailOutOnUnboundNativeObjectCalls => false; + } + + private class ReactViewWithoutBailOut : TestReactView { + + protected override ReactViewFactory Factory => new ViewFactoryWithoutBailOut(); + } + + protected override TestReactView CreateView() { + TestReactView.PreloadedCacheEntriesSize = 0; // disable cache during tests + return new ReactViewWithoutBailOut(); + } + + [Test(Description = "Tests that a call to a native object that is no longer bound fails, and says which method it was, when bailing out is disabled")] + public async Task CallToUnboundNativeObjectFails() { + await Run(async () => { + var callResult = await CallUnboundInnerViewNativeMethod(); + + Assert.That(callResult, Does.StartWith("CallFailed"), "The call to the unbound native object did not fail!"); + Assert.That(callResult, Does.Contain("methodCalled"), "The failure does not say which method was called!"); + }); + } + } +} From c12fc60de78e783077a0d73e77895474cc10d17c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lder=20Gon=C3=A7alves?= Date: Fri, 14 Aug 2026 18:12:29 +0100 Subject: [PATCH 3/5] RDEV-10097 - Log dropped native object calls at warn level console.debug lands on the Verbose level in the devtools console, which is hidden by default, so the log for a dropped call was there but invisible and the bail out looked silent. It is now a warning: the call was ignored, and an object that was never registered still needs to be noticed. Co-authored-by: Cursor --- ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts index 67ced4b8..724ef367 100644 --- a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts +++ b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts @@ -70,7 +70,7 @@ async function invokeNative( * the same from this side, and that one is a bug worth finding. */ function logUnboundCall(nativeObjName: string, key: string, reason: unknown): void { - window.console.debug(`Ignored call to "${nativeObjName}.${key}"`, reason); + window.console.warn(`Ignored call to "${nativeObjName}.${key}"`, reason); } function isUnboundNativeObjectError(error: unknown): boolean { From a74a8ec911b984b1e952e6e15c97d84d5a652cf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lder=20Gon=C3=A7alves?= Date: Fri, 14 Aug 2026 18:36:50 +0100 Subject: [PATCH 4/5] RDEV-10097 - Only bail out of calls that return nothing A call that returns a value cannot be dropped: the proxy resolves it with undefined, the still mounted component writes that into its state, and the render that follows fails somewhere else, away from the teardown race that caused it. Rapidly switching views in ODC Studio turned dropped getToolbarItems, getTopPaneInfo and getGridLayout calls into "Cannot read properties of undefined" crashes. Only methods known to return nothing are dropped now, which is the class the bail out was written for: the mouseEnterNode / mouseLeaveTree notifications from the original report. The host reflects over the native object it registered and hands the loader the names of its void members. A method that returns a value, or one that could not be matched and about which nothing is known, keeps failing exactly as it did before this branch, naming the method it was trying to reach. A method returning Task counts as void: the promise the caller awaits carries no value either way. Co-authored-by: Cursor --- ReactViewControl/ReactViewFactory.cs | 9 +- .../ReactViewRender.LoaderModule.cs | 38 +++++++- ReactViewControl/ReactViewRender.cs | 12 +-- .../Loader/Internal/ViewPropertiesProxy.ts | 10 ++- ReactViewResources/Loader/Loader.ts | 5 +- Tests.ReactView/InnerViewModule.cs | 9 +- Tests.ReactView/TestAppView/InnerView.tsx | 1 + Tests.ReactView/TestAppView/TestApp.tsx | 10 ++- .../UnboundNativeObjectCallsTests.cs | 90 ++++++++++++++----- 9 files changed, 145 insertions(+), 39 deletions(-) diff --git a/ReactViewControl/ReactViewFactory.cs b/ReactViewControl/ReactViewFactory.cs index e653ff30..09a4b8b1 100644 --- a/ReactViewControl/ReactViewFactory.cs +++ b/ReactViewControl/ReactViewFactory.cs @@ -49,9 +49,12 @@ public class ReactViewFactory { public virtual bool EnsureViewPluginsAreDisposed => true; /// - /// Calls through the view properties proxy are dropped, and logged to the console, when the view was - /// destroyed or its native object is no longer bound (typical remount / teardown races). Set to false - /// to restore the previous behaviour, where those calls surface as uncaught errors. + /// Calls through the view properties proxy to methods that return nothing are dropped, and logged to + /// the console, when the view was destroyed or its native object is no longer bound (typical remount + /// / teardown races). Calls to methods that return a value are left alone, and still surface as + /// errors: their result is used by the caller, and resolving it with undefined would only move the + /// failure away from its cause. Set to false to restore the previous behaviour, where every one of + /// those calls surfaces as an uncaught error. /// public virtual bool BailOutOnUnboundNativeObjectCalls => true; } diff --git a/ReactViewControl/ReactViewRender.LoaderModule.cs b/ReactViewControl/ReactViewRender.LoaderModule.cs index d41155c1..b05509e0 100644 --- a/ReactViewControl/ReactViewRender.LoaderModule.cs +++ b/ReactViewControl/ReactViewRender.LoaderModule.cs @@ -1,8 +1,10 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Security.Cryptography; using System.Text; +using System.Threading.Tasks; using WebViewControl; namespace ReactViewControl { @@ -22,7 +24,7 @@ public LoaderModule(ReactViewRender viewRender) { /// /// Loads the specified react component into the specified frame /// - public void LoadComponent(IViewModule component, string frameName, bool hasStyleSheet, bool hasPlugins, bool ensureDisposeInnerViews, bool loadScriptsOncePerDocument, bool ensureViewPluginsAreDisposed, bool bailOutOnUnboundNativeObjectCalls) { + public void LoadComponent(IViewModule component, object componentNativeObject, string frameName, bool hasStyleSheet, bool hasPlugins, bool ensureDisposeInnerViews, bool loadScriptsOncePerDocument, bool ensureViewPluginsAreDisposed, bool bailOutOnUnboundNativeObjectCalls) { var mainSource = ViewRender.ToFullUrl(NormalizeUrl(component.MainJsSource)); var dependencySources = component.DependencyJsSources.Select(s => ViewRender.ToFullUrl(NormalizeUrl(s))).ToArray(); var cssSources = component.CssSources.Select(s => ViewRender.ToFullUrl(NormalizeUrl(s))).ToArray(); @@ -47,6 +49,7 @@ public void LoadComponent(IViewModule component, string frameName, bool hasStyle // loadScriptsOncePerDocument: boolean // ensureViewPluginsAreDisposed: boolean // bailOutOnUnboundNativeObjectCalls: boolean + // voidNativeObjectMethods: string[] var loadArgs = new[] { JavascriptSerializer.Serialize(component.Name), @@ -64,6 +67,7 @@ public void LoadComponent(IViewModule component, string frameName, bool hasStyle JavascriptSerializer.Serialize(loadScriptsOncePerDocument), JavascriptSerializer.Serialize(ensureViewPluginsAreDisposed), JavascriptSerializer.Serialize(bailOutOnUnboundNativeObjectCalls), + JavascriptSerializer.Serialize(GetVoidNativeObjectMethods(component, componentNativeObject)), }; ExecuteLoaderFunction("loadComponent", loadArgs); @@ -149,6 +153,38 @@ private static string SerializeComponent(IViewModule component) { return JavascriptSerializer.Serialize(nativeObjectMethodsMap, o => JavascriptSerializer.Serialize(o)); } + /// + /// The names, as javascript sees them, of the native object methods that return nothing. Only + /// calls to these can be dropped when the object is no longer bound: dropping a call that + /// returns a value would hand the caller an undefined result instead of an error, and the + /// failure would surface far away from its cause. + /// A method that cannot be matched on the native object is left out, so an unexpected shape + /// costs the bail out rather than the correctness of the call. + /// + private static string[] GetVoidNativeObjectMethods(IViewModule component, object componentNativeObject) { + if (componentNativeObject == null) { + return new string[0]; + } + + var nativeMethods = componentNativeObject.GetType() + .GetMethods(BindingFlags.Public | BindingFlags.Instance) + .Where(m => m.DeclaringType != typeof(object)) + .ToLookup(m => m.Name, StringComparer.OrdinalIgnoreCase); + + return component.Events + .Where(e => nativeMethods.Contains(e) && nativeMethods[e].All(m => ReturnsNothing(m.ReturnType))) + .Select(JavascriptSerializer.GetJavascriptName) + .ToArray(); + } + + /// + /// A method returning Task, rather than Task<T>, is as void as one returning void: the + /// promise the caller awaits carries no value either way. + /// + private static bool ReturnsNothing(Type returnType) { + return returnType == typeof(void) || returnType == typeof(Task) || returnType == typeof(ValueTask); + } + private static string ComputeHash(string inputString) { using (var sha256 = SHA256.Create()) { return Convert.ToBase64String(sha256.ComputeHash(Encoding.UTF8.GetBytes(inputString))); diff --git a/ReactViewControl/ReactViewRender.cs b/ReactViewControl/ReactViewRender.cs index ba1c21f9..87999cec 100644 --- a/ReactViewControl/ReactViewRender.cs +++ b/ReactViewControl/ReactViewRender.cs @@ -278,9 +278,9 @@ private void TryLoadComponent(FrameInfo frame) { frame.LoadStatus = LoadStatus.ComponentLoading; - RegisterNativeObject(frame.Component, frame); + var nativeObject = RegisterNativeObject(frame.Component, frame); - Loader.LoadComponent(frame.Component, frame.Name, DefaultStyleSheet != null, frame.Plugins.Length > 0, ensureDisposeInnerViews, loadScriptsOncePerDocument, ensureViewPluginsAreDisposed, bailOutOnUnboundNativeObjectCalls); + Loader.LoadComponent(frame.Component, nativeObject, frame.Name, DefaultStyleSheet != null, frame.Plugins.Length > 0, ensureDisposeInnerViews, loadScriptsOncePerDocument, ensureViewPluginsAreDisposed, bailOutOnUnboundNativeObjectCalls); if (isInputDisabled && frame.IsMain) { Loader.DisableMouseInteractions(); } @@ -521,14 +521,16 @@ private void HandleCustomResourceRequested(ResourceHandler resourceHandler) { } /// - /// Registers a .net object to be available on the js context. + /// Registers a .net object to be available on the js context, and returns it. /// /// /// /// - private void RegisterNativeObject(IViewModule module, FrameInfo frame) { + private object RegisterNativeObject(IViewModule module, FrameInfo frame) { var nativeObjectName = module.GetNativeObjectFullName(frame.Name); - WebView.RegisterJavascriptObject(nativeObjectName, module.CreateNativeObject(), interceptCall: CallNativeMethod); + var nativeObject = module.CreateNativeObject(); + WebView.RegisterJavascriptObject(nativeObjectName, nativeObject, interceptCall: CallNativeMethod); + return nativeObject; } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts index 724ef367..0802e509 100644 --- a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts +++ b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts @@ -3,7 +3,13 @@ import { getBailOutOnUnboundNativeObjectCallsFlag } from "./Flags"; import { Task } from "./Task"; import { ViewMetadata } from "./ViewMetadata"; -export function createPropertiesProxy(rootElement: Element, objProperties: {}, nativeObjName: string, view: ViewMetadata, componentRenderedWaitTask?: Task | null): {} { +export function createPropertiesProxy(rootElement: Element, objProperties: {}, nativeObjName: string, view: ViewMetadata, voidNativeObjectMethods: string[], componentRenderedWaitTask?: Task | null): {} { + // only a call that returns nothing can be dropped. Dropping one that returns a value resolves it with + // undefined, which the caller then reads and stores, so the teardown race stops being an error here and + // becomes one somewhere else, further away from its cause. A method the host said nothing about is + // treated as returning a value, the safe default: it keeps failing exactly as it did before the bail out + const voidMethods = new Set(voidNativeObjectMethods || []); + const proxy = Object.assign({}, objProperties); Object.keys(proxy).forEach(key => { const value = objProperties[key]; @@ -13,7 +19,7 @@ export function createPropertiesProxy(rootElement: Element, objProperties: {}, n proxy[key] = async function () { // read per call: the proxy outlives the view, and what it should do about a call that // arrives after the view is gone is decided by the flag in place at that moment - const bailOut = getBailOutOnUnboundNativeObjectCallsFlag(); + const bailOut = getBailOutOnUnboundNativeObjectCallsFlag() && voidMethods.has(key); if (bailOut && view.isReleased) { // destroying the view is what unregisters its native objects, so there is nothing diff --git a/ReactViewResources/Loader/Loader.ts b/ReactViewResources/Loader/Loader.ts index 3ffb4985..e262790d 100644 --- a/ReactViewResources/Loader/Loader.ts +++ b/ReactViewResources/Loader/Loader.ts @@ -140,7 +140,8 @@ export function loadComponent( ensureDisposeInnerViews: boolean, loadScriptsOncePerDocument: boolean, ensureViewPluginsAreDisposed: boolean, - bailOutOnUnboundNativeObjectCalls: boolean): void { + bailOutOnUnboundNativeObjectCalls: boolean, + voidNativeObjectMethods: string[]): void { async function innerLoad() { let view: ViewMetadata; @@ -194,7 +195,7 @@ export function loadComponent( const renderFinishedTask = cacheEntry ? view.viewLoadTask : null; // create proxy for properties obj to delay its methods execution until native object is ready - const properties = createPropertiesProxy(rootElement, componentNativeObject, componentNativeObjectName, view, renderFinishedTask); + const properties = createPropertiesProxy(rootElement, componentNativeObject, componentNativeObjectName, view, voidNativeObjectMethods, renderFinishedTask); view.nativeObjectNames.push(componentNativeObjectName); // add to the native objects collection const componentClass = (getViewModule(componentName) || {}).default; diff --git a/Tests.ReactView/InnerViewModule.cs b/Tests.ReactView/InnerViewModule.cs index edea4843..5eeb58f1 100644 --- a/Tests.ReactView/InnerViewModule.cs +++ b/Tests.ReactView/InnerViewModule.cs @@ -21,12 +21,19 @@ public void Loaded() { public void MethodCalled(bool contextLoaded) { Owner.MethodCalled?.Invoke(contextLoaded); } + + public string ValueReturningMethodCalled(bool contextLoaded) { + Owner.ValueReturningMethodCalled?.Invoke(contextLoaded); + return nameof(ValueReturningMethodCalled); + } } public event Action Loaded; public event Action MethodCalled; + public event Action ValueReturningMethodCalled; + public void TestMethod() { ExecutionEngine.ExecuteMethod(this, "testMethod"); } @@ -41,7 +48,7 @@ protected override object CreateNativeObject() { return new Properties(this); } - protected override string[] Events => new[] { "loaded", "methodCalled" }; + protected override string[] Events => new[] { "loaded", "methodCalled", "valueReturningMethodCalled" }; } } diff --git a/Tests.ReactView/TestAppView/InnerView.tsx b/Tests.ReactView/TestAppView/InnerView.tsx index 6d83c409..f1728399 100644 --- a/Tests.ReactView/TestAppView/InnerView.tsx +++ b/Tests.ReactView/TestAppView/InnerView.tsx @@ -4,6 +4,7 @@ import { ViewSharedContext } from 'ViewFrame'; interface IInnerViewProperties { loaded: () => void; methodCalled: (contextLoaded: boolean) => void; + valueReturningMethodCalled: (contextLoaded: boolean) => Promise; } interface IInnerViewBehaviors { diff --git a/Tests.ReactView/TestAppView/TestApp.tsx b/Tests.ReactView/TestAppView/TestApp.tsx index 5e46bc67..50c2f48e 100644 --- a/Tests.ReactView/TestAppView/TestApp.tsx +++ b/Tests.ReactView/TestAppView/TestApp.tsx @@ -120,6 +120,14 @@ class App extends React.Component { } callInnerViewNativeMethod(nativeObjectNameToUnbind?: string) { + this.callInnerViewNativeMethodCore(properties => properties.methodCalled(true), nativeObjectNameToUnbind); + } + + callInnerViewNativeValueMethod(nativeObjectNameToUnbind?: string) { + this.callInnerViewNativeMethodCore(properties => properties.valueReturningMethodCalled(true), nativeObjectNameToUnbind); + } + + private callInnerViewNativeMethodCore(call: (properties: any) => Promise, nativeObjectNameToUnbind?: string) { const innerViewProperties = (window as any).InnerViewProperties; if (nativeObjectNameToUnbind) { @@ -133,7 +141,7 @@ class App extends React.Component { } } - innerViewProperties.methodCalled(true).then( + call(innerViewProperties).then( () => this.props.event("CallCompleted"), (error: any) => this.props.event("CallFailed: " + ((error && error.message) || error)) ); diff --git a/Tests.ReactView/UnboundNativeObjectCallsTests.cs b/Tests.ReactView/UnboundNativeObjectCallsTests.cs index 146937e1..b7f83d4b 100644 --- a/Tests.ReactView/UnboundNativeObjectCallsTests.cs +++ b/Tests.ReactView/UnboundNativeObjectCallsTests.cs @@ -14,6 +14,14 @@ public abstract class UnboundNativeObjectCallsTestsBase : ReactViewTestBase { protected const string CallReachedNativeObject = "CallReachedNativeObject"; + /// + /// The test app methods that call into the inner view native object: one reaching a method that + /// returns nothing, the other one a method that returns a value. + /// + protected const string CallVoidNativeMethod = "callInnerViewNativeMethod"; + + protected const string CallValueReturningNativeMethod = "callInnerViewNativeValueMethod"; + protected override void InitializeView() { if (TargetView != null) { TargetView.AutoShowInnerView = true; @@ -35,17 +43,50 @@ protected async Task LoadInnerView() { /// Calls a method of the inner view native object after unbinding it, and returns what came out of /// the call: the result reported by the view, or CallReachedNativeObject if the call went through. /// - protected async Task CallUnboundInnerViewNativeMethod() { + protected async Task CallUnboundInnerViewNativeMethod(string viewMethod = CallVoidNativeMethod) { await LoadInnerView(); var callResult = new TaskCompletionSource(); TargetView.Event += result => callResult.TrySetResult(result); - TargetView.InnerView.MethodCalled += _ => callResult.TrySetResult(CallReachedNativeObject); + ObserveCallsReachingNativeObject(callResult); + + TargetView.ExecuteMethod(viewMethod, InnerViewNativeObjectName); + + return await callResult.Task; + } - TargetView.ExecuteMethod("callInnerViewNativeMethod", InnerViewNativeObjectName); + /// + /// Calls a method of the inner view native object after destroying the view that owns it, and + /// returns what came out of the call, as CallUnboundInnerViewNativeMethod does. + /// + protected async Task CallDestroyedInnerViewNativeMethod(string viewMethod = CallVoidNativeMethod) { + await LoadInnerView(); + + var innerViewHidden = new TaskCompletionSource(); + var callResult = new TaskCompletionSource(); + TargetView.Event += result => { + if (result == "InnerViewHidden") { + innerViewHidden.TrySetResult(true); + } else { + callResult.TrySetResult(result); + } + }; + ObserveCallsReachingNativeObject(callResult); + + // the notification is sent from the set state callback, which react runs after it has + // committed the removal, so the inner view is already torn down by the time it arrives + TargetView.ExecuteMethod("hideInnerView"); + await innerViewHidden.Task; + + TargetView.ExecuteMethod(viewMethod); return await callResult.Task; } + + private void ObserveCallsReachingNativeObject(TaskCompletionSource callResult) { + TargetView.InnerView.MethodCalled += _ => callResult.TrySetResult(CallReachedNativeObject); + TargetView.InnerView.ValueReturningMethodCalled += _ => callResult.TrySetResult(CallReachedNativeObject); + } } public class UnboundNativeObjectCallsTests : UnboundNativeObjectCallsTestsBase { @@ -62,27 +103,28 @@ await Run(async () => { [Test(Description = "Tests that a call to the native object of a destroyed view is ignored")] public async Task CallToDestroyedViewNativeObjectIsIgnored() { await Run(async () => { - await LoadInnerView(); - - var innerViewHidden = new TaskCompletionSource(); - var callResult = new TaskCompletionSource(); - TargetView.Event += result => { - if (result == "InnerViewHidden") { - innerViewHidden.TrySetResult(true); - } else { - callResult.TrySetResult(result); - } - }; - TargetView.InnerView.MethodCalled += _ => callResult.TrySetResult(CallReachedNativeObject); - - // the notification is sent from the set state callback, which react runs after it has - // committed the removal, so the inner view is already torn down by the time it arrives - TargetView.ExecuteMethod("hideInnerView"); - await innerViewHidden.Task; - - TargetView.ExecuteMethod("callInnerViewNativeMethod"); - - Assert.AreEqual("CallCompleted", await callResult.Task, "The call to the destroyed view native object was not ignored!"); + var callResult = await CallDestroyedInnerViewNativeMethod(); + + Assert.AreEqual("CallCompleted", callResult, "The call to the destroyed view native object was not ignored!"); + }); + } + + [Test(Description = "Tests that a value returning call to a native object that is no longer bound fails, instead of being resolved with no value")] + public async Task ValueReturningCallToUnboundNativeObjectFails() { + await Run(async () => { + var callResult = await CallUnboundInnerViewNativeMethod(CallValueReturningNativeMethod); + + Assert.That(callResult, Does.StartWith("CallFailed"), "The value returning call to the unbound native object did not fail!"); + Assert.That(callResult, Does.Contain("valueReturningMethodCalled"), "The failure does not say which method was called!"); + }); + } + + [Test(Description = "Tests that a value returning call to the native object of a destroyed view fails, instead of being resolved with no value")] + public async Task ValueReturningCallToDestroyedViewNativeObjectFails() { + await Run(async () => { + var callResult = await CallDestroyedInnerViewNativeMethod(CallValueReturningNativeMethod); + + Assert.That(callResult, Does.StartWith("CallFailed"), "The value returning call to the destroyed view native object did not fail!"); }); } } From 9a85e09037b030bca0adb9e778ac450829ab8a76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9lder=20Gon=C3=A7alves?= Date: Fri, 14 Aug 2026 18:51:14 +0100 Subject: [PATCH 5/5] RDEV-10097 - Only bail out of calls into a destroyed view The void / value axis this bail out was narrowed to is the wrong one. Dropping a void call on a view that is still alive discards a real user interaction: a doubleClickNode or a mouseEnterNode that vanishes leaves a tree that looks alive and does nothing, and nothing anywhere reports it. A render that crashes is loud and recoverable; view to presenter communication that quietly stops is neither. What separates the two cases is whether the view was released, not what the call returns. A destroyed view has no native object left to reach and nobody left to receive a result, so its calls are dropped whatever their return type, as the getGridInfo call on an already destroyed UIEditorView harmlessly was. Everything else is left alone: a live view that cannot reach its native object is a broken channel to the presenter, so getToolbarItems, getLeftPaneState and the flood of mouseEnterNode calls seen rejecting with "the object is no longer bound" go back to surfacing exactly as they did before this branch. Since isReleased is checked before the call is dispatched, a released view never reaches native, and every rejection the catch used to swallow necessarily came from a live view. The catch, the unbound object check that followed the bind, and the reflected void member metadata that fed both of them are gone with it. BailOutOnUnboundNativeObjectCalls stays, now gating only the dropped call into a destroyed view. The host lifecycle bug that strands the native objects of live views is being diagnosed separately. A bounded wait for a rebind, so that an in flight call survives a legitimate re-registration, is a follow up once that lands. Co-authored-by: Cursor --- ReactViewControl/ReactViewFactory.cs | 14 ++-- .../ReactViewRender.LoaderModule.cs | 38 +-------- ReactViewControl/ReactViewRender.cs | 12 ++- .../Loader/Internal/ViewPropertiesProxy.ts | 78 +++++-------------- ReactViewResources/Loader/Loader.ts | 5 +- .../UnboundNativeObjectCallsTests.cs | 49 ++++++------ 6 files changed, 61 insertions(+), 135 deletions(-) diff --git a/ReactViewControl/ReactViewFactory.cs b/ReactViewControl/ReactViewFactory.cs index 09a4b8b1..4d7e9d14 100644 --- a/ReactViewControl/ReactViewFactory.cs +++ b/ReactViewControl/ReactViewFactory.cs @@ -49,12 +49,14 @@ public class ReactViewFactory { public virtual bool EnsureViewPluginsAreDisposed => true; /// - /// Calls through the view properties proxy to methods that return nothing are dropped, and logged to - /// the console, when the view was destroyed or its native object is no longer bound (typical remount - /// / teardown races). Calls to methods that return a value are left alone, and still surface as - /// errors: their result is used by the caller, and resolving it with undefined would only move the - /// failure away from its cause. Set to false to restore the previous behaviour, where every one of - /// those calls surfaces as an uncaught error. + /// Calls through the view properties proxy into a view that was already destroyed are dropped, and + /// logged to the console, whatever they return: destroying a view unregisters its native objects, so + /// there is nothing left to call into and nobody left to receive a result. + /// Every other call is left alone and still surfaces as an error, including one whose native object + /// was unregistered while its view is still live: that is a broken channel to the presenter, and + /// dropping it would silently discard a real user interaction. + /// Set to false to restore the previous behaviour, where a call into a destroyed view surfaces as an + /// uncaught error as well. /// public virtual bool BailOutOnUnboundNativeObjectCalls => true; } diff --git a/ReactViewControl/ReactViewRender.LoaderModule.cs b/ReactViewControl/ReactViewRender.LoaderModule.cs index b05509e0..d41155c1 100644 --- a/ReactViewControl/ReactViewRender.LoaderModule.cs +++ b/ReactViewControl/ReactViewRender.LoaderModule.cs @@ -1,10 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Reflection; using System.Security.Cryptography; using System.Text; -using System.Threading.Tasks; using WebViewControl; namespace ReactViewControl { @@ -24,7 +22,7 @@ public LoaderModule(ReactViewRender viewRender) { /// /// Loads the specified react component into the specified frame /// - public void LoadComponent(IViewModule component, object componentNativeObject, string frameName, bool hasStyleSheet, bool hasPlugins, bool ensureDisposeInnerViews, bool loadScriptsOncePerDocument, bool ensureViewPluginsAreDisposed, bool bailOutOnUnboundNativeObjectCalls) { + public void LoadComponent(IViewModule component, string frameName, bool hasStyleSheet, bool hasPlugins, bool ensureDisposeInnerViews, bool loadScriptsOncePerDocument, bool ensureViewPluginsAreDisposed, bool bailOutOnUnboundNativeObjectCalls) { var mainSource = ViewRender.ToFullUrl(NormalizeUrl(component.MainJsSource)); var dependencySources = component.DependencyJsSources.Select(s => ViewRender.ToFullUrl(NormalizeUrl(s))).ToArray(); var cssSources = component.CssSources.Select(s => ViewRender.ToFullUrl(NormalizeUrl(s))).ToArray(); @@ -49,7 +47,6 @@ public void LoadComponent(IViewModule component, object componentNativeObject, s // loadScriptsOncePerDocument: boolean // ensureViewPluginsAreDisposed: boolean // bailOutOnUnboundNativeObjectCalls: boolean - // voidNativeObjectMethods: string[] var loadArgs = new[] { JavascriptSerializer.Serialize(component.Name), @@ -67,7 +64,6 @@ public void LoadComponent(IViewModule component, object componentNativeObject, s JavascriptSerializer.Serialize(loadScriptsOncePerDocument), JavascriptSerializer.Serialize(ensureViewPluginsAreDisposed), JavascriptSerializer.Serialize(bailOutOnUnboundNativeObjectCalls), - JavascriptSerializer.Serialize(GetVoidNativeObjectMethods(component, componentNativeObject)), }; ExecuteLoaderFunction("loadComponent", loadArgs); @@ -153,38 +149,6 @@ private static string SerializeComponent(IViewModule component) { return JavascriptSerializer.Serialize(nativeObjectMethodsMap, o => JavascriptSerializer.Serialize(o)); } - /// - /// The names, as javascript sees them, of the native object methods that return nothing. Only - /// calls to these can be dropped when the object is no longer bound: dropping a call that - /// returns a value would hand the caller an undefined result instead of an error, and the - /// failure would surface far away from its cause. - /// A method that cannot be matched on the native object is left out, so an unexpected shape - /// costs the bail out rather than the correctness of the call. - /// - private static string[] GetVoidNativeObjectMethods(IViewModule component, object componentNativeObject) { - if (componentNativeObject == null) { - return new string[0]; - } - - var nativeMethods = componentNativeObject.GetType() - .GetMethods(BindingFlags.Public | BindingFlags.Instance) - .Where(m => m.DeclaringType != typeof(object)) - .ToLookup(m => m.Name, StringComparer.OrdinalIgnoreCase); - - return component.Events - .Where(e => nativeMethods.Contains(e) && nativeMethods[e].All(m => ReturnsNothing(m.ReturnType))) - .Select(JavascriptSerializer.GetJavascriptName) - .ToArray(); - } - - /// - /// A method returning Task, rather than Task<T>, is as void as one returning void: the - /// promise the caller awaits carries no value either way. - /// - private static bool ReturnsNothing(Type returnType) { - return returnType == typeof(void) || returnType == typeof(Task) || returnType == typeof(ValueTask); - } - private static string ComputeHash(string inputString) { using (var sha256 = SHA256.Create()) { return Convert.ToBase64String(sha256.ComputeHash(Encoding.UTF8.GetBytes(inputString))); diff --git a/ReactViewControl/ReactViewRender.cs b/ReactViewControl/ReactViewRender.cs index 87999cec..ba1c21f9 100644 --- a/ReactViewControl/ReactViewRender.cs +++ b/ReactViewControl/ReactViewRender.cs @@ -278,9 +278,9 @@ private void TryLoadComponent(FrameInfo frame) { frame.LoadStatus = LoadStatus.ComponentLoading; - var nativeObject = RegisterNativeObject(frame.Component, frame); + RegisterNativeObject(frame.Component, frame); - Loader.LoadComponent(frame.Component, nativeObject, frame.Name, DefaultStyleSheet != null, frame.Plugins.Length > 0, ensureDisposeInnerViews, loadScriptsOncePerDocument, ensureViewPluginsAreDisposed, bailOutOnUnboundNativeObjectCalls); + Loader.LoadComponent(frame.Component, frame.Name, DefaultStyleSheet != null, frame.Plugins.Length > 0, ensureDisposeInnerViews, loadScriptsOncePerDocument, ensureViewPluginsAreDisposed, bailOutOnUnboundNativeObjectCalls); if (isInputDisabled && frame.IsMain) { Loader.DisableMouseInteractions(); } @@ -521,16 +521,14 @@ private void HandleCustomResourceRequested(ResourceHandler resourceHandler) { } /// - /// Registers a .net object to be available on the js context, and returns it. + /// Registers a .net object to be available on the js context. /// /// /// /// - private object RegisterNativeObject(IViewModule module, FrameInfo frame) { + private void RegisterNativeObject(IViewModule module, FrameInfo frame) { var nativeObjectName = module.GetNativeObjectFullName(frame.Name); - var nativeObject = module.CreateNativeObject(); - WebView.RegisterJavascriptObject(nativeObjectName, nativeObject, interceptCall: CallNativeMethod); - return nativeObject; + WebView.RegisterJavascriptObject(nativeObjectName, module.CreateNativeObject(), interceptCall: CallNativeMethod); } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts index 0802e509..32bb4e60 100644 --- a/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts +++ b/ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts @@ -3,13 +3,7 @@ import { getBailOutOnUnboundNativeObjectCallsFlag } from "./Flags"; import { Task } from "./Task"; import { ViewMetadata } from "./ViewMetadata"; -export function createPropertiesProxy(rootElement: Element, objProperties: {}, nativeObjName: string, view: ViewMetadata, voidNativeObjectMethods: string[], componentRenderedWaitTask?: Task | null): {} { - // only a call that returns nothing can be dropped. Dropping one that returns a value resolves it with - // undefined, which the caller then reads and stores, so the teardown race stops being an error here and - // becomes one somewhere else, further away from its cause. A method the host said nothing about is - // treated as returning a value, the safe default: it keeps failing exactly as it did before the bail out - const voidMethods = new Set(voidNativeObjectMethods || []); - +export function createPropertiesProxy(rootElement: Element, objProperties: {}, nativeObjName: string, view: ViewMetadata, componentRenderedWaitTask?: Task | null): {} { const proxy = Object.assign({}, objProperties); Object.keys(proxy).forEach(key => { const value = objProperties[key]; @@ -19,69 +13,37 @@ export function createPropertiesProxy(rootElement: Element, objProperties: {}, n proxy[key] = async function () { // read per call: the proxy outlives the view, and what it should do about a call that // arrives after the view is gone is decided by the flag in place at that moment - const bailOut = getBailOutOnUnboundNativeObjectCallsFlag() && voidMethods.has(key); - - if (bailOut && view.isReleased) { - // destroying the view is what unregisters its native objects, so there is nothing - // left to call into + if (getBailOutOnUnboundNativeObjectCallsFlag() && view.isReleased) { + // destroying the view is what unregisters its native objects, so there is nothing left + // to call into, and nobody left to receive what the call would have returned logUnboundCall(nativeObjName, key, "the view was destroyed"); return; } - try { - return await invokeNative(nativeObjName, key, arguments, componentRenderedWaitTask, bailOut); - } catch (error) { - // teardown races: the object is still on the window while the host has already - // unregistered it, and the call is rejected on arrival - if (bailOut && isUnboundNativeObjectError(error)) { - logUnboundCall(nativeObjName, key, error); - return; - } - throw error; + // every call that gets this far belongs to a live view, and a live view that cannot reach + // its native object is a broken channel to the presenter, not a teardown race: it has to + // keep failing exactly as it did before this bail out existed. Dropping it would discard a + // real user interaction and leave a view that looks alive but does nothing + const nativeObject = window[nativeObjName] || await bindNativeObject(nativeObjName); + + const result = nativeObject[key].apply(window, arguments); + + if (componentRenderedWaitTask) { + // wait until component is rendered, first render should only render static data + await componentRenderedWaitTask.promise; } + + return result; }; } }); return proxy; } -async function invokeNative( - nativeObjName: string, - key: string, - args: IArguments, - componentRenderedWaitTask: Task | null | undefined, - bailOut: boolean -): Promise { - const nativeObject = window[nativeObjName] || await bindNativeObject(nativeObjName); - - if (bailOut && (!nativeObject || typeof nativeObject[key] !== "function")) { - // unregistering an object deletes it from the window but leaves its binding task behind, already - // resolved from the first registration, so binding it again succeeds and hands back nothing - logUnboundCall(nativeObjName, key, "the object is no longer bound"); - return; - } - - const result = nativeObject[key].apply(window, args); - - if (componentRenderedWaitTask) { - // wait until component is rendered, first render should only render static data - await componentRenderedWaitTask.promise; - } - - return result; -} - /** - * A dropped call is usually a teardown race, but a native object that was never registered looks exactly - * the same from this side, and that one is a bug worth finding. + * A dropped call is expected while a view is being taken down, but one arriving long after that means + * something is still holding on to a released view, and that is a bug worth finding. */ -function logUnboundCall(nativeObjName: string, key: string, reason: unknown): void { +function logUnboundCall(nativeObjName: string, key: string, reason: string): void { window.console.warn(`Ignored call to "${nativeObjName}.${key}"`, reason); } - -function isUnboundNativeObjectError(error: unknown): boolean { - // the call is rejected with a plain string, "Object named X was not found. Make sure it was registered - // before.", raised by cef's NativeObjectMethodDispatcher when the object is no longer registered - const message = typeof error === "string" ? error : error instanceof Error ? error.message : String(error); - return message.includes("was not found") && message.includes("registered before"); -} diff --git a/ReactViewResources/Loader/Loader.ts b/ReactViewResources/Loader/Loader.ts index e262790d..3ffb4985 100644 --- a/ReactViewResources/Loader/Loader.ts +++ b/ReactViewResources/Loader/Loader.ts @@ -140,8 +140,7 @@ export function loadComponent( ensureDisposeInnerViews: boolean, loadScriptsOncePerDocument: boolean, ensureViewPluginsAreDisposed: boolean, - bailOutOnUnboundNativeObjectCalls: boolean, - voidNativeObjectMethods: string[]): void { + bailOutOnUnboundNativeObjectCalls: boolean): void { async function innerLoad() { let view: ViewMetadata; @@ -195,7 +194,7 @@ export function loadComponent( const renderFinishedTask = cacheEntry ? view.viewLoadTask : null; // create proxy for properties obj to delay its methods execution until native object is ready - const properties = createPropertiesProxy(rootElement, componentNativeObject, componentNativeObjectName, view, voidNativeObjectMethods, renderFinishedTask); + const properties = createPropertiesProxy(rootElement, componentNativeObject, componentNativeObjectName, view, renderFinishedTask); view.nativeObjectNames.push(componentNativeObjectName); // add to the native objects collection const componentClass = (getViewModule(componentName) || {}).default; diff --git a/Tests.ReactView/UnboundNativeObjectCallsTests.cs b/Tests.ReactView/UnboundNativeObjectCallsTests.cs index b7f83d4b..cb06d27a 100644 --- a/Tests.ReactView/UnboundNativeObjectCallsTests.cs +++ b/Tests.ReactView/UnboundNativeObjectCallsTests.cs @@ -40,8 +40,9 @@ protected async Task LoadInnerView() { } /// - /// Calls a method of the inner view native object after unbinding it, and returns what came out of - /// the call: the result reported by the view, or CallReachedNativeObject if the call went through. + /// Calls a method of the inner view native object after unbinding it, leaving the view itself alive, + /// and returns what came out of the call: the result reported by the view, or + /// CallReachedNativeObject if the call went through. /// protected async Task CallUnboundInnerViewNativeMethod(string viewMethod = CallVoidNativeMethod) { await LoadInnerView(); @@ -91,40 +92,41 @@ private void ObserveCallsReachingNativeObject(TaskCompletionSource callR public class UnboundNativeObjectCallsTests : UnboundNativeObjectCallsTestsBase { - [Test(Description = "Tests that a call to a native object that is no longer bound is ignored")] - public async Task CallToUnboundNativeObjectIsIgnored() { + [Test(Description = "Tests that a call to the native object of a destroyed view is ignored")] + public async Task CallToDestroyedViewNativeObjectIsIgnored() { await Run(async () => { - var callResult = await CallUnboundInnerViewNativeMethod(); + var callResult = await CallDestroyedInnerViewNativeMethod(); - Assert.AreEqual("CallCompleted", callResult, "The call to the unbound native object was not ignored!"); + Assert.AreEqual("CallCompleted", callResult, "The call to the destroyed view native object was not ignored!"); }); } - [Test(Description = "Tests that a call to the native object of a destroyed view is ignored")] - public async Task CallToDestroyedViewNativeObjectIsIgnored() { + [Test(Description = "Tests that a value returning call to the native object of a destroyed view is ignored as well: nobody is left to read the result")] + public async Task ValueReturningCallToDestroyedViewNativeObjectIsIgnored() { await Run(async () => { - var callResult = await CallDestroyedInnerViewNativeMethod(); + var callResult = await CallDestroyedInnerViewNativeMethod(CallValueReturningNativeMethod); - Assert.AreEqual("CallCompleted", callResult, "The call to the destroyed view native object was not ignored!"); + Assert.AreEqual("CallCompleted", callResult, "The value returning call to the destroyed view native object was not ignored!"); }); } - [Test(Description = "Tests that a value returning call to a native object that is no longer bound fails, instead of being resolved with no value")] - public async Task ValueReturningCallToUnboundNativeObjectFails() { + [Test(Description = "Tests that a call to a native object that is no longer bound, made by a view that is still alive, fails instead of being silently dropped")] + public async Task CallToUnboundNativeObjectOfLiveViewFails() { await Run(async () => { - var callResult = await CallUnboundInnerViewNativeMethod(CallValueReturningNativeMethod); + var callResult = await CallUnboundInnerViewNativeMethod(); - Assert.That(callResult, Does.StartWith("CallFailed"), "The value returning call to the unbound native object did not fail!"); - Assert.That(callResult, Does.Contain("valueReturningMethodCalled"), "The failure does not say which method was called!"); + Assert.That(callResult, Does.StartWith("CallFailed"), "The call of a live view to its unbound native object did not fail!"); + Assert.That(callResult, Does.Contain("methodCalled"), "The failure does not say which method was called!"); }); } - [Test(Description = "Tests that a value returning call to the native object of a destroyed view fails, instead of being resolved with no value")] - public async Task ValueReturningCallToDestroyedViewNativeObjectFails() { + [Test(Description = "Tests that a value returning call to a native object that is no longer bound, made by a view that is still alive, fails and says which method it was")] + public async Task ValueReturningCallToUnboundNativeObjectOfLiveViewFails() { await Run(async () => { - var callResult = await CallDestroyedInnerViewNativeMethod(CallValueReturningNativeMethod); + var callResult = await CallUnboundInnerViewNativeMethod(CallValueReturningNativeMethod); - Assert.That(callResult, Does.StartWith("CallFailed"), "The value returning call to the destroyed view native object did not fail!"); + Assert.That(callResult, Does.StartWith("CallFailed"), "The value returning call of a live view to its unbound native object did not fail!"); + Assert.That(callResult, Does.Contain("valueReturningMethodCalled"), "The failure does not say which method was called!"); }); } } @@ -146,13 +148,12 @@ protected override TestReactView CreateView() { return new ReactViewWithoutBailOut(); } - [Test(Description = "Tests that a call to a native object that is no longer bound fails, and says which method it was, when bailing out is disabled")] - public async Task CallToUnboundNativeObjectFails() { + [Test(Description = "Tests that a call to the native object of a destroyed view fails when bailing out is disabled")] + public async Task CallToDestroyedViewNativeObjectFails() { await Run(async () => { - var callResult = await CallUnboundInnerViewNativeMethod(); + var callResult = await CallDestroyedInnerViewNativeMethod(); - Assert.That(callResult, Does.StartWith("CallFailed"), "The call to the unbound native object did not fail!"); - Assert.That(callResult, Does.Contain("methodCalled"), "The failure does not say which method was called!"); + Assert.That(callResult, Does.StartWith("CallFailed"), "The call to the destroyed view native object did not fail!"); }); } }