Add closure type annotations to enable better DCE with external code (emscripten-core#16208)

We had a report that some common method names such as `getContext`
and `contains` was being kept alive because closure was not able to
determine the type on which they were be used internally.

Author: sbc100

src/Fetch.js

@@ -32,7 +32,7 @@ var Fetch = {
 #if FETCH_DEBUG
       console.log('fetch: IndexedDB upgrade needed. Clearing database.');
 #endif
-      var db = event.target.result;
+      var db = /** @type {IDBDatabase} */ (event.target.result);
       if (db.objectStoreNames.contains('FILES')) {
         db.deleteObjectStore('FILES');
       }
@@ -191,7 +191,7 @@ function fetchLoadCachedData(db, fetch, onsuccess, onerror) {
   }
 }
 
-function fetchCacheData(db, fetch, data, onsuccess, onerror) {
+function fetchCacheData(/** @type {IDBDatabase} */ db, fetch, data, onsuccess, onerror) {
   if (!db) {
 #if FETCH_DEBUG
     console.error('fetch: IndexedDB not available!');

src/IDBStore.js

@@ -29,7 +29,7 @@
       return callback(e);
     }
     req.onupgradeneeded = function(e) {
-      var db = e.target.result;
+      var db = /** @type {IDBDatabase} */ (e.target.result);
       var transaction = e.target.transaction;
       var fileStore;
       if (db.objectStoreNames.contains(IDBStore.DB_STORE_NAME)) {
@@ -39,7 +39,7 @@
       }
     };
     req.onsuccess = function() {
-      db = req.result;
+      db = /** @type {IDBDatabase} */ (req.result);
       // add to the cache
       IDBStore.dbs[name] = db;
       callback(null, db);

src/library_browser.js

@@ -152,7 +152,7 @@ var LibraryBrowser = {
         var img = new Image();
         img.onload = () => {
           assert(img.complete, 'Image ' + name + ' could not be decoded');
-          var canvas = document.createElement('canvas');
+          var canvas = /** @type {!HTMLCanvasElement} */ (document.createElement('canvas'));
           canvas.width = img.width;
           canvas.height = img.height;
           var ctx = canvas.getContext('2d');
@@ -327,7 +327,7 @@ var LibraryBrowser = {
       return handled;
     },
 
-    createContext: function(canvas, useWebGL, setInModule, webGLContextAttributes) {
+    createContext: function(/** @type {HTMLCanvasElement} */ canvas, useWebGL, setInModule, webGLContextAttributes) {
       if (useWebGL && Module.ctx && canvas == Module.canvas) return Module.ctx; // no need to recreate GL context if it's already been created for this canvas.
 
       var ctx;
@@ -946,7 +946,7 @@ var LibraryBrowser = {
         // Emulate setImmediate. (note: not a complete polyfill, we don't emulate clearImmediate() to keep code size to minimum, since not needed)
         var setImmediates = [];
         var emscriptenMainLoopMessageId = 'setimmediate';
-        var Browser_setImmediate_messageHandler = function(event) {
+        var Browser_setImmediate_messageHandler = function(/** @type {Event} */ event) {
           // When called in current thread or Worker, the main loop ID is structured slightly different to accommodate for --proxy-to-worker runtime listening to Worker events,
           // so check for both cases.
           if (event.data === emscriptenMainLoopMessageId || event.data.target === emscriptenMainLoopMessageId) {
@@ -1402,7 +1402,7 @@ var LibraryBrowser = {
 
     path = PATH_FS.resolve(path);
 
-    var canvas = Module["preloadedImages"][path];
+    var canvas = /** @type {HTMLCanvasElement} */(Module["preloadedImages"][path]);
     if (canvas) {
       var ctx = canvas.getContext("2d");
       var image = ctx.getImageData(0, 0, canvas.width, canvas.height);

src/library_eventloop.js

@@ -44,7 +44,7 @@ LibraryJSEventLoop = {
       'var __setImmediate_id_counter = 0;\n' +
       'var __setImmediate_queue = [];\n' +
       'var __setImmediate_message_id = "_si";\n' +
-      'function __setImmediate_cb(e) {\n' +
+      'function __setImmediate_cb(/** @type {Event} */e) {\n' +
         'if (e.data === __setImmediate_message_id) {\n' +
           'e.stopPropagation();\n' +
           '__setImmediate_queue.shift()();\n' +

src/library_idbfs.js

@@ -52,7 +52,7 @@ mergeInto(LibraryManager.library, {
         return callback("Unable to connect to IndexedDB");
       }
       req.onupgradeneeded = (e) => {
-        var db = e.target.result;
+        var db = /** @type {IDBDatabase} */ (e.target.result);
         var transaction = e.target.transaction;
 
         var fileStore;
@@ -68,7 +68,7 @@ mergeInto(LibraryManager.library, {
         }
       };
       req.onsuccess = () => {
-        db = req.result;
+        db = /** @type {IDBDatabase} */ (req.result);
 
         // add to the cache
         IDBFS.dbs[name] = db;

src/library_sdl.js

@@ -1515,12 +1515,14 @@ var LibrarySDL = {
   SDL_AudioQuit__sig: 'v',
   SDL_AudioQuit: function() {
     for (var i = 0; i < SDL.numChannels; ++i) {
-      if (SDL.channels[i].audio) {
-        SDL.channels[i].audio.pause();
-        SDL.channels[i].audio = undefined;
+      var chan = /** @type {{ audio: (HTMLMediaElement|undefined) }} */ (SDL.channels[i]);
+      if (chan.audio) {
+        chan.audio.pause();
+        chan.audio = undefined;
       }
     }
-    if (SDL.music.audio) SDL.music.audio.pause();
+    var audio = /** @type {HTMLMediaElement} */ (SDL.music.audio);
+    if (audio) audio.pause();
     SDL.music.audio = undefined;
   },
 
@@ -2989,7 +2991,7 @@ var LibrarySDL = {
   Mix_HaltChannel__sig: 'ii',
   Mix_HaltChannel: function(channel) {
     function halt(channel) {
-      var info = SDL.channels[channel];
+      var info = /** @type {{ audio: HTMLMediaElement }} */ (SDL.channels[channel]);
       if (info.audio) {
         info.audio.pause();
         info.audio = null;
@@ -3070,7 +3072,7 @@ var LibrarySDL = {
   Mix_PauseMusic__proxy: 'sync',
   Mix_PauseMusic__sig: 'v',
   Mix_PauseMusic: function() {
-    var audio = SDL.music.audio;
+    var audio = /** @type {HTMLMediaElement} */ (SDL.music.audio);
     if (audio) audio.pause();
   },
 
@@ -3084,7 +3086,7 @@ var LibrarySDL = {
   Mix_HaltMusic__proxy: 'sync',
   Mix_HaltMusic__sig: 'i',
   Mix_HaltMusic: function() {
-    var audio = SDL.music.audio;
+    var audio = /** @type {HTMLMediaElement} */ (SDL.music.audio);
     if (audio) {
       audio.src = audio.src; // rewind <media> element
       audio.currentPosition = 0; // rewind Web Audio graph playback.
@@ -3135,6 +3137,7 @@ var LibrarySDL = {
       }
       return;
     }
+    /** @type {{ audio: HTMLMediaElement }} */
     var info = SDL.channels[channel];
     if (info && info.audio) {
       info.audio.pause();
@@ -3199,7 +3202,7 @@ var LibrarySDL = {
         throw 'bad context';
       }
     } catch (ex) {
-      var canvas = document.createElement('canvas');
+      var canvas = /** @type {HTMLCanvasElement} */(document.createElement('canvas'));
       SDL.ttfContext = canvas.getContext('2d');
     }
 #if ASSERTIONS

src/library_webgl.js

@@ -584,7 +584,7 @@ var LibraryGL = {
     },
 #endif
     // Returns the context handle to the new context.
-    createContext: function(canvas, webGLContextAttributes) {
+    createContext: function(/** @type {HTMLCanvasElement} */ canvas, webGLContextAttributes) {
 #if OFFSCREEN_FRAMEBUFFER
       // In proxied operation mode, rAF()/setTimeout() functions do not delimit frame boundaries, so can't have WebGL implementation
       // try to detect when it's ok to discard contents of the rendered backbuffer.
@@ -637,10 +637,12 @@ var LibraryGL = {
       // TODO: Once the bug is fixed and shipped in Safari, adjust the Safari version field in above check.
       if (!canvas.getContextSafariWebGL2Fixed) {
         canvas.getContextSafariWebGL2Fixed = canvas.getContext;
-        canvas.getContext = function(ver, attrs) {
+        /** @type {function(this:HTMLCanvasElement, string, (Object|null)=): (Object|null)} */
+        function fixedGetContext(ver, attrs) {
           var gl = canvas.getContextSafariWebGL2Fixed(ver, attrs);
           return ((ver == 'webgl') == (gl instanceof WebGLRenderingContext)) ? gl : null;
         }
+        canvas.getContext = fixedGetContext;
       }
 #endif
 

tests/test_other.py

@@ -7982,6 +7982,65 @@ def test_closure_cmdline_utf8_chars(self):
     create_file(externs, '')
     self.run_process([EMCC, test, '--closure=1', '--closure-args', '--externs "' + externs + '"'])
 
+  def test_closure_type_annotations(self):
+    # Verify that certain type annotations exist to allow closure to avoid
+    # ambiguity and maximize optimization opportunities in user code.
+    #
+    # Currently we check for a fixed set of known attribute names which
+    # have been reported as unannoted in the past:
+    attributes = ['put', 'getContext', 'contains', 'stopPropagation', 'pause']
+    methods = ''
+    for attribute in attributes:
+      methods += f'''
+        this.{attribute} = function() {{
+          console.error("my {attribute}");
+        }};
+      '''
+    create_file('pre.js', '''
+      /** @constructor */
+      function Foo() {
+        this.bar = function() {
+          console.error("my bar");
+        };
+        this.baz = function() {
+          console.error("my baz");
+        };
+        %s
+        return this;
+      }
+
+      function getObj() {
+        return new Foo();
+      }
+
+      var testobj = getObj();
+      testobj.bar();
+
+      /** Also keep alive certain library functions */
+      Module['keepalive'] = [_emscripten_start_fetch, _emscripten_pause_main_loop, _SDL_AudioQuit];
+    ''' % methods)
+
+    self.build(test_file('hello_world.c'), emcc_args=[
+      '--closure=1',
+      '-sINCLUDE_FULL_LIBRARY',
+      '-sFETCH',
+      '-sFETCH_SUPPORT_INDEXEDDB',
+      '-sCLOSURE_WARNINGS=error',
+      '--pre-js=pre.js'
+    ])
+    code = read_file('hello_world.js')
+    # `bar` method is used so should not be DCE'd
+    self.assertContained('my bar', code)
+    # `baz` method is not used
+    self.assertNotContained('my baz', code)
+
+    for attribute in attributes:
+      # None of the attributes in our list should be used either and should therefore
+      # be DCE'd unless there is some usage of that name within emscripten that is
+      # not annotated.
+      if 'my ' + attribute in code:
+        self.assertFalse('Attribute `%s` could not be DCEd' % attribute)
+
   @with_env_modify({'EMPROFILE': '1'})
   def test_toolchain_profiler(self):
     # Verify some basic functionality of EMPROFILE

tools/file_packager.py

@@ -755,20 +755,20 @@ def generate_js(data_target, data_files, metadata):
             return errback(e);
           }
           openRequest.onupgradeneeded = function(event) {
-            var db = event.target.result;
+            var db = /** @type {IDBDatabase} */ (event.target.result);
 
-            if(db.objectStoreNames.contains(PACKAGE_STORE_NAME)) {
+            if (db.objectStoreNames.contains(PACKAGE_STORE_NAME)) {
               db.deleteObjectStore(PACKAGE_STORE_NAME);
             }
             var packages = db.createObjectStore(PACKAGE_STORE_NAME);
 
-            if(db.objectStoreNames.contains(METADATA_STORE_NAME)) {
+            if (db.objectStoreNames.contains(METADATA_STORE_NAME)) {
               db.deleteObjectStore(METADATA_STORE_NAME);
             }
             var metadata = db.createObjectStore(METADATA_STORE_NAME);
           };
           openRequest.onsuccess = function(event) {
-            var db = event.target.result;
+            var db = /** @type {IDBDatabase} */ (event.target.result);
             callback(db);
           };
           openRequest.onerror = function(error) {