From da9f9aa09d832a5bb3301421e7a8521a4a13371c Mon Sep 17 00:00:00 2001
From: Ilona Tomkowicz <itomkowicz@microsoft.com>
Date: Wed, 12 Apr 2023 17:17:38 +0200
Subject: [PATCH] Pass string instead of pointer with length.

---
 docs/design/features/hybrid-globalization.md       |  2 +-
 .../src/Interop/Browser/Interop.CompareInfo.cs     |  2 +-
 .../Globalization/CompareInfo.WebAssembly.cs       |  9 ++-------
 src/mono/wasm/runtime/corebindings.c               |  2 +-
 .../runtime/net6-legacy/hybrid-globalization.ts    | 14 ++++++++++----
 5 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/docs/design/features/hybrid-globalization.md b/docs/design/features/hybrid-globalization.md
index c6767e7d0c08d..cbf94f49c5ee1 100644
--- a/docs/design/features/hybrid-globalization.md
+++ b/docs/design/features/hybrid-globalization.md
@@ -8,7 +8,7 @@ Hybrid mode does not use ICU data for some functions connected with globalizatio
 
 ### WASM
 
-For WebAssembly in Browser we are using Web API instead of some ICU data.
+For WebAssembly in Browser we are using Web API instead of some ICU data. Ideally, we would use `System.Runtime.InteropServices.JavaScript` to call JS code from inside of C# but we cannot reference any assemblies from inside of `System.Private.CoreLib`. That is why we are using iCalls for that.
 
 **Case change**
 
diff --git a/src/libraries/Common/src/Interop/Browser/Interop.CompareInfo.cs b/src/libraries/Common/src/Interop/Browser/Interop.CompareInfo.cs
index 693b908a0c691..a986c386b881c 100644
--- a/src/libraries/Common/src/Interop/Browser/Interop.CompareInfo.cs
+++ b/src/libraries/Common/src/Interop/Browser/Interop.CompareInfo.cs
@@ -8,6 +8,6 @@ internal static partial class Interop
     internal static unsafe partial class JsGlobalization
     {
         [MethodImplAttribute(MethodImplOptions.InternalCall)]
-        internal static extern unsafe int CompareString(out string exceptionMessage, in string culture, char* str1, int str1Len, char* str2, int str2Len, global::System.Globalization.CompareOptions options);
+        internal static extern unsafe int CompareString(out string exceptionMessage, in string culture, in string str1, in string str2, global::System.Globalization.CompareOptions options);
     }
 }
diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.WebAssembly.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.WebAssembly.cs
index 3ca001a9573bd..5620a385a6c79 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.WebAssembly.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.WebAssembly.cs
@@ -8,7 +8,7 @@ namespace System.Globalization
 {
     public partial class CompareInfo
     {
-        private unsafe int JsCompareString(ReadOnlySpan<char> string1, ReadOnlySpan<char> string2, CompareOptions options)
+        private unsafe int JsCompareString(ReadOnlySpan<char> span1, ReadOnlySpan<char> span2, CompareOptions options)
         {
             Debug.Assert(!GlobalizationMode.Invariant);
             Debug.Assert(!GlobalizationMode.UseNls);
@@ -25,12 +25,7 @@ private unsafe int JsCompareString(ReadOnlySpan<char> string1, ReadOnlySpan<char
                 throw new PlatformNotSupportedException(GetPNSEForCulture(options, cultureName));
 
             string exceptionMessage;
-            int cmpResult;
-            fixed (char* pString1 = &MemoryMarshal.GetReference(string1))
-            fixed (char* pString2 = &MemoryMarshal.GetReference(string2))
-            {
-                cmpResult = Interop.JsGlobalization.CompareString(out exceptionMessage, cultureName, pString1, string1.Length, pString2, string2.Length, options);
-            }
+            int cmpResult = Interop.JsGlobalization.CompareString(out exceptionMessage, cultureName, span1.ToString(), span2.ToString(), options);
 
             if (!string.IsNullOrEmpty(exceptionMessage))
                 throw new Exception(exceptionMessage);
diff --git a/src/mono/wasm/runtime/corebindings.c b/src/mono/wasm/runtime/corebindings.c
index d07e106668c3e..3c95deca8da94 100644
--- a/src/mono/wasm/runtime/corebindings.c
+++ b/src/mono/wasm/runtime/corebindings.c
@@ -45,7 +45,7 @@ extern void* mono_wasm_invoke_js_blazor (MonoString **exceptionMessage, void *ca
 // HybridGlobalization
 extern void mono_wasm_change_case_invariant(MonoString **exceptionMessage, const uint16_t* src, int32_t srcLength, uint16_t* dst, int32_t dstLength, mono_bool bToUpper);
 extern void mono_wasm_change_case(MonoString **exceptionMessage, MonoString **culture, const uint16_t* src, int32_t srcLength, uint16_t* dst, int32_t dstLength, mono_bool bToUpper);
-extern int mono_wasm_compare_string(MonoString **exceptionMessage, MonoString **culture, const uint16_t* str1, int32_t str1Length, const uint16_t* str2, int32_t str2Length, int32_t options);
+extern int mono_wasm_compare_string(MonoString **exceptionMessage, MonoString **culture, MonoString **str1, MonoString **str2, int32_t options);
 
 void bindings_initialize_internals (void)
 {
diff --git a/src/mono/wasm/runtime/net6-legacy/hybrid-globalization.ts b/src/mono/wasm/runtime/net6-legacy/hybrid-globalization.ts
index ae3ee2ac648f6..7829319394ca3 100644
--- a/src/mono/wasm/runtime/net6-legacy/hybrid-globalization.ts
+++ b/src/mono/wasm/runtime/net6-legacy/hybrid-globalization.ts
@@ -47,14 +47,18 @@ export function mono_wasm_change_case(exceptionMessage: Int32Ptr, culture: MonoS
     }
 }
 
-export function mono_wasm_compare_string(exceptionMessage: Int32Ptr, culture: MonoStringRef, str1: number, str1Length: number, str2: number, str2Length: number, options: number) : number{
+export function mono_wasm_compare_string(exceptionMessage: Int32Ptr, culture: MonoStringRef, str1: MonoStringRef, str2: MonoStringRef, options: number) : number{
     const cultureRoot = mono_wasm_new_external_root<MonoString>(culture);
+    const str1Root = mono_wasm_new_external_root<MonoString>(str1);
+    const str2Root = mono_wasm_new_external_root<MonoString>(str2);
     try{
+        const string1 = conv_string_root(str1Root);
+        const string2 = conv_string_root(str2Root);
+        if (string1 === null || string2 === null)
+            throw new Error("$String conversion failed.");
         const cultureName = conv_string_root(cultureRoot);
-        const string1  = get_utf16_string(str1, str1Length);
-        const string2 = get_utf16_string(str2, str2Length);
-        const casePicker = (options & 0x1f);
         const locale = cultureName ? cultureName : undefined;
+        const casePicker = (options & 0x1f);
         const result = compare_strings(string1, string2, locale, casePicker);
         if (result == -2)
             throw new Error("$Invalid comparison option.");
@@ -66,6 +70,8 @@ export function mono_wasm_compare_string(exceptionMessage: Int32Ptr, culture: Mo
     }
     finally {
         cultureRoot.release();
+        str1Root.release();
+        str2Root.release();
     }
 }