[browser] fix trimming of LegacyExports with WasmEnableLegacyJsInterop

eng/testing/tests.browser.targets

@@ -41,6 +41,21 @@
     <WasmIncludeFullIcuData>true</WasmIncludeFullIcuData>
   </PropertyGroup>
 
+  <PropertyGroup Condition="'$(BuildAOTTestsOn)' == 'local'">
+    <!--
+      When building for BuildAOTTestsOnHelix=true, the WasmApp*targets are *not* imported, because
+      they get instead used by the AOT proxy project on helix.
+
+      On the build machine only the regular part of the build is run, which includes trimming. But if
+      WasmApp*targets modify any trimming arguments, then those will not get picked up by this build.
+      For example - linker substitution files used with simd builds.
+
+      So, set those parameters explicitly here.
+      -->
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MonoProjectRoot)\wasm\build\ILLink.Substitutions.LegacyJsInterop.xml&quot;</_ExtraTrimmerArgs>
+  </PropertyGroup>
+
+
   <!-- On CI this is installed as part of pretest, but it should still be installed
        for WBT, and debugger tests -->
   <Import Project="$(MSBuildThisFileDirectory)wasm-provisioning.targets"
@@ -125,11 +140,6 @@
   <PropertyGroup Condition="'$(BuildAOTTestsOnHelix)' == 'true'">
     <!-- wasm targets are not imported at all, in this case, because we run the wasm build on helix -->
   </PropertyGroup>
-  <ItemGroup Condition="'$(BuildAOTTestsOn)' == 'helix'">
-    <TrimmerRootDescriptor
-      Condition="'$(WasmEnableLegacyJsInterop)' == 'true'"
-      Include="$(MonoProjectRoot)\wasm\build\ILLink.Descriptors.LegacyJsInterop.xml" />
-  </ItemGroup>
 
   <PropertyGroup Condition="'$(IsWasmProject)' == 'true' and '$(BuildAOTTestsOnHelix)' != 'true'">
     <WasmBuildOnlyAfterPublish>true</WasmBuildOnlyAfterPublish>

src/installer/pkg/sfx/Microsoft.NETCore.App/Directory.Build.props

@@ -253,7 +253,7 @@
     <PlatformManifestFileEntry Include="emcc-props.json" IsNative="true" />
     <PlatformManifestFileEntry Include="ILLink.Substitutions.WasmIntrinsics.xml" IsNative="true" />
     <PlatformManifestFileEntry Include="ILLink.Substitutions.NoWasmIntrinsics.xml" IsNative="true" />
-    <PlatformManifestFileEntry Include="ILLink.Descriptors.LegacyJsInterop.xml" IsNative="true" />
+    <PlatformManifestFileEntry Include="ILLink.Substitutions.LegacyJsInterop.xml" IsNative="true" />
     <!-- wasi specific -->
     <PlatformManifestFileEntry Include="main.c" IsNative="true" />
     <PlatformManifestFileEntry Include="driver.h" IsNative="true" />

src/libraries/System.Runtime.InteropServices.JavaScript/src/System.Runtime.InteropServices.JavaScript.csproj

@@ -12,10 +12,7 @@
     <FeatureWasmThreads Condition="'$(TargetPlatformIdentifier)' == 'browser' and '$(MonoWasmBuildVariant)' == 'multithread'">true</FeatureWasmThreads>
     <WasmEnableLegacyJsInterop Condition="'$(TargetPlatformIdentifier)' == 'browser' and '$(WasmEnableLegacyJsInterop)' == ''">true</WasmEnableLegacyJsInterop>
     <DefineConstants Condition="'$(FeatureWasmThreads)' == 'true'" >$(DefineConstants);FEATURE_WASM_THREADS</DefineConstants>
-    <DefineConstants Condition="'$(WasmEnableLegacyJsInterop)' == 'true'" >$(DefineConstants);ENABLE_LEGACY_JS_INTEROP</DefineConstants>
-    <ILLinkDescriptorsLibraryBuildXml
-      Condition="'$(TargetPlatformIdentifier)' == 'browser' and '$(WasmEnableLegacyJsInterop)' == 'true'"
-      >$(MonoProjectRoot)wasm\build\ILLink.Descriptors.LegacyJsInterop.xml</ILLinkDescriptorsLibraryBuildXml>
+    <DefineConstants Condition="'$(WasmEnableLegacyJsInterop)' == 'false'" >$(DefineConstants);DISABLE_LEGACY_JS_INTEROP</DefineConstants>
   </PropertyGroup>
 
   <ItemGroup>
