Merge remote-tracking branch 'origin/main' into fix/ui_command_buffer…

…_instance_leak

bridge/CMakeLists.txt

@@ -171,7 +171,11 @@ if ($ENV{KRAKEN_JS_ENGINE} MATCHES "quickjs")
     ${CMAKE_CURRENT_SOURCE_DIR}/third_party/quickjs/quickjs-atom.h
     ${CMAKE_CURRENT_SOURCE_DIR}/third_party/quickjs/quickjs-opcode.h
     )
-  add_library(quickjs SHARED ${QUICK_JS_SOURCE})
+  if(${STATIC_QUICKJS})
+    add_library(quickjs STATIC ${QUICK_JS_SOURCE})
+  else()
+    add_library(quickjs SHARED ${QUICK_JS_SOURCE})
+  endif()
 
   list(APPEND BRIDGE_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/third_party)
   list(APPEND BRIDGE_LINK_LIBS quickjs)
@@ -182,6 +186,7 @@ if ($ENV{KRAKEN_JS_ENGINE} MATCHES "quickjs")
     bindings/qjs/garbage_collected.h
     bindings/qjs/executing_context.cc
     bindings/qjs/executing_context.h
+    bindings/qjs/heap_hashmap.h
     bindings/qjs/native_value.cc
     bindings/qjs/native_value.h
     bindings/qjs/host_object.h
@@ -366,11 +371,14 @@ if (${CMAKE_SYSTEM_NAME} MATCHES "iOS")
     PUBLIC_HEADER ${KRAKEN_PUBLIC_HEADERS}
     )
 
-  set_target_properties(quickjs PROPERTIES
-    OUTPUT_NAME quickjs
-    FRAMEWORK TRUE
-    FRAMEWORK_VERSION C
-    MACOSX_FRAMEWORK_IDENTIFIER com.openkraken.quickjs
-    PUBLIC_HEADER ${QUICKJS_PUBLIC_HEADERS}
-    )
+  # If quickjs is static, there will be no quickjs.framework any more.
+  if(NOT ${STATIC_QUICKJS})
+    set_target_properties(quickjs PROPERTIES
+      OUTPUT_NAME quickjs
+      FRAMEWORK TRUE
+      FRAMEWORK_VERSION C
+      MACOSX_FRAMEWORK_IDENTIFIER com.openkraken.quickjs
+      PUBLIC_HEADER ${QUICKJS_PUBLIC_HEADERS}
+      )
+  endif()
 endif ()

bridge/bindings/qjs/bom/timer.cc

@@ -39,6 +39,10 @@ void DOMTimer::trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func) const {
   JS_MarkValue(rt, m_callback, mark_func);
 }
 
+void DOMTimer::dispose() const {
+  JS_FreeValueRT(m_runtime, m_callback);
+}
+
 int32_t DOMTimer::timerId() {
   return m_timerId;
 }

bridge/bindings/qjs/bom/timer.h

@@ -26,6 +26,7 @@ class DOMTimer : public GarbageCollected<DOMTimer> {
   [[nodiscard]] FORCE_INLINE const char* getHumanReadableName() const override { return "DOMTimer"; }
 
   void trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func) const override;
+  void dispose() const override;
 
  private:
   int32_t m_timerId{-1};

bridge/bindings/qjs/bom/window.cc

@@ -211,7 +211,7 @@ IMPL_PROPERTY_SETTER(Window, onerror)(JSContext* ctx, JSValue this_val, int argc
     JS_FreeValue(ctx, window->onerror);
   }
 
-  window->onerror = onerrorHandler;
+  window->onerror = JS_DupValue(ctx, onerrorHandler);
   JS_FreeValue(ctx, eventString);
   return JS_NULL;
 }
@@ -229,6 +229,8 @@ WindowInstance::WindowInstance(Window* window) : EventTargetInstance(window, Win
 
 void WindowInstance::trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func) {
   EventTargetInstance::trace(rt, val, mark_func);
+
+  JS_MarkValue(rt, onerror, mark_func);
 }
 
 DocumentInstance* WindowInstance::document() {

bridge/bindings/qjs/dom/document.cc

@@ -534,7 +534,6 @@ void DocumentInstance::removeElementById(JSAtom id, ElementInstance* element) {
   if (m_elementMapById.count(id) > 0) {
     auto& list = m_elementMapById[id];
     JS_FreeValue(m_ctx, element->jsObject);
-    list_del(&element->documentLink.link);
     list.erase(std::find(list.begin(), list.end(), element));
   }
 }
@@ -548,7 +547,6 @@ void DocumentInstance::addElementById(JSAtom id, ElementInstance* element) {
 
   if (it == list.end()) {
     JS_DupValue(m_ctx, element->jsObject);
-    list_add_tail(&element->documentLink.link, &m_context->document_job_list);
     m_elementMapById[id].emplace_back(element);
   }
 }
@@ -578,7 +576,14 @@ void DocumentInstance::cancelAnimationFrame(uint32_t callbackId) {
 
 void DocumentInstance::trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func) {
   NodeInstance::trace(rt, val, mark_func);
+  // Trace scriptAnimationController
   JS_MarkValue(rt, m_scriptAnimationController->toQuickJS(), mark_func);
+  // Trace elementByIdMaps
+  for (auto& entry : m_elementMapById) {
+    for (auto& value : entry.second) {
+      JS_MarkValue(rt, value->jsObject, mark_func);
+    }
+  }
 }
 
 }  // namespace kraken::binding::qjs

