wasm: make EventCallback use addEventListener()

EventCallback would previously set the on<Event> property
on the event target, which is a singleton property where
there can be only one event handler.

This was OK if the event target was owned by Qt, for
example the canvas element, where we could guarantee
that there was only one event handler.

However this approach fell through when registering
event handlers for global event targets, such as for
window.onLanguageChange, where setting a singleton event
handler may conflict with other users.

Fix this by using the addEventListener() API instead,
which has a variant which takes an event listener object,
which gives us the ability to provide C++ context for
the listener. 

The C++ context in this case is a std::function that
contains the event handler callback. Attempts to pass
this type to JavaScript was met with some resistance
from Emscripten, so as a late night hack pass it as
a uintptr_t for now.

Change-Id: I1a547b49af467882ae4f57f8d909ffdff0be6b51

Reviewed-by: Piotr Wierciński <piotr.wiercinski@qt.io>
Reviewed-by: Lorn Potter <lorn.potter@gmail.com>

2 files changed, 30 insertions, 33 deletions

diff --git a/src/corelib/platform/wasm/qstdweb.cpp b/src/corelib/platform/wasm/qstdweb.cpp
index dacddd2f95..75e76a6806 100644
--- a/src/corelib/platform/wasm/qstdweb.cpp
+++ b/src/corelib/platform/wasm/qstdweb.cpp
@@ -448,7 +448,7 @@ ArrayBuffer Blob::arrayBuffer_sync() const
 {
     QEventLoop loop;
     emscripten::val buffer;
-    qstdweb::Promise::make(m_blob, "arrayBuffer", {
+    qstdweb::Promise::make(m_blob, QStringLiteral("arrayBuffer"), {
         .thenFunc = [&loop, &buffer](emscripten::val arrayBuffer) {
             buffer = arrayBuffer;
             loop.quit();
@@ -715,47 +715,45 @@ emscripten::val Uint8Array::constructor_()
     return emscripten::val::global("Uint8Array");
 }
 
+class EventListener {
+public:
+    EventListener(uintptr_t handler)
+        :m_handler(handler)
+    {
+
+    }
+
+    // Special function - addEventListender() allows adding an object with a
+    // handleEvent() function which eceives the event.
+    void handleEvent(emscripten::val event) {
+        auto handlerPtr = reinterpret_cast<std::function<void(emscripten::val)> *>(m_handler);
+        (*handlerPtr)(event);
+    }
+
+    uintptr_t m_handler;
+};
+
 // Registers a callback function for a named event on the given element. The event
 // name must be the name as returned by the Event.type property: e.g. "load", "error".
 EventCallback::~EventCallback()
 {
-    // Clean up if this instance's callback is still installed on the element
-    if (m_element[contextPropertyName(m_eventName).c_str()].as<intptr_t>() == intptr_t(this)) {
-        m_element.set(contextPropertyName(m_eventName).c_str(), emscripten::val::undefined());
-        m_element.set((std::string("on") + m_eventName).c_str(), emscripten::val::undefined());
-    }
+    m_element.call<void>("removeEventListener", m_eventName, m_eventListener);
 }
 
-EventCallback::EventCallback(emscripten::val element, const std::string &name, const std::function<void(emscripten::val)> &fn)
+EventCallback::EventCallback(emscripten::val element, const std::string &name, const std::function<void(emscripten::val)> &handler)
     :m_element(element)
     ,m_eventName(name)
-    ,m_fn(fn)
-{
-    Q_ASSERT_X(m_element[contextPropertyName(m_eventName)].isUndefined(), Q_FUNC_INFO,
-               "Only one event callback of type currently supported with EventCallback");
-    m_element.set(contextPropertyName(m_eventName).c_str(), emscripten::val(intptr_t(this)));
-    m_element.set((std::string("on") + m_eventName).c_str(), emscripten::val::module_property("qtStdWebEventCallbackActivate"));
-}
-
-void EventCallback::activate(emscripten::val event)
-{
-    emscripten::val target = event["currentTarget"];
-    std::string eventName = event["type"].as<std::string>();
-    emscripten::val property = target[contextPropertyName(eventName)];
-    // This might happen when the event bubbles
-    if (property.isUndefined())
-        return;
-    EventCallback *that = reinterpret_cast<EventCallback *>(property.as<intptr_t>());
-    that->m_fn(event);
-}
-
-std::string EventCallback::contextPropertyName(const std::string &eventName)
+    ,m_handler(std::make_unique<std::function<void(emscripten::val)>>(handler))
 {
-    return std::string("data-qtEventCallbackContext") + eventName;
+    uintptr_t handlerUint = reinterpret_cast<uintptr_t>(m_handler.get()); // FIXME: pass pointer directly instead
+    m_eventListener = emscripten::val::module_property("QtEventListener").new_(handlerUint);
+    m_element.call<void>("addEventListener", m_eventName, m_eventListener);
 }
 
 EMSCRIPTEN_BINDINGS(qtStdwebCalback) {
-    emscripten::function("qtStdWebEventCallbackActivate", &EventCallback::activate);
+    emscripten::class_<EventListener>("QtEventListener")
+        .constructor<uintptr_t>()
+        .function("handleEvent", &EventListener::handleEvent);
 }
 
 namespace Promise {
diff --git a/src/corelib/platform/wasm/qstdweb_p.h b/src/corelib/platform/wasm/qstdweb_p.h
index 707d96704b..566e02e8a1 100644
--- a/src/corelib/platform/wasm/qstdweb_p.h
+++ b/src/corelib/platform/wasm/qstdweb_p.h
@@ -199,13 +199,12 @@ namespace qstdweb {
         EventCallback& operator=(EventCallback const&) = delete;
         EventCallback(emscripten::val element, const std::string &name,
                       const std::function<void(emscripten::val)> &fn);
-        static void activate(emscripten::val event);
 
     private:
-        static std::string contextPropertyName(const std::string &eventName);
         emscripten::val m_element = emscripten::val::undefined();
         std::string m_eventName;
-        std::function<void(emscripten::val)> m_fn;
+        std::unique_ptr<std::function<void(emscripten::val)>> m_handler;
+        emscripten::val m_eventListener = emscripten::val::undefined();
     };
 
     struct PromiseCallbacks