Stop Serializing Return Values for InvokeVoidAsync (#35807)

TanayParikh · web-flow · commit 05303c7571a0 · 2021-08-31T00:52:21.000Z
* Stop Serializing `InvokeVoidAsync` Return Value * Fix E2E Test * Add JSObjectReferenceTest * Remove `IJSVoidResult` from the public API * release .js files * Revert "Remove `IJSVoidResult` from the public API" This reverts commit e754872. * Add monocrash to gitignore * Fix Tests * Update src/JSInterop/Microsoft.JSInterop/src/PublicAPI.Unshipped.txt * PR Feedback @pranavkm
diff --git a/.gitignore b/.gitignore
@@ -42,3 +42,4 @@ StyleCop.Cache
 UpgradeLog.htm
 .idea
 *.svclog
+mono_crash.*.json
diff --git a/src/Components/Server/test/ProtectedBrowserStorageTest.cs b/src/Components/Server/test/ProtectedBrowserStorageTest.cs
@@ -12,6 +12,8 @@
 using Microsoft.AspNetCore.DataProtection;
 using Microsoft.AspNetCore.WebUtilities;
 using Microsoft.JSInterop;
+using Microsoft.JSInterop.Infrastructure;
+using Moq;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Components.Server.ProtectedBrowserStorage
@@ -265,7 +267,7 @@ public async Task ReusesCachedProtectorsByPurpose()
         {
             // Arrange
             var jsRuntime = new TestJSRuntime();
-            jsRuntime.NextInvocationResult = new ValueTask<object>((object)null);
+            jsRuntime.NextInvocationResult = new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>());
             var dataProtectionProvider = new TestDataProtectionProvider();
             var protectedBrowserStorage = new TestProtectedBrowserStorage("testStore", jsRuntime, dataProtectionProvider);
 
diff --git a/src/Components/Web.JS/dist/Release/blazor.server.js b/src/Components/Web.JS/dist/Release/blazor.server.js
diff --git a/src/Components/Web.JS/dist/Release/blazor.webview.js b/src/Components/Web.JS/dist/Release/blazor.webview.js
diff --git a/src/Components/Web.JS/src/Boot.WebAssembly.ts b/src/Components/Web.JS/src/Boot.WebAssembly.ts
@@ -163,6 +163,8 @@ function invokeJSFromDotNet(callInfo: Pointer, arg0: any, arg1: any, arg2: any):
         const streamReference = DotNet.createJSStreamReference(result);
         const resultJson = JSON.stringify(streamReference);
         return BINDING.js_string_to_mono_string(resultJson);
+      case DotNet.JSCallResultType.JSVoidResult:
+        return null;
       default:
         throw new Error(`Invalid JS call result type '${resultType}'.`);
     }
diff --git a/src/Components/WebAssembly/JSInterop/src/WebAssemblyJSRuntime.cs b/src/Components/WebAssembly/JSInterop/src/WebAssemblyJSRuntime.cs
@@ -91,6 +91,7 @@ internal TResult InvokeUnmarshalled<T0, T1, T2, TResult>(string identifier, T0 a
             switch (resultType)
             {
                 case JSCallResultType.Default:
+                case JSCallResultType.JSVoidResult:
                     var result = InternalCalls.InvokeJS<T0, T1, T2, TResult>(out exception, ref callInfo, arg0, arg1, arg2);
                     return exception != null
                         ? throw new JSException(exception)
diff --git a/src/Components/test/E2ETest/Tests/InteropTest.cs b/src/Components/test/E2ETest/Tests/InteropTest.cs
@@ -92,6 +92,8 @@ public void CanInvokeDotNetMethods()
                 ["asyncGenericInstanceMethod"] = @"""Updated value 1""",
                 ["requestDotNetStreamReferenceAsync"] = @"""Success""",
                 ["requestDotNetStreamWrapperReferenceAsync"] = @"""Success""",
+                ["invokeVoidAsyncReturnsWithoutSerializing"] = "Success",
+                ["invokeVoidAsyncReturnsWithoutSerializingInJSObjectReference"] = "Success",
             };
 
             var expectedSyncValues = new Dictionary<string, string>
diff --git a/src/Components/test/testassets/BasicTestApp/InteropComponent.razor b/src/Components/test/testassets/BasicTestApp/InteropComponent.razor
@@ -138,12 +138,36 @@
             ReturnValues["returnPrimitive"] = ((IJSInProcessRuntime)JSRuntime).Invoke<int>("returnPrimitive").ToString();
             ReturnValues["returnArray"] = string.Join(",", ((IJSInProcessRuntime)JSRuntime).Invoke<Segment[]>("returnArray").Select(x => x.Source).ToArray());
         }
