fix: fix event type atom id changed when free.

openkraken · Dec 30, 2021 · ce92647 · ce92647
1 parent 5732bc4
commit ce92647
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 5 deletions.
diff --git a/bridge/bindings/qjs/dom/event_listener_map.cc b/bridge/bindings/qjs/dom/event_listener_map.cc
@@ -89,6 +89,7 @@ void EventListenerMap::trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func)
 EventListenerMap::~EventListenerMap() {
   for (const auto& entry : m_entries) {
     for (const auto& vector : entry.second) {
+      JS_FreeAtomRT(m_runtime, entry.first);
       JS_FreeValueRT(m_runtime, vector);
     }
   }

diff --git a/bridge/bindings/qjs/dom/event_target.cc b/bridge/bindings/qjs/dom/event_target.cc
@@ -66,6 +66,7 @@ JSValue EventTarget::addEventListener(JSContext* ctx, JSValue this_val, int argc
     return JS_UNDEFINED;
   }
 
+  // EventType atom will be freed when eventTarget finalized.
   JSAtom eventType = JS_ValueToAtom(ctx, eventTypeValue);
 
   // Dart needs to be notified for the first registration event.
@@ -79,7 +80,6 @@ JSValue EventTarget::addEventListener(JSContext* ctx, JSValue this_val, int argc
   }
 
   eventTargetInstance->m_eventListenerMap.add(eventType, JS_DupValue(ctx, callback));
-  JS_FreeAtom(ctx, eventType);
 
   return JS_UNDEFINED;
 }
@@ -361,7 +361,7 @@ int EventTargetInstance::setProperty(JSContext* ctx, JSValue obj, JSAtom atom, J
       JS_FreeValue(eventTarget->m_ctx, eventTarget->m_properties.getProperty(atom));
     }
 
-    eventTarget->m_properties.setProperty(atom, JS_DupValue(ctx, value));
+    eventTarget->m_properties.setProperty(JS_DupAtom(ctx, atom), JS_DupValue(ctx, value));
     if (isJavaScriptExtensionElementInstance(eventTarget->context(), eventTarget->jsObject) && !p->is_wide_char && p->u.str8[0] != '_') {
       std::unique_ptr<NativeString> args_01 = atomToNativeString(ctx, atom);
       std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, value);
@@ -407,7 +407,6 @@ void EventTargetInstance::setAttributesEventHandler(JSString* p, JSValue value)
   // When evaluate scripts like 'element.onclick = null', we needs to remove the event handlers callbacks
   if (JS_IsNull(value)) {
     m_eventHandlerMap.erase(atom);
-    JS_FreeAtom(m_ctx, atom);
     return;
   }
 
@@ -420,8 +419,6 @@ void EventTargetInstance::setAttributesEventHandler(JSString* p, JSValue value)
     int32_t type = JS_IsFunction(m_ctx, value) ? UICommand::addEvent : UICommand::removeEvent;
     foundation::UICommandBuffer::instance(contextId)->addCommand(m_eventTargetId, type, *args_01, nullptr);
   }
-
-  JS_FreeAtom(m_ctx, atom);
 }
 
 JSValue EventTargetInstance::getAttributesEventHandler(JSString* p) {

diff --git a/bridge/bindings/qjs/dom/event_target_test.cc b/bridge/bindings/qjs/dom/event_target_test.cc
@@ -196,3 +196,24 @@ TEST(EventTarget, globalBindListener) {
   bridge->evaluateScript(code.c_str(), code.size(), "internal://", 0);
   EXPECT_EQ(logCalled, true);
 }
+
+TEST(EventTarget, shouldKeepAtom) {
+  auto bridge = TEST_init();
+  bool static logCalled = false;
+  kraken::KrakenPage::consoleMessageHandler = [](void* ctx, const std::string& message, int logLevel) {
+    logCalled = true;
+    EXPECT_STREQ(message.c_str(), "2");
+  };
+  std::string code = "addEventListener('click', () => {console.log(1)});";
+  bridge->evaluateScript(code.c_str(), code.size(), "internal://", 0);
+  JS_RunGC(bridge->getContext()->runtime());
+
+  std::string code2 = "addEventListener('appear', () => {console.log(2)});";
+  bridge->evaluateScript(code2.c_str(), code2.size(), "internal://", 0);
+
+  JS_RunGC(bridge->getContext()->runtime());
+
+  std::string code3 = "(function() { var eeee = new Event('appear'); dispatchEvent(eeee); } )();";
+  bridge->evaluateScript(code3.c_str(), code3.size(), "internal://", 0);
+  EXPECT_EQ(logCalled, true);
+}
diff --git a/bridge/bindings/qjs/heap_hashmap.h b/bridge/bindings/qjs/heap_hashmap.h
@@ -38,6 +38,7 @@ HeapHashMap<K>::HeapHashMap(JSContext* ctx) : m_runtime(JS_GetRuntime(ctx)), m_c
 template <typename K>
 HeapHashMap<K>::~HeapHashMap() {
   for (auto& entry : m_entries) {
+    JS_FreeAtomRT(m_runtime, entry.first);
     JS_FreeValueRT(m_runtime, entry.second);
   }
 }
@@ -53,6 +54,11 @@ JSValue HeapHashMap<K>::getProperty(K key) {
 
 template <typename K>
 void HeapHashMap<K>::setProperty(K key, JSValue value) {
+  if (m_entries.count(key) > 0) {
+    JS_FreeAtom(m_ctx, key);
+    JS_FreeValue(m_ctx, value);
+  }
+
   m_entries[key] = value;
 }