diff --git a/Directory.Build.props b/Directory.Build.props index e30e7bfb..9a70202b 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 @@ -18,7 +18,7 @@ 11.0.10 - 3.120.10 + 3.120.12 diff --git a/ReactViewControl.Avalonia/ReactViewControl.Avalonia.csproj b/ReactViewControl.Avalonia/ReactViewControl.Avalonia.csproj index b34e9beb..9c6acf51 100644 --- a/ReactViewControl.Avalonia/ReactViewControl.Avalonia.csproj +++ b/ReactViewControl.Avalonia/ReactViewControl.Avalonia.csproj @@ -25,7 +25,9 @@ + + diff --git a/ReactViewControl/ExecutionEngine.cs b/ReactViewControl/ExecutionEngine.cs index e4f8f074..4a9a5384 100644 --- a/ReactViewControl/ExecutionEngine.cs +++ b/ReactViewControl/ExecutionEngine.cs @@ -27,6 +27,7 @@ public void ExecuteMethod(IViewModule module, string methodCall, params object[] webView.ExecuteScriptFunctionWithSerializedParams(method, args); } else { PendingExecutions.Enqueue(Tuple.Create(module, methodCall, args)); + ReactViewDiagnostics.Log($"Call '{methodCall}' to module '{module.Name}' was buffered: execution engine not started (queued: {PendingExecutions.Count})"); } } @@ -34,6 +35,7 @@ public void ExecuteMethod(IViewModule module, string methodCall, params object[] public Task EvaluateMethodAsync(IViewModule module, string methodCall, params object[] args) { if (webView == null) { + ReactViewDiagnostics.Log($"Evaluate '{methodCall}' on module '{module.Name}' answered with a default value: execution engine not started"); return Task.FromResult(default); } module.Host?.HandledBeforeExecuteMethod(); @@ -42,6 +44,9 @@ public Task EvaluateMethodAsync(IViewModule module, string methodCall, par } public void Start(ExtendedWebView webView, string frameName, string id) { + if (!PendingExecutions.IsEmpty) { + ReactViewDiagnostics.Log($"Execution engine started for '{frameName}': flushing {PendingExecutions.Count} buffered call(s)"); + } this.id = id; this.frameName = frameName; this.webView = webView; diff --git a/ReactViewControl/ReactView.cs b/ReactViewControl/ReactView.cs index 34924c25..848f0d4e 100644 --- a/ReactViewControl/ReactView.cs +++ b/ReactViewControl/ReactView.cs @@ -129,6 +129,13 @@ public T WithPlugin() { /// public bool IsReady => View.IsReady; + /// + /// Unloads a child view, releasing its renderer-side resources even if the react tree that owns + /// its frame never re-renders. + /// + /// + public void UnloadChildView(string frameName) => View.UnloadChildView(frameName); + /// /// Gets or sets the control zoom percentage (1 = 100%) /// diff --git a/ReactViewControl/ReactViewDiagnostics.cs b/ReactViewControl/ReactViewDiagnostics.cs new file mode 100644 index 00000000..6779fba5 --- /dev/null +++ b/ReactViewControl/ReactViewDiagnostics.cs @@ -0,0 +1,15 @@ +using System; + +namespace ReactViewControl { + + /// + /// Surfaces view-lifecycle situations that would otherwise fail silently. The library stays + /// logger-free: hosts subscribe and route the messages into their own logging or telemetry. + /// + public static class ReactViewDiagnostics { + + public static event Action Message; + + internal static void Log(string message) => Message?.Invoke(message); + } +} diff --git a/ReactViewControl/ReactViewRender.LoaderModule.cs b/ReactViewControl/ReactViewRender.LoaderModule.cs index 56742276..541606c9 100644 --- a/ReactViewControl/ReactViewRender.LoaderModule.cs +++ b/ReactViewControl/ReactViewRender.LoaderModule.cs @@ -94,6 +94,15 @@ public void LoadPlugins(IViewModule[] plugins, string frameName) { ExecuteLoaderFunction("loadPlugins", loadArgs); } + /// + /// Unloads the specified child view without waiting for the react tree that owns its + /// frame to re-render. + /// + /// + public void UnloadView(string frameName) { + ExecuteLoaderFunction("unloadView", JavascriptSerializer.Serialize(frameName)); + } + /// /// Shows an resource load error message for the spcified url. /// @@ -135,7 +144,7 @@ public void EnableMouseInteractions() { private void ExecuteLoaderFunction(string functionName, params string[] args) { // using setimeout we make sure the function is already defined var loaderUrl = new ResourceUrl(ResourcesAssembly, ReactViewResources.Resources.LoaderUrl); - ViewRender.WebView.ExecuteScript($"import('{loaderUrl}').then(m => m.default.{LoaderModuleName}).then({LoaderModuleName} => {LoaderModuleName}.{functionName}({string.Join(",", args)}))"); + ViewRender.WebView.ExecuteScript($"import('{loaderUrl}').then(m => m.default.{LoaderModuleName}).then({LoaderModuleName} => {LoaderModuleName}.{functionName}({string.Join(",", args)})).catch(e => console.error('Loader.{functionName} failed: ' + (e && (e.stack || e.message) || e)))"); } private static string SerializeComponent(IViewModule component) { diff --git a/ReactViewControl/ReactViewRender.NativeAPI.cs b/ReactViewControl/ReactViewRender.NativeAPI.cs index b83640e0..07f0b7ef 100644 --- a/ReactViewControl/ReactViewRender.NativeAPI.cs +++ b/ReactViewControl/ReactViewRender.NativeAPI.cs @@ -28,6 +28,11 @@ public static void Initialize(ReactViewRender viewRender) { /// public void NotifyViewInitialized(string frameName) { lock (ViewRender.SyncRoot) { + if (frameName == FrameInfo.MainViewFrameName) { + // a new main view proves the released context was really replaced; children register after it + ViewRender.RunPendingContextLossCleanup(); + } + var frame = ViewRender.GetOrCreateFrame(frameName); frame.LoadStatus = LoadStatus.ViewInitialized; @@ -71,6 +76,7 @@ public void NotifyViewLoaded(string frameName, string id) { /// public void NotifyViewDestroyed(string frameName) { lock (ViewRender.SyncRoot) { + ReactViewDiagnostics.Log($"View '{frameName}' destroyed"); if (ViewRender.Frames.TryGetValue(frameName, out var frame)) { IEnumerable modules = frame.Plugins; if (frame.Component != null) { diff --git a/ReactViewControl/ReactViewRender.cs b/ReactViewControl/ReactViewRender.cs index 2d677267..62056f21 100644 --- a/ReactViewControl/ReactViewRender.cs +++ b/ReactViewControl/ReactViewRender.cs @@ -26,6 +26,7 @@ internal partial class ReactViewRender : IChildViewHost, IDisposable { private Dictionary Frames { get; } = new Dictionary(); private Dictionary> RecoverableFrames { get; } = new Dictionary>(); + private bool hasPendingContextLossCleanup; private Dictionary> ChildViewModules { get; } = new Dictionary>(); private ExtendedWebView WebView { get; } @@ -189,7 +190,9 @@ public event ResourceLoadFailedEventHandler ResourceLoadFailed { internal EditCommands EditCommands { get; } /// - /// Javascript context was destroyed, cleanup everything. + /// Javascript context was released. A release can arrive out of order with its replacement's + /// creation, so the cleanup waits for the new main view to initialize instead of running here, + /// which would strand the current document's live views. /// /// private void OnWebViewJavascriptContextReleased(string frameName) { @@ -199,6 +202,22 @@ private void OnWebViewJavascriptContextReleased(string frameName) { } lock (SyncRoot) { + hasPendingContextLossCleanup = true; + } + ReactViewDiagnostics.Log("Main javascript context released: stale frames will be cleaned when a new main view initializes"); + } + + /// + /// Discards the frames of a document whose main javascript context was lost. Runs before the + /// replacement's child views register, so same-name views get fresh frames. + /// + private void RunPendingContextLossCleanup() { + lock (SyncRoot) { + if (!hasPendingContextLossCleanup) { + return; + } + hasPendingContextLossCleanup = false; + var mainFrame = Frames[FrameInfo.MainViewFrameName]; Frames.Remove(mainFrame.Name); @@ -208,6 +227,8 @@ private void OnWebViewJavascriptContextReleased(string frameName) { UnregisterNativeObject(keyValuePair.Value.Component, keyValuePair.Value); } + ReactViewDiagnostics.Log($"Cleaned {Frames.Count} stale frame(s) after a main javascript context loss"); + Frames.Clear(); Frames.Add(mainFrame.Name, mainFrame); ChildViewModules.Clear(); @@ -217,6 +238,30 @@ private void OnWebViewJavascriptContextReleased(string frameName) { } } + /// + /// Unloads a child view without waiting for the react tree that owns its frame to re-render. + /// Both sides are idempotent with the regular ViewFrame unmount teardown. + /// + /// + public void UnloadChildView(string frameName) { + Loader.UnloadView(frameName); + + lock (SyncRoot) { + if (Frames.TryGetValue(frameName, out var frame) && !frame.IsMain) { + IEnumerable modules = frame.Plugins; + if (frame.Component != null) { + modules = modules.Concat(new[] { frame.Component }); + } + foreach (var module in modules) { + UnregisterNativeObject(module, frame); + } + Frames.Remove(frameName); + ReactViewDiagnostics.Log($"View '{frameName}' unloaded on host initiative"); + } + ChildViewModules.Remove(frameName); + } + } + public void Dispose() { WebView.Dispose(); } diff --git a/ReactViewResources/Loader/Internal/ObservableCollection.ts b/ReactViewResources/Loader/Internal/ObservableCollection.ts index e4286ef9..3a32f843 100644 --- a/ReactViewResources/Loader/Internal/ObservableCollection.ts +++ b/ReactViewResources/Loader/Internal/ObservableCollection.ts @@ -23,6 +23,13 @@ export abstract class ObservableCollection { this.listeners.push(listener); } + public removeChangedListener(listener: CollectionChangedListener) { + const index = this.listeners.indexOf(listener); + if (index >= 0) { + this.listeners.splice(index, 1); + } + } + private triggerCollectionChangedListeners(item: T, operation: Operation) { this.listeners.forEach(l => l(item, operation)); } diff --git a/ReactViewResources/Loader/Loader.ts b/ReactViewResources/Loader/Loader.ts index 5d8488d7..f6ba840c 100644 --- a/ReactViewResources/Loader/Loader.ts +++ b/ReactViewResources/Loader/Loader.ts @@ -17,6 +17,18 @@ import { setEnsureViewPluginsAreDisposedFlag, setLoadScriptsOncePerDocumentFlag export { disableMouseInteractions, enableMouseInteractions } from "./Internal/InputManager"; export { showErrorMessage } from "./Internal/MessagesProvider"; +/** + * Unloads a child view without waiting for its owner ViewFrame to re-render. Removing the view from + * its parent's collection drives the regular portal teardown. + */ +export function unloadView(viewName: string): void { + const view = tryGetView(viewName); + if (!view || view.isMain || !view.parentView) { + return; + } + view.parentView.childViews.remove(view); +} + const bootstrapTask = new Task(); const defaultStylesheetLoadTask = new Task(); diff --git a/ReactViewResources/Loader/Public/ViewFrame.tsx b/ReactViewResources/Loader/Public/ViewFrame.tsx index 8bdb10f2..ea20f773 100644 --- a/ReactViewResources/Loader/Public/ViewFrame.tsx +++ b/ReactViewResources/Loader/Public/ViewFrame.tsx @@ -87,7 +87,16 @@ class InternalViewFrame extends React.Component, { return this.parentView.childViews.items.find(c => c.name === fullName); } + private onChildViewsChanged = () => { + if (!this.getView()) { + // view unloaded from outside (Loader.unloadView): drop the portal so it tears down + this.forceUpdate(); + } + }; + public componentDidMount() { + this.parentView.childViews.addChangedListener(this.onChildViewsChanged); + const existingView = this.getView(); if (existingView) { // update the existing view generation @@ -114,6 +123,9 @@ class InternalViewFrame extends React.Component, { } public componentWillUnmount() { + // stop listening before the removal below re-enters this frame + this.parentView.childViews.removeChangedListener(this.onChildViewsChanged); + if (this.replacement) { // put back the original container, otherwise react will complain this.replacement.parentElement!.replaceChild(this.placeholder, this.replacement); @@ -128,8 +140,10 @@ class InternalViewFrame extends React.Component, { } public render() { + // the view can be unloaded (Loader.unloadView) before this frame re-renders + const view = this.getView(); return
- {this.shadowRoot && } + {this.shadowRoot && view && }
; } } diff --git a/Tests.ReactView/ExecutionEngineDiagnosticsTests.cs b/Tests.ReactView/ExecutionEngineDiagnosticsTests.cs new file mode 100644 index 00000000..786ee71c --- /dev/null +++ b/Tests.ReactView/ExecutionEngineDiagnosticsTests.cs @@ -0,0 +1,56 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using NUnit.Framework; +using ReactViewControl; + +namespace Tests.ReactView { + + // Plain fixture on purpose: the stopped-engine behavior needs no browser. + public class ExecutionEngineDiagnosticsTests { + + private class FakeModule : ViewModuleContainer { + protected override string ModuleName => "FakeModule"; + } + + private class Unsubscriber : IDisposable { + private readonly Action dispose; + public Unsubscriber(Action dispose) => this.dispose = dispose; + public void Dispose() => dispose(); + } + + private static IDisposable CaptureDiagnostics(List sink) { + void OnMessage(string message) => sink.Add(message); + ReactViewDiagnostics.Message += OnMessage; + return new Unsubscriber(() => ReactViewDiagnostics.Message -= OnMessage); + } + + [Test(Description = "A call executed before the engine starts is buffered and reported, not lost silently")] + public void CallOnStoppedEngineIsBufferedAndReported() { + var messages = new List(); + using (CaptureDiagnostics(messages)) { + var engine = new ExecutionEngine(); + IViewModule module = new FakeModule(); + + engine.ExecuteMethod(module, "refreshInnerPanes"); + + Assert.That(messages, Has.Some.Contains("buffered"), "buffering must be surfaced through diagnostics"); + Assert.That(messages, Has.Some.Contains("refreshInnerPanes"), "the diagnostic must name the buffered call"); + } + } + + [Test(Description = "An evaluation on a stopped engine answers default and is reported, not silent")] + public async Task EvaluateOnStoppedEngineAnswersDefaultAndReports() { + var messages = new List(); + using (CaptureDiagnostics(messages)) { + var engine = new ExecutionEngine(); + IViewModule module = new FakeModule(); + + var result = await engine.EvaluateMethodAsync(module, "getBottomPaneInfo"); + + Assert.AreEqual(0, result, "a stopped engine answers default(T)"); + Assert.That(messages, Has.Some.Contains("default value"), "the default answer must be surfaced through diagnostics"); + } + } + } +} diff --git a/Tests.ReactView/MainContextLossTests.cs b/Tests.ReactView/MainContextLossTests.cs new file mode 100644 index 00000000..81d8e306 --- /dev/null +++ b/Tests.ReactView/MainContextLossTests.cs @@ -0,0 +1,89 @@ +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Reflection; +using System.Threading.Tasks; +using NUnit.Framework; +using ReactViewControl; + +namespace Tests.ReactView { + + // Regression coverage for the superseded-main-context defect (RDPIM-4385): a main context release + // used to run the document-gone cleanup immediately, stranding every live inner view. The cleanup + // is now deferred until a new main view initializes. + public class MainContextLossTests : ReactViewTestBase { + + // Fires the real (private) release handler; a stray release cannot be produced on demand otherwise. + private static void SimulateMainContextReleased(ReactViewControl.ReactView view) { + var render = typeof(ReactViewControl.ReactView) + .GetProperty("View", BindingFlags.Instance | BindingFlags.NonPublic)! + .GetValue(view)!; + render.GetType() + .GetMethod("OnWebViewJavascriptContextReleased", BindingFlags.Instance | BindingFlags.NonPublic)! + .Invoke(render, new object[] { FrameInfo.MainViewFrameName }); + } + + private ConcurrentQueue diagnostics; + + protected override void InitializeView() { + diagnostics = new ConcurrentQueue(); + ReactViewDiagnostics.Message += OnDiagnosticMessage; + if (TargetView != null) { + TargetView.AutoShowInnerView = true; + } + base.InitializeView(); + } + + [TearDown] + public void DetachDiagnostics() { + ReactViewDiagnostics.Message -= OnDiagnosticMessage; + } + + private void OnDiagnosticMessage(string message) => diagnostics.Enqueue(message); + + [Test(Description = "A stray main-context release must not break the inner view: calls still arrive")] + public async Task InnerViewSurvivesStrayMainContextRelease() { + await Run(async () => { + var loaded = new TaskCompletionSource(); + TargetView.InnerView.Loaded += () => loaded.TrySetResult(true); + TargetView.InnerView.Load(); + await loaded.Task; + + SimulateMainContextReleased(TargetView); + + var methodCalled = new TaskCompletionSource(); + TargetView.InnerView.MethodCalled += _ => methodCalled.TrySetResult(true); + TargetView.InnerView.TestMethod(); + + var completed = await Task.WhenAny(methodCalled.Task, Task.Delay(TimeSpan.FromSeconds(10))); + Assert.AreSame(methodCalled.Task, completed, "inner view stopped answering after a stray main-context release"); + Assert.IsFalse(diagnostics.Any(m => m.Contains("buffered")), "no call may be buffered on a stopped engine after a stray release"); + }); + } + + [Test(Description = "A reload after a main-context release runs the deferred cleanup and comes back functional")] + public async Task ReloadAfterMainContextReleaseCleansStaleFrames() { + await Run(async () => { + var loaded = new TaskCompletionSource(); + TargetView.InnerView.Loaded += () => loaded.TrySetResult(true); + TargetView.InnerView.Load(); + await loaded.Task; + + SimulateMainContextReleased(TargetView); + + var reloaded = new TaskCompletionSource(); + TargetView.InnerView.Loaded += () => reloaded.TrySetResult(true); + TargetView.ExecuteMethod("reload"); + await reloaded.Task; + + Assert.IsTrue(diagnostics.Any(m => m.Contains("stale frame")), "the deferred cleanup must run when the new main view initializes"); + + var methodCalled = new TaskCompletionSource(); + TargetView.InnerView.MethodCalled += _ => methodCalled.TrySetResult(true); + TargetView.InnerView.TestMethod(); + var completed = await Task.WhenAny(methodCalled.Task, Task.Delay(TimeSpan.FromSeconds(10))); + Assert.AreSame(methodCalled.Task, completed, "inner view must be functional after the reload"); + }); + } + } +} diff --git a/Tests.ReactView/UnloadChildViewTests.cs b/Tests.ReactView/UnloadChildViewTests.cs new file mode 100644 index 00000000..253b091d --- /dev/null +++ b/Tests.ReactView/UnloadChildViewTests.cs @@ -0,0 +1,50 @@ +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Threading.Tasks; +using NUnit.Framework; +using ReactViewControl; + +namespace Tests.ReactView { + + public class UnloadChildViewTests : ReactViewTestBase { + + private ConcurrentQueue diagnostics; + + protected override void InitializeView() { + diagnostics = new ConcurrentQueue(); + ReactViewDiagnostics.Message += OnDiagnosticMessage; + if (TargetView != null) { + TargetView.AutoShowInnerView = true; + } + base.InitializeView(); + } + + [TearDown] + public void DetachDiagnostics() { + ReactViewDiagnostics.Message -= OnDiagnosticMessage; + } + + private void OnDiagnosticMessage(string message) => diagnostics.Enqueue(message); + + [Test(Description = "The host can unload a child view directly, without a react re-render of its owner")] + public async Task HostInitiatedUnloadReleasesTheChildView() { + await Run(async () => { + var loaded = new TaskCompletionSource(); + TargetView.InnerView.Loaded += () => loaded.TrySetResult(true); + TargetView.InnerView.Load(); + await loaded.Task; + + TargetView.UnloadChildView("test"); + + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10); + while (DateTime.UtcNow < deadline && !diagnostics.Any(m => m.Contains("'test' destroyed"))) { + await Task.Delay(100); + } + + Assert.IsTrue(diagnostics.Any(m => m.Contains("'test' unloaded")), "the native side must be released eagerly"); + Assert.IsTrue(diagnostics.Any(m => m.Contains("'test' destroyed")), "the JS side must tear the view down"); + }); + } + } +}