+        
+        try
+        {
+            // Triger a non-serializable WindowProxy (https://developer.mozilla.org/en-US/docs/Web/API/Window)
+            // InvokeVoidAsync shouldn't serialize return values, and thus there shouldn't be any exception thrown. 
+            await JSRuntime.InvokeVoidAsync("eval", "window");
+            ReturnValues["invokeVoidAsyncReturnsWithoutSerializing"] = "Success";
+        }
+        catch (Exception ex)
+        {
+            ReturnValues["invokeVoidAsyncReturnsWithoutSerializing"] = $"Failure: {ex.Message}";
+        }
 
         var jsObjectReference = await JSRuntime.InvokeAsync<IJSObjectReference>("returnJSObjectReference");
         ReturnValues["jsObjectReference.identity"] = await jsObjectReference.InvokeAsync<string>("identity", "Invoked from JSObjectReference");
         ReturnValues["jsObjectReference.nested.add"] = (await jsObjectReference.InvokeAsync<int>("nested.add", 2, 3)).ToString();
         ReturnValues["addViaJSObjectReference"] = (await JSRuntime.InvokeAsync<int>("addViaJSObjectReference", jsObjectReference, 2, 3)).ToString();
 
+        try
+        {
+            // Fetch a non-serializable Window (https://developer.mozilla.org/en-US/docs/Web/API/Window)
+            // InvokeVoidAsync shouldn't serialize return values, and thus there shouldn't be any exception thrown. 
+            await jsObjectReference.InvokeVoidAsync("getWindow");
+            ReturnValues["invokeVoidAsyncReturnsWithoutSerializingInJSObjectReference"] = "Success";
+        }
+        catch (Exception ex)
+        {
+            ReturnValues["invokeVoidAsyncReturnsWithoutSerializingInJSObjectReference"] = $"Failure: {ex.Message}";
+        }
+
         try
         {
             await jsObjectReference.InvokeAsync<object>("nonFunction");
@@ -201,7 +225,7 @@
 
         var dotNetStreamReferenceWrapper = DotNetStreamReferenceInterop.GetDotNetStreamWrapperReference();
         ReturnValues["dotNetToJSReceiveDotNetStreamWrapperReferenceAsync"] = await JSRuntime.InvokeAsync<string>("jsInteropTests.receiveDotNetStreamWrapperReference", dotNetStreamReferenceWrapper);
-
+        
         Invocations = invocations;
         DoneWithInterop = true;
     }