bridge/bindings/qjs/dom/element.cc

@@ -55,35 +55,35 @@ JSClassID Element::classId() {
   return kElementClassId;
 }
 
-JSAtom ElementAttributes::getAttribute(const std::string& name) {
+JSClassID ElementAttributes::classId{0};
+JSValue ElementAttributes::getAttribute(const std::string& name) {
   bool numberIndex = isNumberIndex(name);
 
   if (numberIndex) {
-    return JS_ATOM_NULL;
+    return JS_NULL;
   }
 
-  return m_attributes[name];
+  return JS_DupValue(m_ctx, m_attributes[name]);
 }
 
-ElementAttributes::~ElementAttributes() {
-  for (auto& attr : m_attributes) {
-    JS_FreeAtom(m_ctx, attr.second);
-  }
-}
-
-JSValue ElementAttributes::setAttribute(const std::string& name, JSAtom atom) {
+JSValue ElementAttributes::setAttribute(const std::string& name, JSValue value) {
   bool numberIndex = isNumberIndex(name);
 
   if (numberIndex) {
     return JS_ThrowTypeError(m_ctx, "Failed to execute 'setAttribute' on 'Element': '%s' is not a valid attribute name.", name.c_str());
   }
 
   if (name == "class") {
-    std::string classNameString = jsAtomToStdString(m_ctx, atom);
+    std::string classNameString = jsValueToStdString(m_ctx, value);
     m_className->set(classNameString);
   }
 
-  m_attributes[name] = JS_DupAtom(m_ctx, atom);
+  // If attribute exists, should free the previous value.
+  if (m_attributes.count(name) > 0) {
+    JS_FreeValue(m_ctx, m_attributes[name]);
+  }
+
+  m_attributes[name] = JS_DupValue(m_ctx, value);
 
   return JS_NULL;
 }
@@ -99,14 +99,14 @@ bool ElementAttributes::hasAttribute(std::string& name) {
 }
 
 void ElementAttributes::removeAttribute(std::string& name) {
-  JSAtom value = m_attributes[name];
-  JS_FreeAtom(m_ctx, value);
+  JSValue value = m_attributes[name];
+  JS_FreeValue(m_ctx, value);
   m_attributes.erase(name);
 }
 
 void ElementAttributes::copyWith(ElementAttributes* attributes) {
   for (auto& attr : attributes->m_attributes) {
-    m_attributes[attr.first] = JS_DupAtom(m_ctx, attr.second);
+    m_attributes[attr.first] = JS_DupValue(m_ctx, attr.second);
   }
 }
 
@@ -119,14 +119,25 @@ std::string ElementAttributes::toString() {
 
   for (auto& attr : m_attributes) {
     s += attr.first + "=";
-    const char* pstr = JS_AtomToCString(m_ctx, attr.second);
+    const char* pstr = JS_ToCString(m_ctx, attr.second);
     s += "\"" + std::string(pstr) + "\"";
     JS_FreeCString(m_ctx, pstr);
   }
 
   return s;
 }
 
+void ElementAttributes::dispose() const {
+  for (auto& attr : m_attributes) {
+    JS_FreeValueRT(m_runtime, attr.second);
+  }
+}
+void ElementAttributes::trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func) const {
+  for (auto& attr : m_attributes) {
+    JS_MarkValue(rt, attr.second, mark_func);
+  }
+}
+
 JSValue Element::instanceConstructor(JSContext* ctx, JSValue func_obj, JSValue this_val, int argc, JSValue* argv) {
   if (argc == 0)
     return JS_ThrowTypeError(ctx, "Illegal constructor");
@@ -183,9 +194,7 @@ JSValue Element::setAttribute(JSContext* ctx, JSValue this_val, int argc, JSValu
   }
 
   JSValue nameValue = argv[0];
-  JSValue attributeValue = argv[1];
-  JSValue attributeString = JS_ToString(ctx, attributeValue);
-  JSAtom attributeAtom = JS_ValueToAtom(ctx, attributeString);
+  JSValue attributeValue = JS_ToString(ctx, argv[1]);
 
   if (!JS_IsString(nameValue)) {
     return JS_ThrowTypeError(ctx, "Failed to execute 'setAttribute' on 'Element': name attribute is not valid.");
@@ -198,26 +207,25 @@ JSValue Element::setAttribute(JSContext* ctx, JSValue this_val, int argc, JSValu
   auto* attributes = element->m_attributes;
 
   if (attributes->hasAttribute(name)) {
-    JSAtom oldAtom = attributes->getAttribute(name);
-    JSValue exception = attributes->setAttribute(name, attributeAtom);
+    JSValue oldAttribute = attributes->getAttribute(name);
+    JSValue exception = attributes->setAttribute(name, attributeValue);
     if (JS_IsException(exception))
       return exception;
-    element->_didModifyAttribute(name, oldAtom, attributeAtom);
-    JS_FreeAtom(ctx, oldAtom);
+    element->_didModifyAttribute(name, oldAttribute, attributeValue);
+    JS_FreeValue(ctx, oldAttribute);
   } else {
-    JSValue exception = attributes->setAttribute(name, attributeAtom);
+    JSValue exception = attributes->setAttribute(name, attributeValue);
     if (JS_IsException(exception))
       return exception;
-    element->_didModifyAttribute(name, JS_ATOM_NULL, attributeAtom);
+    element->_didModifyAttribute(name, JS_NULL, attributeValue);
   }
 
   std::unique_ptr<NativeString> args_01 = stringToNativeString(name);
-  std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, attributeString);
+  std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, attributeValue);
 
   element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
 
-  JS_FreeValue(ctx, attributeString);
-  JS_FreeAtom(ctx, attributeAtom);
+  JS_FreeValue(ctx, attributeValue);
 
   return JS_NULL;
 }
@@ -239,7 +247,7 @@ JSValue Element::getAttribute(JSContext* ctx, JSValue this_val, int argc, JSValu
   auto* attributes = element->m_attributes;
 
   if (attributes->hasAttribute(name)) {
-    return JS_AtomToValue(ctx, attributes->getAttribute(name));
+    return attributes->getAttribute(name);
   }
 
   return JS_NULL;
@@ -261,9 +269,10 @@ JSValue Element::removeAttribute(JSContext* ctx, JSValue this_val, int argc, JSV
   auto* attributes = element->m_attributes;
 
   if (attributes->hasAttribute(name)) {
-    JSAtom id = attributes->getAttribute(name);
+    JSValue targetValue = attributes->getAttribute(name);
     element->m_attributes->removeAttribute(name);
-    element->_didModifyAttribute(name, id, JS_ATOM_NULL);
+    element->_didModifyAttribute(name, targetValue, JS_NULL);
+    JS_FreeValue(ctx, targetValue);
 
     std::unique_ptr<NativeString> args_01 = stringToNativeString(name);
     element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::removeProperty, *args_01, nullptr);
@@ -390,17 +399,14 @@ IMPL_PROPERTY_GETTER(Element, tagName)(JSContext* ctx, JSValue this_val, int arg
 
 IMPL_PROPERTY_GETTER(Element, className)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
   auto* element = static_cast<ElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
-  JSAtom valueAtom = element->m_attributes->getAttribute("class");
-  return JS_AtomToString(ctx, valueAtom);
+  return element->m_attributes->getAttribute("class");
 }
 IMPL_PROPERTY_SETTER(Element, className)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
   auto* element = static_cast<ElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
-  JSAtom atom = JS_ValueToAtom(ctx, argv[0]);
-  element->m_attributes->setAttribute("class", atom);
+  element->m_attributes->setAttribute("class", argv[0]);
   std::unique_ptr<NativeString> args_01 = stringToNativeString("class");
   std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, argv[0]);
   element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
-  JS_FreeAtom(ctx, atom);
   return JS_NULL;
 }
 
@@ -558,6 +564,11 @@ IMPL_PROPERTY_GETTER(Element, children)(JSContext* ctx, JSValue this_val, int ar
   return array;
 }
 
+IMPL_PROPERTY_GETTER(Element, attributes)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
+  auto* element = static_cast<ElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
+  return JS_DupValue(ctx, element->m_attributes->toQuickJS());
+}
+
 IMPL_PROPERTY_GETTER(Element, innerHTML)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
   auto* element = static_cast<ElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
   return JS_NewString(ctx, element->innerHTML().c_str());
