RDEV-10097 - Bail out of unbound native object proxy calls

Directory.Build.props

@@ -5,7 +5,7 @@
     <AssemblyVersion>2.0.0.0</AssemblyVersion>
     <FileVersion>2.0.0.0</FileVersion>
     <!-- Please see https://github.com/OutSystems/reactview?tab=readme-ov-file#versioning for versioning rules -->
-    <Version>5.120.6</Version>
+    <Version>5.120.7</Version>
     <Authors>OutSystems</Authors>
     <Product>ReactView</Product>
     <Copyright>Copyright © OutSystems 2023</Copyright>

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();
                 }

ReactViewControl/ReactViewFactory.cs

@@ -47,5 +47,17 @@ public class ReactViewFactory {
         /// behaviour, where only the host released them.
         /// </summary>
         public virtual bool EnsureViewPluginsAreDisposed => true;
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        public virtual bool BailOutOnUnboundNativeObjectCalls => true;
     }
 }

ReactViewControl/ReactViewRender.LoaderModule.cs

@@ -22,7 +22,7 @@ public LoaderModule(ReactViewRender viewRender) {
             /// <summary>
             /// Loads the specified react component into the specified frame
             /// </summary>
-            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);

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<IViewModule[]> initializePlugins, bool preloadWebView, bool enableDebugMode, bool ensureInnerViewsAreDisposed, bool loadScriptsOncePerDocument = true, bool ensureViewPluginsAreDisposed = true) {
+        public ReactViewRender(ResourceUrl defaultStyleSheet, Func<IViewModule[]> 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();
             }

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;
+}

ReactViewResources/Loader/Internal/ViewPropertiesProxy.ts

@@ -1,14 +1,29 @@
 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<void> | null): {} {
+export function createPropertiesProxy(rootElement: Element, objProperties: {}, nativeObjName: string, view: ViewMetadata, componentRenderedWaitTask?: Task<void> | null): {} {
     const proxy = Object.assign({}, objProperties);
     Object.keys(proxy).forEach(key => {
         const value = objProperties[key];
         if (value !== undefined) {
             proxy[key] = value;
         } else {
             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
+                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;
+                }
+
+                // 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);
@@ -23,4 +38,12 @@ export function createPropertiesProxy(rootElement: Element, objProperties: {}, n
         }
     });
     return proxy;
-}
+}
+
+/**
+ * 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: string): void {
+    window.console.warn(`Ignored call to "${nativeObjName}.${key}"`, reason);
+}

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)!;
@@ -192,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;

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<bool> MethodCalled;
 
+        public event Action<bool> 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" };
 
     }
 }

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<string>;
 }
 
 interface IInnerViewBehaviors {
@@ -15,6 +16,9 @@ export default class InnerView extends React.Component<IInnerViewProperties, {}>
     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();
     }
 

Tests.ReactView/TestAppView/TestApp.tsx

@@ -119,6 +119,34 @@ class App extends React.Component<IAppProperties, IAppState> {
         return (window as any).DisposedPluginModules;
     }
 
+    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<any>, 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;
+            }
+        }
+
+        call(innerViewProperties).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");