diff --git a/src/Components/test/testassets/BasicTestApp/wwwroot/js/jsinteroptests.js b/src/Components/test/testassets/BasicTestApp/wwwroot/js/jsinteroptests.js
@@ -324,6 +324,9 @@ function returnJSObjectReference() {
     identity: function (value) {
       return value;
     },
+    getWindow: function() {
+      return window;
+    },
     nonFunction: 123,
     nested: {
       add: function (a, b) {
diff --git a/src/JSInterop/Microsoft.JSInterop.JS/src/src/Microsoft.JSInterop.ts b/src/JSInterop/Microsoft.JSInterop.JS/src/src/Microsoft.JSInterop.ts
@@ -280,6 +280,7 @@ export module DotNet {
     Default = 0,
     JSObjectReference = 1,
     JSStreamReference = 2,
+    JSVoidResult = 3,
   }
 
   /**
@@ -566,6 +567,8 @@ export module DotNet {
         return createJSObjectReference(returnValue);
       case JSCallResultType.JSStreamReference:
         return createJSStreamReference(returnValue);
+      case JSCallResultType.JSVoidResult:
+        return null;
       default:
         throw new Error(`Invalid JS call result type '${resultType}'.`);
     }
diff --git a/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/IJSVoidResult.cs b/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/IJSVoidResult.cs
@@ -0,0 +1,15 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+using System.ComponentModel;
+
+namespace Microsoft.JSInterop.Infrastructure
+{
+    /// <summary>
+    /// Represents a void result from a JavaScript call.
+    /// This property is public to support cross-assembly accessibility for WebAssembly and should not be used by user code.
+    /// </summary>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public interface IJSVoidResult
+    {
+    }
+}
diff --git a/src/JSInterop/Microsoft.JSInterop/src/JSCallResultType.cs b/src/JSInterop/Microsoft.JSInterop/src/JSCallResultType.cs
@@ -22,5 +22,10 @@ public enum JSCallResultType : int
         /// Indicates that the returned value is to be treated as a JS data reference.
         /// </summary>
         JSStreamReference = 2,
+
+        /// <summary>
+        /// Indicates a void result type.
+        /// </summary>
+        JSVoidResult = 3,
     }
 }
diff --git a/src/JSInterop/Microsoft.JSInterop/src/JSObjectReferenceExtensions.cs b/src/JSInterop/Microsoft.JSInterop/src/JSObjectReferenceExtensions.cs
@@ -5,6 +5,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.JSInterop.Infrastructure;
 using static Microsoft.AspNetCore.Internal.LinkerFlags;
 
 namespace Microsoft.JSInterop
@@ -28,7 +29,7 @@ public static async ValueTask InvokeVoidAsync(this IJSObjectReference jsObjectRe
                 throw new ArgumentNullException(nameof(jsObjectReference));
             }
 
-            await jsObjectReference.InvokeAsync<object>(identifier, args);
+            await jsObjectReference.InvokeAsync<IJSVoidResult>(identifier, args);
         }
 
         /// <summary>
@@ -93,7 +94,7 @@ public static async ValueTask InvokeVoidAsync(this IJSObjectReference jsObjectRe
                 throw new ArgumentNullException(nameof(jsObjectReference));
             }
 
-            await jsObjectReference.InvokeAsync<object>(identifier, cancellationToken, args);
+            await jsObjectReference.InvokeAsync<IJSVoidResult>(identifier, cancellationToken, args);
         }
 
         /// <summary>
@@ -135,7 +136,7 @@ public static async ValueTask InvokeVoidAsync(this IJSObjectReference jsObjectRe
             using var cancellationTokenSource = timeout == Timeout.InfiniteTimeSpan ? null : new CancellationTokenSource(timeout);
             var cancellationToken = cancellationTokenSource?.Token ?? CancellationToken.None;
 
-            await jsObjectReference.InvokeAsync<object>(identifier, cancellationToken, args);
+            await jsObjectReference.InvokeAsync<IJSVoidResult>(identifier, cancellationToken, args);
         }
     }
 }
diff --git a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeExtensions.cs b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeExtensions.cs
@@ -5,6 +5,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.JSInterop.Infrastructure;
 using static Microsoft.AspNetCore.Internal.LinkerFlags;
 
 namespace Microsoft.JSInterop
@@ -28,7 +29,7 @@ public static async ValueTask InvokeVoidAsync(this IJSRuntime jsRuntime, string
                 throw new ArgumentNullException(nameof(jsRuntime));
             }
 
-            await jsRuntime.InvokeAsync<object>(identifier, args);
+            await jsRuntime.InvokeAsync<IJSVoidResult>(identifier, args);
         }
 
         /// <summary>
@@ -93,7 +94,7 @@ public static async ValueTask InvokeVoidAsync(this IJSRuntime jsRuntime, string
                 throw new ArgumentNullException(nameof(jsRuntime));
             }
 
-            await jsRuntime.InvokeAsync<object>(identifier, cancellationToken, args);
+            await jsRuntime.InvokeAsync<IJSVoidResult>(identifier, cancellationToken, args);
         }
 
         /// <summary>
@@ -135,7 +136,7 @@ public static async ValueTask InvokeVoidAsync(this IJSRuntime jsRuntime, string
             using var cancellationTokenSource = timeout == Timeout.InfiniteTimeSpan ? null : new CancellationTokenSource(timeout);
             var cancellationToken = cancellationTokenSource?.Token ?? CancellationToken.None;
 
-            await jsRuntime.InvokeAsync<object>(identifier, cancellationToken, args);
+            await jsRuntime.InvokeAsync<IJSVoidResult>(identifier, cancellationToken, args);
         }
     }
 }
diff --git a/src/JSInterop/Microsoft.JSInterop/src/PublicAPI.Unshipped.txt b/src/JSInterop/Microsoft.JSInterop/src/PublicAPI.Unshipped.txt
@@ -11,7 +11,9 @@ Microsoft.JSInterop.DotNetStreamReference.DotNetStreamReference(System.IO.Stream
 Microsoft.JSInterop.DotNetStreamReference.LeaveOpen.get -> bool
 Microsoft.JSInterop.DotNetStreamReference.Stream.get -> System.IO.Stream!
 Microsoft.JSInterop.Infrastructure.DotNetInvocationResult.ResultJson.get -> string?
+Microsoft.JSInterop.Infrastructure.IJSVoidResult
 Microsoft.JSInterop.JSCallResultType.JSStreamReference = 2 -> Microsoft.JSInterop.JSCallResultType
+Microsoft.JSInterop.JSCallResultType.JSVoidResult = 3 -> Microsoft.JSInterop.JSCallResultType
 Microsoft.JSInterop.JSRuntime.Dispose() -> void
 static Microsoft.JSInterop.Implementation.JSObjectReferenceJsonWorker.ReadJSObjectReferenceIdentifier(ref System.Text.Json.Utf8JsonReader reader) -> long
 static Microsoft.JSInterop.Implementation.JSObjectReferenceJsonWorker.WriteJSObjectReference(System.Text.Json.Utf8JsonWriter! writer, Microsoft.JSInterop.Implementation.JSObjectReference! objectReference) -> void
diff --git a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeExtensionsTest.cs b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeExtensionsTest.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.JSInterop.Infrastructure;
 using Moq;
 using Xunit;
 
@@ -65,7 +66,7 @@ public async Task InvokeVoidAsync_WithoutCancellationToken()
             var method = "someMethod";
             var args = new[] { "a", "b" };
             var jsRuntime = new Mock<IJSRuntime>(MockBehavior.Strict);
-            jsRuntime.Setup(s => s.InvokeAsync<object>(method, args)).Returns(new ValueTask<object>(new object()));
+            jsRuntime.Setup(s => s.InvokeAsync<IJSVoidResult>(method, args)).Returns(new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>()));
 
             // Act
             await jsRuntime.Object.InvokeVoidAsync(method, args);
@@ -80,7 +81,7 @@ public async Task InvokeVoidAsync_WithCancellationToken()
             var method = "someMethod";
             var args = new[] { "a", "b" };
             var jsRuntime = new Mock<IJSRuntime>(MockBehavior.Strict);
-            jsRuntime.Setup(s => s.InvokeAsync<object>(method, It.IsAny<CancellationToken>(), args)).Returns(new ValueTask<object>(new object()));
+            jsRuntime.Setup(s => s.InvokeAsync<IJSVoidResult>(method, It.IsAny<CancellationToken>(), args)).Returns(new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>()));
 
             // Act
             await jsRuntime.Object.InvokeVoidAsync(method, new CancellationToken(), args);
@@ -142,14 +143,14 @@ public async Task InvokeVoidAsync_WithTimeout()
             var method = "someMethod";
             var args = new[] { "a", "b" };
             var jsRuntime = new Mock<IJSRuntime>(MockBehavior.Strict);
-            jsRuntime.Setup(s => s.InvokeAsync<object>(method, It.IsAny<CancellationToken>(), args))
+            jsRuntime.Setup(s => s.InvokeAsync<IJSVoidResult>(method, It.IsAny<CancellationToken>(), args))
                 .Callback<string, CancellationToken, object[]>((method, cts, args) =>
                 {
                     // There isn't a very good way to test when the cts will cancel. We'll just verify that
                     // it'll get cancelled eventually.
                     Assert.True(cts.CanBeCanceled);
                 })
-                .Returns(new ValueTask<object>(new object()));
+                .Returns(new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>()));
 
             // Act
             await jsRuntime.Object.InvokeVoidAsync(method, TimeSpan.FromMinutes(5), args);
@@ -164,13 +165,13 @@ public async Task InvokeVoidAsync_WithInfiniteTimeout()
             var method = "someMethod";
             var args = new[] { "a", "b" };
             var jsRuntime = new Mock<IJSRuntime>(MockBehavior.Strict);
-            jsRuntime.Setup(s => s.InvokeAsync<object>(method, It.IsAny<CancellationToken>(), args))
+            jsRuntime.Setup(s => s.InvokeAsync<IJSVoidResult>(method, It.IsAny<CancellationToken>(), args))
                 .Callback<string, CancellationToken, object[]>((method, cts, args) =>
                 {
                     Assert.False(cts.CanBeCanceled);
                     Assert.True(cts == CancellationToken.None);
                 })
-                .Returns(new ValueTask<object>(new object()));
+                .Returns(new ValueTask<IJSVoidResult>(Mock.Of<IJSVoidResult>()));
 
             // Act
             await jsRuntime.Object.InvokeVoidAsync(method, Timeout.InfiniteTimeSpan, args);
diff --git a/src/Shared/JSInterop/JSCallResultTypeHelper.cs b/src/Shared/JSInterop/JSCallResultTypeHelper.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Reflection;
+using Microsoft.JSInterop.Infrastructure;
 
 namespace Microsoft.JSInterop
 {
@@ -12,21 +13,25 @@ internal static class JSCallResultTypeHelper
 
         public static JSCallResultType FromGeneric<TResult>()
         {
-            if (typeof(TResult).Assembly == _currentAssembly
-                && (typeof(TResult) == typeof(IJSObjectReference)
-                || typeof(TResult) == typeof(IJSInProcessObjectReference)
-                || typeof(TResult) == typeof(IJSUnmarshalledObjectReference)))
+            if (typeof(TResult).Assembly == _currentAssembly)
             {
-                return JSCallResultType.JSObjectReference;
-            }
-            else if (typeof(TResult).Assembly == _currentAssembly && typeof(TResult) == typeof(IJSStreamReference))
-            {
-                return JSCallResultType.JSStreamReference;
-            }
-            else
-            {
-                return JSCallResultType.Default;
+                if (typeof(TResult) == typeof(IJSObjectReference) ||
+                    typeof(TResult) == typeof(IJSInProcessObjectReference) ||
+                    typeof(TResult) == typeof(IJSUnmarshalledObjectReference))
+                {
+                    return JSCallResultType.JSObjectReference;
+                }
+                else if (typeof(TResult) == typeof(IJSStreamReference))
+                {
+                    return JSCallResultType.JSStreamReference;
+                }
+                else if (typeof(TResult) == typeof(IJSVoidResult))
+                {
+                    return JSCallResultType.JSVoidResult;
+                }
             }
+
+            return JSCallResultType.Default;
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -163,6 +163,8 @@ function invokeJSFromDotNet(callInfo: Pointer, arg0: any, arg1: any, arg2: any):`
`163`	`163`	`const streamReference = DotNet.createJSStreamReference(result);`
`164`	`164`	`const resultJson = JSON.stringify(streamReference);`
`165`	`165`	`return BINDING.js_string_to_mono_string(resultJson);`
	`166`	`+ case DotNet.JSCallResultType.JSVoidResult:`
	`167`	`+ return null;`
`166`	`168`	`default:`
`167`	`169`	throw new Error(`Invalid JS call result type '${resultType}'.`);
`168`	`170`	`}`
Original file line number	Diff line number	Diff line change
`@@ -91,6 +91,7 @@ internal TResult InvokeUnmarshalled<T0, T1, T2, TResult>(string identifier, T0 a`
`91`	`91`	`switch (resultType)`
`92`	`92`	`{`
`93`	`93`	`case JSCallResultType.Default:`
	`94`	`+ case JSCallResultType.JSVoidResult:`
`94`	`95`	`var result = InternalCalls.InvokeJS<T0, T1, T2, TResult>(out exception, ref callInfo, arg0, arg1, arg2);`
`95`	`96`	`return exception != null`
`96`	`97`	`? throw new JSException(exception)`