@@ -759,10 +770,13 @@ void ElementInstance::_notifyNodeRemoved(NodeInstance* insertionNode) {
 }
 
 void ElementInstance::_notifyChildRemoved() {
-  std::string id = "id";
-  if (m_attributes->hasAttribute(id)) {
-    JSAtom v = m_attributes->getAttribute(id);
-    document()->removeElementById(v, this);
+  std::string prop = "id";
+  if (m_attributes->hasAttribute(prop)) {
+    JSValue idValue = m_attributes->getAttribute(prop);
+    JSAtom id = JS_ValueToAtom(m_ctx, idValue);
+    document()->removeElementById(id, this);
+    JS_FreeValue(m_ctx, idValue);
+    JS_FreeAtom(m_ctx, id);
   }
 }
 
@@ -781,42 +795,55 @@ void ElementInstance::_notifyNodeInsert(NodeInstance* insertNode) {
 }
 
 void ElementInstance::_notifyChildInsert() {
-  std::string idKey = "id";
-  if (m_attributes->hasAttribute(idKey)) {
-    JSAtom v = m_attributes->getAttribute(idKey);
-    document()->addElementById(v, this);
+  std::string prop = "id";
+  if (m_attributes->hasAttribute(prop)) {
+    JSValue idValue = m_attributes->getAttribute(prop);
+    JSAtom id = JS_ValueToAtom(m_ctx, idValue);
+    document()->addElementById(id, this);
+    JS_FreeValue(m_ctx, idValue);
+    JS_FreeAtom(m_ctx, id);
   }
 }
 
-void ElementInstance::_didModifyAttribute(std::string& name, JSAtom oldId, JSAtom newId) {
+void ElementInstance::_didModifyAttribute(std::string& name, JSValue oldId, JSValue newId) {
   if (name == "id") {
     _beforeUpdateId(oldId, newId);
   }
 }
 
-void ElementInstance::_beforeUpdateId(JSAtom oldId, JSAtom newId) {
+void ElementInstance::_beforeUpdateId(JSValue oldIdValue, JSValue newIdValue) {
+  JSAtom oldId = JS_ValueToAtom(m_ctx, oldIdValue);
+  JSAtom newId = JS_ValueToAtom(m_ctx, newIdValue);
+
   if (oldId == newId) {
     return;
   }
 
-  if (oldId != JS_ATOM_NULL) {
+  if (!JS_IsNull(oldIdValue)) {
     document()->removeElementById(oldId, this);
   }
 
-  if (newId != JS_ATOM_NULL) {
+  if (!JS_IsNull(newIdValue)) {
     document()->addElementById(newId, this);
   }
+
+  JS_FreeAtom(m_ctx, oldId);
+  JS_FreeAtom(m_ctx, newId);
+}
+
+void ElementInstance::trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func) {
+  JS_MarkValue(rt, m_attributes->toQuickJS(), mark_func);
+  NodeInstance::trace(rt, val, mark_func);
 }
 
 ElementInstance::ElementInstance(Element* element, std::string tagName, bool shouldAddUICommand)
     : m_tagName(tagName), NodeInstance(element, NodeType::ELEMENT_NODE, Element::classId(), exoticMethods, "Element") {
-  m_attributes = new ElementAttributes(m_context);
+  m_attributes = makeGarbageCollected<ElementAttributes>()->initialize(m_ctx, &ElementAttributes::classId);
   JSValue arguments[] = {jsObject};
   JSValue style = JS_CallConstructor(m_ctx, CSSStyleDeclaration::instance(m_context)->jsObject, 1, arguments);
   m_style = static_cast<StyleDeclarationInstance*>(JS_GetOpaque(style, CSSStyleDeclaration::kCSSStyleDeclarationClassId));
 
   JS_DefinePropertyValueStr(m_ctx, jsObject, "style", m_style->jsObject, JS_PROP_C_W_E);
-  JS_DefinePropertyValueStr(m_ctx, jsObject, "attributes", m_attributes->jsObject, JS_PROP_C_W_E);
 
   if (shouldAddUICommand) {
     std::unique_ptr<NativeString> args_01 = stringToNativeString(tagName);
@@ -830,10 +857,6 @@ StyleDeclarationInstance* ElementInstance::style() {
   return m_style;
 }
 
-ElementAttributes* ElementInstance::attributes() {
-  return m_attributes;
-}
-
 IMPL_PROPERTY_GETTER(BoundingClientRect, x)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
   auto* boundingClientRect = static_cast<BoundingClientRect*>(JS_GetOpaque(this_val, ExecutionContext::kHostObjectClassId));
   return JS_NewFloat64(ctx, boundingClientRect->m_nativeBoundingClientRect->x);