@@ -66,7 +63,7 @@
   </ItemGroup>
 
   <!-- only include legacy interop when WasmEnableLegacyJsInterop is enabled -->
-  <ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'browser' and '$(WasmEnableLegacyJsInterop)' == 'true'">
+  <ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'browser' and '$(WasmEnableLegacyJsInterop)' != 'false'">
     <Compile Include="System\Runtime\InteropServices\JavaScript\Interop\LegacyExports.cs" />
     <Compile Include="System\Runtime\InteropServices\JavaScript\Legacy\Runtime.cs" />
     <Compile Include="System\Runtime\InteropServices\JavaScript\Legacy\Array.cs" />

src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/JavaScriptImports.cs

@@ -20,7 +20,7 @@ public static void MarshalPromise(Span<JSMarshalerArgument> arguments)
             }
         }
 
-#if ENABLE_LEGACY_JS_INTEROP
+#if !DISABLE_LEGACY_JS_INTEROP
         #region legacy
 
         public static object GetGlobalObject(string? str = null)

src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Interop/LegacyExports.cs

@@ -2,16 +2,34 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Text;
 using System.Threading.Tasks;
 
 namespace System.Runtime.InteropServices.JavaScript
 {
-    // the public methods are protected from trimming by ILLink.Descriptors.LegacyJsInterop.xml
+    internal static unsafe partial class LegacyExportsTrimmingRoot
+    {
+        // the public methods are used from JavaScript, but the trimmer doesn't know about it.
+        // It's protected by DynamicDependencyAttribute on JSFunctionBinding.BindJSFunction.
+        public static void TrimWhenNotWasmEnableLegacyJsInterop()
+        {
+            // if MSBuild property WasmEnableLegacyJsInterop==false this call would be substituted away and LegacyExports would be trimmed.
+            LegacyExports.PreventTrimming();
+        }
+    }
+
     internal static unsafe partial class LegacyExports
     {
+        // the public methods of this class are used from JavaScript, but the trimmer doesn't know about it.
+        // They are protected by LegacyExportsTrimmingRoot.PreventTrimming and JSFunctionBinding.BindJSFunction.
+        [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(LegacyExports))]
+        internal static void PreventTrimming()
+        {
+        }
+
         public static void GetCSOwnedObjectByJSHandleRef(nint jsHandle, int shouldAddInflight, out JSObject? result)
         {
             lock (JSHostImplementation.s_csOwnedObjects)

src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSFunctionBinding.cs

@@ -142,8 +142,8 @@ public static void InvokeJS(JSFunctionBinding signature, Span<JSMarshalerArgumen
         /// <exception cref="PlatformNotSupportedException">The method is executed on an architecture other than WebAssembly.</exception>
         // JavaScriptExports need to be protected from trimming because they are used from C/JS code which IL linker can't see
         [DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, "System.Runtime.InteropServices.JavaScript.JavaScriptExports", "System.Runtime.InteropServices.JavaScript")]
-        // TODO make this DynamicDependency conditional again, see https://github.com/dotnet/runtime/pull/82826
-        [DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, "System.Runtime.InteropServices.JavaScript.LegacyExports", "System.Runtime.InteropServices.JavaScript")]
+        // Same for legacy, but the type could be explicitly trimmed by setting WasmEnableLegacyJsInterop=false which would use ILLink.Descriptors.LegacyJsInterop.xml
+        [DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, "System.Runtime.InteropServices.JavaScript.LegacyExportsTrimmingRoot", "System.Runtime.InteropServices.JavaScript")]
         public static JSFunctionBinding BindJSFunction(string functionName, string moduleName, ReadOnlySpan<JSMarshalerType> signatures)
         {
             if (RuntimeInformation.OSArchitecture != Architecture.Wasm)

src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSObject.References.cs

@@ -10,7 +10,7 @@ public partial class JSObject
     {
         internal nint JSHandle;
 
-#if ENABLE_LEGACY_JS_INTEROP
+#if !DISABLE_LEGACY_JS_INTEROP
         internal GCHandle? InFlight;
         internal int InFlightCounter;
 #endif
@@ -21,7 +21,7 @@ internal JSObject(IntPtr jsHandle)
             JSHandle = jsHandle;
         }
 
-#if ENABLE_LEGACY_JS_INTEROP
+#if !DISABLE_LEGACY_JS_INTEROP
         internal void AddInFlight()
         {
             ObjectDisposedException.ThrowIf(IsDisposed, this);

src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.Legacy.UnitTests/System.Runtime.InteropServices.JavaScript.Legacy.UnitTests.csproj

@@ -6,10 +6,10 @@
     <WasmXHarnessArgs>$(WasmXHarnessArgs) --engine-arg=--expose-gc --web-server-use-cop</WasmXHarnessArgs>
     <NoWarn>0612</NoWarn>
     <WasmEnableLegacyJsInterop Condition="'$(WasmEnableLegacyJsInterop)' == ''">true</WasmEnableLegacyJsInterop>
-    <DefineConstants Condition="'$(WasmEnableLegacyJsInterop)' == 'true'">$(DefineConstants);ENABLE_LEGACY_JS_INTEROP</DefineConstants>
+    <DefineConstants Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(DefineConstants);DISABLE_LEGACY_JS_INTEROP</DefineConstants>
   </PropertyGroup>
 
-  <ItemGroup Condition="'$(WasmEnableLegacyJsInterop)' == 'true'">
+  <ItemGroup Condition="'$(WasmEnableLegacyJsInterop)' != 'false'">
     <Compile Include="System\Runtime\InteropServices\JavaScript\JavaScriptTests.cs" />
     <Compile Include="System\Runtime\InteropServices\JavaScript\DataViewTests.cs" />
     <Compile Include="System\Runtime\InteropServices\JavaScript\MemoryTests.cs" />

src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System.Runtime.InteropServices.JavaScript.Tests.csproj

@@ -4,8 +4,10 @@
     <TargetFrameworks>$(NetCoreAppCurrent)-browser</TargetFrameworks>
     <TestRuntime>true</TestRuntime>
     <WasmXHarnessArgs>$(WasmXHarnessArgs) --engine-arg=--expose-gc --web-server-use-cop</WasmXHarnessArgs>
-    <WasmEnableLegacyJsInterop Condition="'$(WasmEnableLegacyJsInterop)' == ''">true</WasmEnableLegacyJsInterop>
-    <DefineConstants Condition="'$(WasmEnableLegacyJsInterop)' == 'true'">$(DefineConstants);ENABLE_LEGACY_JS_INTEROP</DefineConstants>
+    <EnableAggressiveTrimming>true</EnableAggressiveTrimming>
+    <PublishTrimmed>true</PublishTrimmed>
+    <WasmEnableLegacyJsInterop>false</WasmEnableLegacyJsInterop>
+    <DefineConstants Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(DefineConstants);DISABLE_LEGACY_JS_INTEROP</DefineConstants>
     <!-- Use following lines to write the generated files to disk. -->
     <EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
   </PropertyGroup>

src/libraries/System.Runtime.InteropServices.JavaScript/tests/System.Runtime.InteropServices.JavaScript.UnitTests/System/Runtime/InteropServices/JavaScript/JSImportExportTest.cs

@@ -58,7 +58,7 @@ public unsafe void GlobalThis()
         [Fact]
         public unsafe void DotnetInstance()
         {
-#if ENABLE_LEGACY_JS_INTEROP
+#if !DISABLE_LEGACY_JS_INTEROP
             Assert.True(JSHost.DotnetInstance.HasProperty("MONO"));
             Assert.Equal("object", JSHost.DotnetInstance.GetTypeOfProperty("MONO"));
 #endif

src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Current.Manifest/WorkloadManifest.targets.in

@@ -40,7 +40,7 @@
   <PropertyGroup Condition="'$(RuntimeIdentifier)' == 'browser-wasm' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
     <!-- $(WasmBuildNative)==true is needed to enable workloads, when using native references, without AOT -->
     <!-- FIXME: is the blazor condition here correct? -->
-    <_WasmNativeWorkloadNeeded Condition="'$(RunAOTCompilation)' == 'true' or '$(WasmEnableSIMD)' == 'true' or '$(WasmBuildNative)' == 'true' or
+    <_WasmNativeWorkloadNeeded Condition="'$(RunAOTCompilation)' == 'true' or '$(WasmEnableSIMD)' == 'true' or '$(WasmEnableLegacyJsInterop)' == 'false' or '$(WasmBuildNative)' == 'true' or
       '$(WasmGenerateAppBundle)' == 'true' or '$(_UsingBlazorOrWasmSdk)' != 'true'" >true</_WasmNativeWorkloadNeeded>
 
     <UsingBrowserRuntimeWorkload Condition="'$(_BrowserWorkloadNotSupportedForTFM)' == 'true'">false</UsingBrowserRuntimeWorkload>

src/mono/sample/wasm/browser-advanced/Wasm.Advanced.Sample.csproj

@@ -3,6 +3,7 @@
   <PropertyGroup>
     <EnableAggressiveTrimming>true</EnableAggressiveTrimming>
     <PublishTrimmed>true</PublishTrimmed>
+    <WasmEnableLegacyJsInterop>false</WasmEnableLegacyJsInterop>
     <WasmEnableWebcil>true</WasmEnableWebcil>
     <WasmEmitSymbolMap>true</WasmEmitSymbolMap>
     <EmccEnableAssertions>true</EmccEnableAssertions>

src/mono/sample/wasm/browser-advanced/main.js

@@ -29,11 +29,11 @@ try {
                 fetch: (url, fetchArgs) => {
                     console.log("fetching " + url);
                     // we are testing that we can retry loading of the assembly
-                    if (testAbort && url.indexOf('System.Private.Uri.dll') != -1) {
+                    if (testAbort && url.indexOf('System.Private.CoreLib') != -1) {
                         testAbort = false;
                         return fetch(url + "?testAbort=true", fetchArgs);
                     }
-                    if (testError && url.indexOf('System.Console.dll') != -1) {
+                    if (testError && url.indexOf('System.Console') != -1) {
                         testError = false;
                         return fetch(url + "?testError=true", fetchArgs);
                     }

src/mono/wasm/build/ILLink.Substitutions.LegacyJsInterop.xml

@@ -0,0 +1,7 @@
+<linker>
+  <assembly fullname="System.Runtime.InteropServices.JavaScript">
+    <type fullname="System.Runtime.InteropServices.JavaScript.LegacyExportsTrimmingRoot">
+      <method signature="System.Void TrimWhenNotWasmEnableLegacyJsInterop()" body="remove" />
+    </type>
+  </assembly>
+</linker>

src/mono/wasm/build/WasmApp.Native.targets

@@ -113,6 +113,9 @@
       <!-- build AOT, only if explicitly requested -->
       <WasmBuildNative Condition="'$(RunAOTCompilation)' == 'true' and '$(RunAOTCompilationAfterBuild)' == 'true'">true</WasmBuildNative>
 
+      <!-- removing legacy interop needs relink -->
+      <WasmBuildNative Condition="'$(WasmBuildNative)' == '' and '$(WasmEnableLegacyJsInterop)' == 'false'" >true</WasmBuildNative>
+
       <WasmBuildNative Condition="'$(WasmBuildNative)' == '' and @(NativeFileReference->Count()) > 0" >true</WasmBuildNative>
 
       <WasmBuildNative Condition="'$(WasmBuildNative)' == ''">false</WasmBuildNative>
@@ -124,6 +127,9 @@
       <WasmBuildNative Condition="'$(RunAOTCompilation)' == 'true'">true</WasmBuildNative>
       <WasmBuildNative Condition="'$(WasmBuildNative)' == '' and @(NativeFileReference->Count()) > 0" >true</WasmBuildNative>
 
+      <!-- removing legacy interop needs relink -->
+      <WasmBuildNative Condition="'$(WasmBuildNative)' == '' and '$(WasmEnableLegacyJsInterop)' == 'false'" >true</WasmBuildNative>
+
       <!-- not aot, not trimmed app, no reason to relink -->
       <WasmBuildNative Condition="'$(WasmBuildNative)' == '' and '$(PublishTrimmed)' != 'true'">false</WasmBuildNative>
 
@@ -188,6 +194,7 @@
       <EmccInitialHeapSize Condition="'$(EmccInitialHeapSize)' == ''">$(EmccTotalMemory)</EmccInitialHeapSize>
       <EmccStackSize Condition="'$(EmccStackSize)' == ''">5MB</EmccStackSize>
       <WasmAllowUndefinedSymbols Condition="'$(WasmAllowUndefinedSymbols)' == ''">false</WasmAllowUndefinedSymbols>
+      <WasmEnableLegacyJsInterop Condition="'$(WasmEnableLegacyJsInterop)' == ''">true</WasmEnableLegacyJsInterop>
     </PropertyGroup>
 
     <ItemGroup>
@@ -217,6 +224,8 @@
       <_EmccCFlags Include="-DLINK_ICALLS=1"                   Condition="'$(WasmLinkIcalls)' == 'true'" />
       <_EmccCFlags Include="-DENABLE_AOT_PROFILER=1"           Condition="$(WasmProfilers.Contains('aot'))" />
       <_EmccCFlags Include="-DENABLE_BROWSER_PROFILER=1"       Condition="$(WasmProfilers.Contains('browser'))" />
+      <_EmccCFlags Include="-DDISABLE_LEGACY_JS_INTEROP=1"     Condition="'$(WasmEnableLegacyJsInterop)' == 'false'" />
+
       <_EmccCFlags Include="-DGEN_PINVOKE=1" />
       <_EmccCFlags Include="-emit-llvm" />
 
@@ -256,7 +265,8 @@
       <EmscriptenEnvVars Include="PYTHONHOME=" Condition="'$(OS)' == 'Windows_NT'" />
       <EmscriptenEnvVars Include="EM_CACHE=$(WasmCachePath)" Condition="'$(WasmCachePath)' != ''" />
       <EmscriptenEnvVars Include="EM_FROZEN_CACHE=True" />
-      <EmscriptenEnvVars Include="WasmEnableLegacyJsInterop=$(WasmEnableLegacyJsInterop)"/>
+      <EmscriptenEnvVars Include="DISABLE_LEGACY_JS_INTEROP=1" Condition="'$(WasmEnableLegacyJsInterop)' == 'false'" />
+      <EmscriptenEnvVars Include="DISABLE_LEGACY_JS_INTEROP=0" Condition="'$(WasmEnableLegacyJsInterop)' != 'false'" />
     </ItemGroup>
 
     <ItemGroup Condition="'$(WasmAllowUndefinedSymbols)' == 'true'">

src/mono/wasm/build/WasmApp.targets

@@ -124,6 +124,7 @@
     <TrimMode Condition="'$(TrimMode)' == ''">full</TrimMode>
     <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' == 'true' and '$(RunAOTCompilation)' == 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.WasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
     <_ExtraTrimmerArgs Condition="'$(WasmEnableSIMD)' != 'true' or '$(RunAOTCompilation)' != 'true'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.NoWasmIntrinsics.xml&quot;</_ExtraTrimmerArgs>
+    <_ExtraTrimmerArgs Condition="'$(WasmEnableLegacyJsInterop)' == 'false'">$(_ExtraTrimmerArgs) --substitutions &quot;$(MSBuildThisFileDirectory)ILLink.Substitutions.LegacyJsInterop.xml&quot;</_ExtraTrimmerArgs>
 
     <!-- Temporarily `false`, till sdk gets a fix for supporting the new file -->
     <WasmEmitSymbolMap Condition="'$(WasmEmitSymbolMap)' == '' and '$(RunAOTCompilation)' != 'true'">false</WasmEmitSymbolMap>
@@ -141,10 +142,6 @@
 
     <SupportedPlatform Condition="'$(IsBrowserWasmProject)' == 'true'" Remove="@(SupportedPlatform)" />
     <SupportedPlatform Condition="'$(IsBrowserWasmProject)' == 'true'" Include="browser" />
-
-    <TrimmerRootDescriptor
-      Condition="'$(WasmEnableLegacyJsInterop)' == 'true'"
-      Include="$(MSBuildThisFileDirectory)ILLink.Descriptors.LegacyJsInterop.xml" />
   </ItemGroup>
 
   <PropertyGroup Label="Identify app bundle directory to run from">

src/mono/wasm/runtime/CMakeLists.txt

@@ -4,7 +4,7 @@ project(mono-wasm-runtime C)
 
 option(DISABLE_THREADS "defined if the build does NOT support multithreading" ON)
 option(DISABLE_WASM_USER_THREADS "defined if the build does not allow user threads to be created in a multithreaded build" OFF)
-option(ENABLE_LEGACY_JS_INTEROP "defined if the build supports legacy JavaScript interop" ON)
+option(DISABLE_LEGACY_JS_INTEROP "defined if the build does not support legacy JavaScript interop" OFF)
 
 set(CMAKE_EXECUTABLE_SUFFIX ".js")
 add_executable(dotnet corebindings.c driver.c pinvoke.c)