Fix for #88

Author: jspuij

src/WebWindow.Blazor.JS/WebWindow.Blazor.JS.csproj

@@ -15,6 +15,10 @@
     <WebpackInputs Include="**\*.ts" Exclude="node_modules\**" />
   </ItemGroup>
 
+  <ItemGroup>
+    <WebpackInputs Remove="src\RenderQueue.ts" />
+  </ItemGroup>
+
   <ItemGroup>
     <Folder Include="dist\" />
   </ItemGroup>

src/WebWindow.Blazor.JS/src/Boot.Desktop.ts

@@ -1,17 +1,17 @@
 import '@dotnet/jsinterop/dist/Microsoft.JSInterop';
 import '@browserjs/GlobalExports';
-import { OutOfProcessRenderBatch } from '@browserjs/Rendering/RenderBatch/OutOfProcessRenderBatch';
 import { setEventDispatcher } from '@browserjs/Rendering/RendererEventDispatcher';
 import { internalFunctions as navigationManagerFunctions } from '@browserjs/Services/NavigationManager';
-import { renderBatch } from '@browserjs/Rendering/Renderer';
 import { decode } from 'base64-arraybuffer';
 import * as ipc from './IPC';
+import { RenderQueue } from './RenderQueue';
 
 function boot() {
   setEventDispatcher((eventDescriptor, eventArgs) => DotNet.invokeMethodAsync('WebWindow.Blazor', 'DispatchEvent', eventDescriptor, JSON.stringify(eventArgs)));
   navigationManagerFunctions.listenForNavigationEvents((uri: string, intercepted: boolean) => {
     return DotNet.invokeMethodAsync('WebWindow.Blazor', 'NotifyLocationChanged', uri, intercepted);
   });
+  const renderQueue = RenderQueue.getOrCreate();
 
   // Configure the mechanism for JS<->NET calls
   DotNet.attachDispatcher({
@@ -33,9 +33,9 @@ function boot() {
     DotNet.jsCallDispatcher.endInvokeDotNetFromJS(callId, success, resultOrError);
   });
 
-  ipc.on('JS.RenderBatch', (rendererId, batchBase64) => {
-    var batchData = new Uint8Array(decode(batchBase64));
-    renderBatch(rendererId, new OutOfProcessRenderBatch(batchData));
+  ipc.on('JS.RenderBatch', (batchId, batchBase64) => {
+      var batchData = new Uint8Array(decode(batchBase64));
+      renderQueue.processBatch(batchId, batchData);
   });
 
   ipc.on('JS.Error', (message) => {

src/WebWindow.Blazor.JS/src/RenderQueue.ts

@@ -0,0 +1,66 @@
+import { renderBatch } from '@browserjs/Rendering/Renderer';
+import { OutOfProcessRenderBatch } from '@browserjs/Rendering/RenderBatch/OutOfProcessRenderBatch';
+import * as ipc from './IPC';
+
+export class RenderQueue {
+    private static instance: RenderQueue;
+
+    private nextBatchId = 2;
+
+    private fatalError?: string;
+
+    public browserRendererId: number;
+
+    public constructor(browserRendererId: number) {
+        this.browserRendererId = browserRendererId;
+    }
+
+    public static getOrCreate(): RenderQueue {
+        if (!RenderQueue.instance) {
+            RenderQueue.instance = new RenderQueue(0);
+        }
+
+        return this.instance;
+    }
+
+    public async processBatch(receivedBatchId: number, batchData: Uint8Array): Promise<void> {
+        if (receivedBatchId < this.nextBatchId) {
+            await this.completeBatch(receivedBatchId);
+            return;
+        }
+
+        if (receivedBatchId > this.nextBatchId) {
+            if (this.fatalError) {
+                console.log(`Received a new batch ${receivedBatchId} but errored out on a previous batch ${this.nextBatchId - 1}`);
+                await ipc.send('OnRenderCompleted', [this.nextBatchId - 1, this.fatalError.toString()]);
+                return;
+            }
+            return;
+        }
+
+        try {
+            this.nextBatchId++;
+            renderBatch(this.browserRendererId, new OutOfProcessRenderBatch(batchData));
+            await this.completeBatch(receivedBatchId);
+        } catch (error) {
+            this.fatalError = error.toString();
+            console.error(`There was an error applying batch ${receivedBatchId}.`);
+
+            // If there's a rendering exception, notify server *and* throw on client
+            ipc.send('OnRenderCompleted', [receivedBatchId, error.toString()]);
+            throw error;
+        }
+    }
+
+    public getLastBatchid(): number {
+        return this.nextBatchId - 1;
+    }
+
+    private async completeBatch(batchId: number): Promise<void> {
+        try {
+            await ipc.send('OnRenderCompleted', [batchId, null]);
+        } catch {
+            console.warn(`Failed to deliver completion notification for render '${batchId}'.`);
+        }
+    }
+}

src/WebWindow.Blazor/DesktopRenderer.cs

@@ -5,8 +5,10 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.JSInterop;
 using System;
+using System.Collections.Concurrent;
 using System.IO;
 using System.Reflection;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace WebWindows.Blazor
@@ -17,11 +19,16 @@ namespace WebWindows.Blazor
 
     internal class DesktopRenderer : Renderer
     {
+        private static readonly Task CanceledTask = Task.FromCanceled(new CancellationToken(canceled: true));
+
         private const int RendererId = 0; // Not relevant, since we have only one renderer in Desktop
         private readonly IPC _ipc;
         private readonly IJSRuntime _jsRuntime;
         private static readonly Type _writer;
         private static readonly MethodInfo _writeMethod;
+        internal readonly ConcurrentQueue<UnacknowledgedRenderBatch> _unacknowledgedRenderBatches = new ConcurrentQueue<UnacknowledgedRenderBatch>();
+        private bool _disposing = false;
+        private long _nextRenderId = 1;
 
         public override Dispatcher Dispatcher { get; } = NullDispatcher.Instance;
 
@@ -78,6 +85,12 @@ public Task AddComponentAsync(Type componentType, string domElementSelector)
         /// <inheritdoc />
         protected override Task UpdateDisplayAsync(in RenderBatch batch)
         {
+            if (_disposing)
+            {
+                // We are being disposed, so do no work.
+                return CanceledTask;
+            }
+
             string base64;
             using (var memoryStream = new MemoryStream())
             {
@@ -91,13 +104,111 @@ protected override Task UpdateDisplayAsync(in RenderBatch batch)
                 base64 = Convert.ToBase64String(batchBytes);
             }
 
-            _ipc.Send("JS.RenderBatch", RendererId, base64);
+            var renderId = Interlocked.Increment(ref _nextRenderId);
+
+            var pendingRender = new UnacknowledgedRenderBatch(
+                renderId,
+                new TaskCompletionSource<object>());
+
+            // Buffer the rendered batches no matter what. We'll send it down immediately when the client
+            // is connected or right after the client reconnects.
+
+            _unacknowledgedRenderBatches.Enqueue(pendingRender);
 
-            // TODO: Consider finding a way to get back a completion message from the Desktop side
-            // in case there was an error. We don't really need to wait for anything to happen, since
-            // this is not prerendering and we don't care how quickly the UI is updated, but it would
-            // be desirable to flow back errors.
-            return Task.CompletedTask;
+            _ipc.Send("JS.RenderBatch", renderId, base64);
+
+            return pendingRender.CompletionSource.Task;
+        }
+
+        public Task OnRenderCompletedAsync(long incomingBatchId, string errorMessageOrNull)
+        {
+            if (_disposing)
+            {
+                // Disposing so don't do work.
+                return Task.CompletedTask;
+            }
+
+            // When clients send acks we know for sure they received and applied the batch.
+            // We send batches right away, and hold them in memory until we receive an ACK.
+            // If one or more client ACKs get lost (e.g., with long polling, client->server delivery is not guaranteed)
+            // we might receive an ack for a higher batch.
+            // We confirm all previous batches at that point (because receiving an ack is guarantee
+            // from the client that it has received and successfully applied all batches up to that point).
+
+            // If receive an ack for a previously acknowledged batch, its an error, as the messages are
+            // guaranteed to be delivered in order, so a message for a render batch of 2 will never arrive
+            // after a message for a render batch for 3.
+            // If that were to be the case, it would just be enough to relax the checks here and simply skip
+            // the message.
+
+            // A batch might get lost when we send it to the client, because the client might disconnect before receiving and processing it.
+            // In this case, once it reconnects the server will re-send any unacknowledged batches, some of which the
+            // client might have received and even believe it did send back an acknowledgement for. The client handles
+            // those by re-acknowledging.
+
+            // Even though we're not on the renderer sync context here, it's safe to assume ordered execution of the following
+            // line (i.e., matching the order in which we received batch completion messages) based on the fact that SignalR
+            // synchronizes calls to hub methods. That is, it won't issue more than one call to this method from the same hub
+            // at the same time on different threads.
+
+            if (!_unacknowledgedRenderBatches.TryPeek(out var nextUnacknowledgedBatch) || incomingBatchId < nextUnacknowledgedBatch.BatchId)
+            {
+                // TODO: Log duplicated batch ack.
+                return Task.CompletedTask;
+            }
+            else
+            {
+                var lastBatchId = nextUnacknowledgedBatch.BatchId;
+                // Order is important here so that we don't prematurely dequeue the last nextUnacknowledgedBatch
+                while (_unacknowledgedRenderBatches.TryPeek(out nextUnacknowledgedBatch) && nextUnacknowledgedBatch.BatchId <= incomingBatchId)
+                {
+                    lastBatchId = nextUnacknowledgedBatch.BatchId;
+                    // At this point the queue is definitely not full, we have at least emptied one slot, so we allow a further
+                    // full queue log entry the next time it fills up.
+                    _unacknowledgedRenderBatches.TryDequeue(out _);
+                    ProcessPendingBatch(errorMessageOrNull, nextUnacknowledgedBatch);
+                }
+
+                if (lastBatchId < incomingBatchId)
+                {
+                    // This exception is due to a bad client input, so we mark it as such to prevent logging it as a warning and
+                    // flooding the logs with warnings.
+                    throw new InvalidOperationException($"Received an acknowledgement for batch with id '{incomingBatchId}' when the last batch produced was '{lastBatchId}'.");
+                }
+
+                // Normally we will not have pending renders, but it might happen that we reached the limit of
+                // available buffered renders and new renders got queued.
+                // Invoke ProcessBufferedRenderRequests so that we might produce any additional batch that is
+                // missing.
+
+                // We return the task in here, but the caller doesn't await it.
+                return Dispatcher.InvokeAsync(() =>
+                {
+                    // Now we're on the sync context, check again whether we got disposed since this
+                    // work item was queued. If so there's nothing to do.
+                    if (!_disposing)
+                    {
+                        ProcessPendingRender();
+                    }
+                });
+            }
+        }
+
+        private void ProcessPendingBatch(string errorMessageOrNull, UnacknowledgedRenderBatch entry)
+        {
+            CompleteRender(entry.CompletionSource, errorMessageOrNull);
+        }
+
+        private void CompleteRender(TaskCompletionSource<object> pendingRenderInfo, string errorMessageOrNull)
+        {
+            if (errorMessageOrNull == null)
+            {
+                pendingRenderInfo.TrySetResult(null);
+            }
+            else
+            {
+                pendingRenderInfo.TrySetException(new InvalidOperationException(errorMessageOrNull));
+            }
         }
 
         private async void CaptureAsyncExceptions(ValueTask<object> task)
@@ -116,5 +227,29 @@ protected override void HandleException(Exception exception)
         {
             Console.WriteLine(exception.ToString());
         }
+
+        /// <inheritdoc />
+        protected override void Dispose(bool disposing)
+        {
+            _disposing = true;
+            while (_unacknowledgedRenderBatches.TryDequeue(out var entry))
+            {
+                entry.CompletionSource.TrySetCanceled();
+            }
+            base.Dispose(true);
+        }
+
+        internal readonly struct UnacknowledgedRenderBatch
+        {
+            public UnacknowledgedRenderBatch(long batchId, TaskCompletionSource<object> completionSource)
+            {
+                BatchId = batchId;
+                CompletionSource = completionSource;
+            }
+
+            public long BatchId { get; }
+
+            public TaskCompletionSource<object> CompletionSource { get; }
+        }
     }
 }

src/WebWindow.Blazor/JSInteropMethods.cs

@@ -26,5 +26,12 @@ public static void NotifyLocationChanged(string uri, bool isInterceptedLink)
         {
             DesktopNavigationManager.Instance.SetLocation(uri, isInterceptedLink);
         }
+
+        [JSInvokable(nameof(OnRenderCompleted))]
+        public static async Task OnRenderCompleted(long renderId, string errorMessageOrNull)
+        {
+            var renderer = ComponentsDesktop.DesktopRenderer;
+            await renderer.OnRenderCompletedAsync(renderId, errorMessageOrNull);
+        }
     }
 }