Fix regression with Safari and EXPORT_NAME

Fixes: #18357

Author: sbc100

.circleci/config.yml

@@ -601,7 +601,7 @@ jobs:
             echo "JSC_ENGINE = [os.path.expanduser('~/.jsvu/bin/javascriptcore')]" >> ~/emsdk/.emscripten
             echo "JS_ENGINES = [JSC_ENGINE]" >> ~/emsdk/.emscripten
       - run-tests:
-          test_targets: "core0.test_hello_world"
+          test_targets: "core0.test_hello_world other.test_modularize_incoming other.test_modularize_incoming_export_name"
   test-spidermonkey:
     executor: linux-python
     steps:

emcc.py

@@ -3229,8 +3229,7 @@ def phase_final_emitting(options, state, target, wasm_target, memfile):
 
   if settings.MODULARIZE:
     modularize()
-
-  if settings.USE_CLOSURE_COMPILER:
+  elif settings.USE_CLOSURE_COMPILER:
     module_export_name_substitution()
 
   # Run a final optimization pass to clean up items that were not possible to
@@ -3863,7 +3862,10 @@ def modularize():
      shared.target_environment_may_be('web'):
     async_emit = 'async '
 
-  return_value = settings.EXPORT_NAME
+  # Return ths incoming moduleArg.  This is is equeivielt to the `Module` var within the
+  # generated code is not effected by closure minifiection so we can references in the
+  # the return statement.
+  return_value = 'moduleArg'
   if settings.WASM_ASYNC_COMPILATION:
     return_value += '.ready'
   if not settings.EXPORT_READY_PROMISE:
@@ -3874,7 +3876,7 @@ def modularize():
     diagnostics.warning('emcc', 'EXPORT_NAME should not be named "config" when targeting Safari')
 
   src = '''
-%(maybe_async)sfunction(%(EXPORT_NAME)s = {})  {
+%(maybe_async)sfunction(moduleArg = {}) {
 
 %(src)s
 
@@ -3883,7 +3885,6 @@ def modularize():
 %(capture_module_function_for_audio_worklet)s
 ''' % {
     'maybe_async': async_emit,
-    'EXPORT_NAME': settings.EXPORT_NAME,
     'src': src,
     'return_value': return_value,
     # Given the async nature of how the Module function and Module object come into existence in AudioWorkletGlobalScope,
@@ -3945,6 +3946,7 @@ def modularize():
 
 
 def module_export_name_substitution():
+  assert not settings.MODULARIZE
   global final_js
   logger.debug(f'Private module export name substitution with {settings.EXPORT_NAME}')
   src = read_file(final_js)

src/closure-externs/closure-externs.js

@@ -261,3 +261,5 @@ var sampleRate;
  * Avoid closure minifying anything to "id". See #13965
  */
 var id;
+
+var moduleArg;

src/preamble.js

@@ -630,6 +630,11 @@ if (Module['locateFile']) {
   }
 #if EXPORT_ES6 && USE_ES6_IMPORT_META && !SINGLE_FILE // in single-file mode, repeating WASM_BINARY_FILE would emit the contents again
 } else {
+#if ENVIRONMENT_MAY_BE_SHELL
+  if (ENVIRONMENT_IS_SHELL)
+    wasmBinaryFile = '{{{ WASM_BINARY_FILE }}}';
+  else
+#endif
   // Use bundler-friendly `new URL(..., import.meta.url)` pattern; works in browsers too.
   wasmBinaryFile = new URL('{{{ WASM_BINARY_FILE }}}', import.meta.url).href;
 }

src/shell.js

@@ -20,7 +20,9 @@
 // after the generated code, you will need to define   var Module = {};
 // before the code. Then that object will be used in the code, and you
 // can continue to use Module afterwards as well.
-#if USE_CLOSURE_COMPILER
+#if MODULARIZE
+var Module = moduleArg;
+#elif USE_CLOSURE_COMPILER
 // if (!Module)` is crucial for Closure Compiler here as it will otherwise replace every `Module` occurrence with a string
 var /** @type {{
   noImageDecoding: boolean,
@@ -318,19 +320,16 @@ if (ENVIRONMENT_IS_SHELL) {
     return data;
   };
 
+  // This shell version of readAsync is not techically async, since we don't
+  // have an async `read()` available.
   readAsync = (f, onload, onerror) => {
-    setTimeout(() => onload(readBinary(f)));
+    onload(readBinary(f));
   };
 
   if (typeof clearTimeout == 'undefined') {
     globalThis.clearTimeout = (id) => {};
   }
 
-  if (typeof setTimeout == 'undefined') {
-    // spidermonkey lacks setTimeout but we use it above in readAsync.
-    globalThis.setTimeout = (f) => (typeof f == 'function') ? f() : abort();
-  }
-
   if (typeof scriptArgs != 'undefined') {
     arguments_ = scriptArgs;
   } else if (typeof arguments != 'undefined') {

src/shell_minimal.js

@@ -4,8 +4,9 @@
  * SPDX-License-Identifier: MIT
  */
 
-#if USE_CLOSURE_COMPILER
-
+#if MODULARIZE
+var Module = moduleArg;
+#elif USE_CLOSURE_COMPILER
 // if (!Module)` is crucial for Closure Compiler here as it will
 // otherwise replace every `Module` occurrence with the object below
 var /** @type{Object} */ Module;
@@ -15,7 +16,7 @@ if (!Module) /** @suppress{checkTypes}*/Module =
 #endif
   {"__EMSCRIPTEN_PRIVATE_MODULE_EXPORT_NAME_SUBSTITUTION__":1};
 
-#elif !MODULARIZE && (ENVIRONMENT_MAY_BE_NODE || ENVIRONMENT_MAY_BE_SHELL)
+#elif ENVIRONMENT_MAY_BE_NODE || ENVIRONMENT_MAY_BE_SHELL
 
 // When running on the web we expect Module to be defined externally, in the
 // HTML.  Otherwise we must define it here before its first use

test/code_size/hello_webgl2_wasm2js.json

@@ -1,10 +1,10 @@
 {
   "a.html": 567,
   "a.html.gz": 379,
-  "a.js": 18209,
-  "a.js.gz": 8055,
+  "a.js": 18218,
+  "a.js.gz": 8059,
   "a.mem": 3171,
   "a.mem.gz": 2713,
-  "total": 21947,
-  "total_gz": 11147
+  "total": 21956,
+  "total_gz": 11151
 }

test/code_size/hello_webgl_wasm2js.json

@@ -1,10 +1,10 @@
 {
   "a.html": 567,
   "a.html.gz": 379,
-  "a.js": 17681,
-  "a.js.gz": 7872,
+  "a.js": 17689,
+  "a.js.gz": 7874,
   "a.mem": 3171,
   "a.mem.gz": 2713,
-  "total": 21419,
-  "total_gz": 10964
+  "total": 21427,
+  "total_gz": 10966
 }

test/other/metadce/test_metadce_hello_O0.jssize

@@ -1 +1 @@
-23635
+23614

test/other/metadce/test_metadce_minimal_O0.jssize

@@ -1 +1 @@
-20062
+20041