Fix closure warnings in JS library code

For some reason closure doesn't report these real issues in the current
configuration but they show up in the `MODULARIZE_INSTANCE`
configuration that I'm working on.

Most of these issues consist of adding default parameters to functions
declared on objects such as `FS`.

In addition there are a couple of other changes that are all real fixes:

- ErrnoError unified between old FS and wasm FS.  This should have been
  done as part of emscripten-core#21149
- Default initializer for `canvasResizedCallbackTargetThread` removed
  since `JSEvents.getTargetThreadForEventCallback()` will always return
  undefined when called with no arguments
- Extra arguments to copyIndexedColorData removed in library_sdl.js
  since it only ever called with one arg.
- Sorting callback for JSEvents.deferredCalls.sort updated to correctly
  return number rather than a boolean.

This change is basically code size neutral with a few tests coming in
a byte or two heavier and others a byte or two lighter.

Author: sbc100

src/library_browser.js

@@ -54,6 +54,9 @@ var LibraryBrowser = {
       timingValue: 0,
       currentFrameNumber: 0,
       queue: [],
+#if USE_CLOSURE_COMPILER
+      expectedBlockers: 0,
+#endif
       pause() {
         Browser.mainLoop.scheduler = null;
         // Incrementing this signals the previous main loop that it's now become old, and it must return.
@@ -628,7 +631,7 @@ var LibraryBrowser = {
       Browser.resizeListeners.forEach((listener) => listener(canvas.width, canvas.height));
     },
 
-    setCanvasSize(width, height, noUpdates) {
+    setCanvasSize(width, height, noUpdates = false) {
       var canvas = Module['canvas'];
       Browser.updateCanvasDimensions(canvas, width, height);
       if (!noUpdates) Browser.updateResizeListeners();
@@ -658,7 +661,7 @@ var LibraryBrowser = {
       Browser.updateResizeListeners();
     },
 
-    updateCanvasDimensions(canvas, wNative, hNative) {
+    updateCanvasDimensions(canvas, wNative = 0, hNative = 0) {
       if (wNative && hNative) {
         canvas.widthNative = wNative;
         canvas.heightNative = hNative;

src/library_fs.js

@@ -51,7 +51,6 @@ FS.staticInit();` +
     devices: {},
     streams: [],
     nextInode: 1,
-    nameTable: null,
     currentPath: '/',
     initialized: false,
     // Whether we are currently ignoring permissions. Useful when preparing the
@@ -62,40 +61,10 @@ FS.staticInit();` +
 #if FS_DEBUG
     trackingDelegate: {},
 #endif
-    ErrnoError: null, // set during init
     genericErrors: {},
     filesystems: null,
     syncFSRequests: 0, // we warn if there are multiple in flight at once
-
-#if ASSERTIONS
-    ErrnoError: class extends Error {
-#else
-    ErrnoError: class {
-#endif
-      // We set the `name` property to be able to identify `FS.ErrnoError`
-      // - the `name` is a standard ECMA-262 property of error objects. Kind of good to have it anyway.
-      // - when using PROXYFS, an error can come from an underlying FS
-      // as different FS objects have their own FS.ErrnoError each,
-      // the test `err instanceof FS.ErrnoError` won't detect an error coming from another filesystem, causing bugs.
-      // we'll use the reliable test `err.name == "ErrnoError"` instead
-      constructor(errno) {
-#if ASSERTIONS
-        super(ERRNO_MESSAGES[errno]);
-#endif
-        // TODO(sbc): Use the inline member declaration syntax once we
-        // support it in acorn and closure.
-        this.name = 'ErrnoError';
-        this.errno = errno;
-#if ASSERTIONS
-        for (var key in ERRNO_CODES) {
-          if (ERRNO_CODES[key] === errno) {
-            this.code = key;
-            break;
-          }
-        }
-#endif
-      }
-    },
+    ErrnoError: ErrnoError,
 
     FSStream: class {
       constructor() {
@@ -304,7 +273,7 @@ FS.staticInit();` +
       // if we failed to find it in the cache, call into the VFS
       return FS.lookup(parent, name);
     },
-    createNode(parent, name, mode, rdev) {
+    createNode(parent, name, mode, rdev = 0) {
 #if ASSERTIONS
       assert(typeof parent == 'object')
 #endif
@@ -673,13 +642,13 @@ FS.staticInit();` +
       return parent.node_ops.mknod(parent, name, mode, dev);
     },
     // helpers to create specific types of nodes
-    create(path, mode) {
+    create(path, mode = undefined) {
       mode = mode !== undefined ? mode : 438 /* 0666 */;
       mode &= {{{ cDefs.S_IALLUGO }}};
       mode |= {{{ cDefs.S_IFREG }}};
       return FS.mknod(path, mode, 0);
     },
-    mkdir(path, mode) {
+    mkdir(path, mode = undefined) {
       mode = mode !== undefined ? mode : 511 /* 0777 */;
       mode &= {{{ cDefs.S_IRWXUGO }}} | {{{ cDefs.S_ISVTX }}};
       mode |= {{{ cDefs.S_IFDIR }}};
@@ -691,7 +660,7 @@ FS.staticInit();` +
       return FS.mknod(path, mode, 0);
     },
     // Creates a whole directory tree chain if it doesn't yet exist
-    mkdirTree(path, mode) {
+    mkdirTree(path, mode = undefined) {
       var dirs = path.split('/');
       var d = '';
       for (var i = 0; i < dirs.length; ++i) {
@@ -704,7 +673,7 @@ FS.staticInit();` +
         }
       }
     },
-    mkdev(path, mode, dev) {
+    mkdev(path, mode, dev = undefined) {
       if (typeof dev == 'undefined') {
         dev = mode;
         mode = 438 /* 0666 */;
@@ -912,7 +881,7 @@ FS.staticInit();` +
       }
       return PATH_FS.resolve(FS.getPath(link.parent), link.node_ops.readlink(link));
     },
-    stat(path, dontFollow) {
+    stat(path, dontFollow = false) {
       var lookup = FS.lookupPath(path, { follow: !dontFollow });
       var node = lookup.node;
       if (!node) {
@@ -926,7 +895,7 @@ FS.staticInit();` +
     lstat(path) {
       return FS.stat(path, true);
     },
-    chmod(path, mode, dontFollow) {
+    chmod(path, mode, dontFollow = false) {
       var node;
       if (typeof path == 'string') {
         var lookup = FS.lookupPath(path, { follow: !dontFollow });
@@ -949,7 +918,7 @@ FS.staticInit();` +
       var stream = FS.getStreamChecked(fd);
       FS.chmod(stream.node, mode);
     },
-    chown(path, uid, gid, dontFollow) {
+    chown(path, uid, gid, dontFollow = false) {
       var node;
       if (typeof path == 'string') {
         var lookup = FS.lookupPath(path, { follow: !dontFollow });
@@ -1015,7 +984,7 @@ FS.staticInit();` +
         timestamp: Math.max(atime, mtime)
       });
     },
-    open(path, flags, mode) {
+    open(path, flags, mode = undefined) {
       if (path === "") {
         throw new FS.ErrnoError({{{ cDefs.ENOENT }}});
       }
@@ -1193,7 +1162,7 @@ FS.staticInit();` +
 #endif
       return bytesRead;
     },
-    write(stream, buffer, offset, length, position, canOwn) {
+    write(stream, buffer, offset, length, position = undefined, canOwn = false) {
 #if ASSERTIONS
       assert(offset >= 0);
 #endif
@@ -1466,7 +1435,7 @@ FS.staticInit();` +
 #endif
       };
     },
-    init(input, output, error) {
+    init(input = undefined, output = undefined, error = undefined) {
 #if ASSERTIONS
       assert(!FS.init.initialized, 'FS.init was previously called. If you want to initialize later with custom parameters, remove any earlier calls (note that one is automatically added to the generated code)');
 #endif
@@ -1505,7 +1474,7 @@ FS.staticInit();` +
       }
       return ret.object;
     },
-    analyzePath(path, dontResolveLastLink) {
+    analyzePath(path, dontResolveLastLink = false) {
       // operate from within the context of the symlink's target
       try {
         var lookup = FS.lookupPath(path, { follow: !dontResolveLastLink });
@@ -1576,7 +1545,7 @@ FS.staticInit();` +
         FS.chmod(node, mode);
       }
     },
-    createDevice(parent, name, input, output) {
+    createDevice(parent, name, input, output = undefined) {
       var path = PATH.join2(typeof parent == 'string' ? parent : FS.getPath(parent), name);
       var mode = FS_getMode(!!input, !!output);
       if (!FS.createDevice.major) FS.createDevice.major = 64;

src/library_fs_shared.js

@@ -4,6 +4,36 @@
  * SPDX-License-Identifier: MIT
  */
 
+#if ASSERTIONS
+class ErrnoError extends Error {
+#else
+class ErrnoError {
+#endif
+  // We set the `name` property to be able to identify `FS.ErrnoError`
+  // - the `name` is a standard ECMA-262 property of error objects. Kind of good to have it anyway.
+  // - when using PROXYFS, an error can come from an underlying FS
+  // as different FS objects have their own FS.ErrnoError each,
+  // the test `err instanceof FS.ErrnoError` won't detect an error coming from another filesystem, causing bugs.
+  // we'll use the reliable test `err.name == "ErrnoError"` instead
+  constructor(errno) {
+#if ASSERTIONS
+    super(ERRNO_MESSAGES[errno]);
+#endif
+    // TODO(sbc): Use the inline member declaration syntax once we
+    // support it in acorn and closure.
+    this.name = 'ErrnoError';
+    this.errno = errno;
+#if ASSERTIONS
+    for (var key in ERRNO_CODES) {
+      if (ERRNO_CODES[key] === errno) {
+        this.code = key;
+        break;
+      }
+    }
+#endif
+  }
+}
+
 addToLibrary({
   $preloadPlugins: "{{{ makeModuleReceiveExpr('preloadPlugins', '[]') }}}",
 
@@ -50,7 +80,7 @@ addToLibrary({
     '$FS_handledByPreloadPlugin',
 #endif
   ],
-  $FS_createPreloadedFile: (parent, name, url, canRead, canWrite, onload, onerror, dontCreateFile, canOwn, preFinish) => {
+  $FS_createPreloadedFile: (parent, name, url, canRead, canWrite, onload, onerror, dontCreateFile, canOwn = false, preFinish = undefined) => {
     // TODO we should allow people to just pass in a complete filename instead
     // of parent and name being that we just join them anyways
     var fullname = name ? PATH_FS.resolve(PATH.join2(parent, name)) : parent;

src/library_glemu.js

@@ -69,7 +69,6 @@ var LibraryGLEmulation = {
     fogStart: 0,
     fogEnd: 1,
     fogDensity: 1.0,
-    fogColor: null,
     fogMode: 0x800, // GL_EXP
     fogEnabled: false,
 
@@ -2127,7 +2126,7 @@ var LibraryGLEmulation = {
       return renderer;
     },
 
-    createRenderer(renderer) {
+    createRenderer() {
       var useCurrProgram = !!GL.currProgram;
       var hasTextures = false;
       for (var i = 0; i < GLImmediate.MAX_TEXTURES; i++) {
@@ -2995,7 +2994,7 @@ var LibraryGLEmulation = {
       }
     },
 
-    flush(numProvidedIndexes, startIndex = 0, ptr = 0) {
+    flush(numProvidedIndexes = 0, startIndex = 0, ptr = 0) {
 #if ASSERTIONS
       assert(numProvidedIndexes >= 0 || !numProvidedIndexes);
 #endif

src/library_html5.js

@@ -94,8 +94,7 @@ var LibraryHTML5 = {
         return true;
       }
       // Test if the given call was already queued, and if so, don't add it again.
-      for (var i in JSEvents.deferredCalls) {
-        var call = JSEvents.deferredCalls[i];
+      for (var call of JSEvents.deferredCalls) {
         if (call.targetFunction == targetFunction && arraysHaveEqualContent(call.argsList, argsList)) {
           return;
         }
@@ -106,7 +105,10 @@ var LibraryHTML5 = {
         argsList
       });
 
-      JSEvents.deferredCalls.sort((x,y) => x.precedence < y.precedence);
+      // sortcallback(x, y):
+      // A negative value indicates that x should come before y.
+      // A positive value indicates that x should come after y.
+      JSEvents.deferredCalls.sort((x,y) => y.precedence - x.precedence);
     },
 
     // Erases all deferred calls to the given target function from the queue list.
@@ -151,10 +153,9 @@ var LibraryHTML5 = {
     // Removes all event handlers on the given DOM element of the given type.
     // Pass in eventTypeString == undefined/null to remove all event handlers
     // regardless of the type.
-    removeAllHandlersOnTarget: (target, eventTypeString) => {
+    removeAllHandlersOnTarget: (target) => {
       for (var i = 0; i < JSEvents.eventHandlers.length; ++i) {
-        if (JSEvents.eventHandlers[i].target == target &&
-          (!eventTypeString || eventTypeString == JSEvents.eventHandlers[i].eventTypeString)) {
+        if (JSEvents.eventHandlers[i].target) {
            JSEvents._removeHandler(i--);
          }
       }
@@ -1512,9 +1513,6 @@ var LibraryHTML5 = {
         filteringMode: {{{ makeGetValue('fullscreenStrategy', C_STRUCTS.EmscriptenFullscreenStrategy.filteringMode, 'i32') }}},
         canvasResizedCallback: {{{ makeGetValue('fullscreenStrategy', C_STRUCTS.EmscriptenFullscreenStrategy.canvasResizedCallback, 'i32') }}},
         canvasResizedCallbackUserData: {{{ makeGetValue('fullscreenStrategy', C_STRUCTS.EmscriptenFullscreenStrategy.canvasResizedCallbackUserData, 'i32') }}},
-#if PTHREADS
-        canvasResizedCallbackTargetThread: JSEvents.getTargetThreadForEventCallback(),
-#endif
         target,
         softFullscreen: true
     };

src/library_nodefs.js

@@ -64,9 +64,9 @@ addToLibrary({
 #if ASSERTIONS
       assert(ENVIRONMENT_IS_NODE);
 #endif
-      return NODEFS.createNode(null, '/', NODEFS.getMode(mount.opts.root), 0);
+      return NODEFS.createNode(null, '/', NODEFS.getMode(mount.opts.root));
     },
-    createNode(parent, name, mode, dev) {
+    createNode(parent, name, mode) {
       if (!FS.isDir(mode) && !FS.isFile(mode) && !FS.isLink(mode)) {
         throw new FS.ErrnoError({{{ cDefs.EINVAL }}});
       }
@@ -176,7 +176,7 @@ addToLibrary({
         return NODEFS.createNode(parent, name, mode);
       },
       mknod(parent, name, mode, dev) {
-        var node = NODEFS.createNode(parent, name, mode, dev);
+        var node = NODEFS.createNode(parent, name, mode);
         // create the backing node for this in the fs root as well
         var path = NODEFS.realPath(node);
         NODEFS.tryFSOperation(() => {
@@ -277,7 +277,7 @@ addToLibrary({
         return { ptr, allocated: true };
       },
       msync(stream, buffer, offset, length, mmapFlags) {
-        NODEFS.stream_ops.write(stream, buffer, 0, length, offset, false);
+        NODEFS.stream_ops.write(stream, buffer, 0, length, offset);
         // should we check if bytesWritten and length are the same?
         return 0;
       }

src/library_openal.js

@@ -103,7 +103,7 @@ var LibraryOpenAL = {
     // represents the queue of buffers scheduled for physical playback. These two queues are
     // distinct because of the differing semantics of OpenAL and web audio. Some changes
     // to OpenAL parameters, such as pitch, may require the web audio queue to be flushed and rescheduled.
-    scheduleSourceAudio: (src, lookahead) => {
+    scheduleSourceAudio: (src, lookahead = false) => {
       // See comment on scheduleContextAudio above.
       if (Browser.mainLoop.timingMode === {{{ cDefs.EM_TIMING_RAF }}} && document['visibilityState'] != 'visible') {
         return;