Added test for emscripten_html5_remove_event_listener

[ypujante]

I realized while taking another look at the code, that the 2 event handlers added in the "Safari" case do not depend on where the test is running but only on a constant (MIN_SAFARI_VERSION):

#if MIN_SAFARI_VERSION != TARGET_NOT_SUPPORTED
    // As of Safari 13.0.3 on macOS Catalina 10.15.1 still ships with prefixed webkitfullscreenchange. TODO: revisit this check once Safari ships unprefixed version.
    registerFullscreenChangeEventCallback(target, userData, useCapture, callbackfunc, {{{ cDefs.EMSCRIPTEN_EVENT_FULLSCREENCHANGE }}}, "webkitfullscreenchange", targetThread);
#endif

So I added the test that verifies that the fix for bug #25846 (emscripten_html5_remove_event_listener does not clean fullscreenchange entirely) is properly implemented, since the test can pass on other browsers (Chrome, Firefox, etc...).

[ypujante]

It looks like my assumptions were wrong since the test-browser-chrome-wasm64-4gb only adds 1 even listeners and not 2 like the others...

Unless there is a way to prevent this test to run with test-browser-chrome-wasm64-4gb I will delete this PR...

[ypujante]

Seems like the tests are stuck:

While installing emsdk...

##O=#  #                                                                      

gzip: stdin: not in gzip format
tar: Child returned status 1
tar: Error is not recoverable: exiting now

Exited with code exit status 2

[sbc100]

Should shouldn't need this change since we default to including safari support.

[ypujante]

I agree but the test test-browser-chrome-wasm64-4gb failed because of that: there was only 1 listener set instead of 2 which means the code

#if MIN_SAFARI_VERSION != TARGET_NOT_SUPPORTED
    // As of Safari 13.0.3 on macOS Catalina 10.15.1 still ships with prefixed webkitfullscreenchange. TODO: revisit this check once Safari ships unprefixed version.
    registerFullscreenChangeEventCallback(target, userData, useCapture, callbackfunc, {{{ cDefs.EMSCRIPTEN_EVENT_FULLSCREENCHANGE }}}, "webkitfullscreenchange", targetThread);
#endif

somehow didn't work during that test...

[sbc100]

can you trying running that locally with ./test/runner browser64_4gb.test_name can you then check the generated JS file to ensure that the second call to registerFullscreenChangeEventCallback is generated.

Perhaps safari is somehow being disabled in that mode? We should probably fix that if it is.

[ypujante]

I didn't know you could do that.

With -sMIN_SAFARI_VERSION=150000 the test does not even compile:

emcc: error: MIN_SAFARI_VERSION=150000 is not compatible with MEMORY64 (MIN_SAFARI_VERSION=2147483647 or above required) [-Wcompatibility] [-Werror]

And without, this is the generated code which does not include the other event listener:

  function _emscripten_set_fullscreenchange_callback_on_thread(target, userData, useCapture, callbackfunc, targetThread) {
    target = bigintToI53Checked(target);
    userData = bigintToI53Checked(userData);
    callbackfunc = bigintToI53Checked(callbackfunc);
    targetThread = bigintToI53Checked(targetThread);
  
  
      if (!JSEvents.fullscreenEnabled()) return -1;
      target = findEventTarget(target);
      if (!target) return -4;
  
      return registerFullscreenChangeEventCallback(target, userData, useCapture, callbackfunc, 19, "fullscreenchange", targetThread);
    ;
  }

So I guess either way the test will fail...

[sbc100]

Can you run with EMCC_DEBUG=1 and see if/why safari support is being removed.

[ypujante]

It seems that the relevant section is:

utils:DEBUG: successfully executed /usr/local/emsdk/upstream/bin/clang --version
feature_matrix:DEBUG: Enabling MIN_CHROME_VERSION=128 to accommodate MEMORY64
feature_matrix:DEBUG: Enabling MIN_FIREFOX_VERSION=129 to accommodate MEMORY64
feature_matrix:DEBUG: Enabling MIN_SAFARI_VERSION=2147483647 to accommodate MEMORY64
feature_matrix:DEBUG: Enabling MIN_NODE_VERSION=230000 to accommodate MEMORY64

[sbc100]

In that case I think you should make that part of the test conditional on safari support. Something like #if SAFARI_SUPPORT in the code and then f'-DSAFARY_SUPPORT={not self.is_wasm64()}' in the test command line.

[ypujante]

I implemented your suggestion and now all tests are green!