Merge pull request #1014 from openkraken/fix/ui_command_buffer_instan…

…ce_leak

fix: fix ui command buffer instance leak.

bridge/CMakeLists.txt

@@ -96,6 +96,7 @@ list(APPEND BRIDGE_SOURCE
   foundation/task_queue.cc
   foundation/task_queue.h
   foundation/ui_command_buffer.cc
+  foundation/ui_command_buffer.h
   foundation/ui_command_callback_queue.cc
   foundation/closure.h
   dart_methods.cc

bridge/bindings/qjs/dom/comment_node.cc

@@ -44,7 +44,7 @@ IMPL_PROPERTY_GETTER(Comment, length)(JSContext* ctx, JSValue this_val, int argc
 }
 
 CommentInstance::CommentInstance(Comment* comment) : NodeInstance(comment, NodeType::COMMENT_NODE, Comment::classId(), "Comment") {
-  ::foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(m_eventTargetId, UICommand::createComment, nativeEventTarget);
+  m_context->uiCommandBuffer()->addCommand(m_eventTargetId, UICommand::createComment, nativeEventTarget);
 }
 
 }  // namespace kraken::binding::qjs

bridge/bindings/qjs/dom/document_fragment.cc

@@ -33,6 +33,6 @@ JSValue DocumentFragment::instanceConstructor(JSContext* ctx, JSValue func_obj,
 
 DocumentFragmentInstance::DocumentFragmentInstance(DocumentFragment* fragment) : NodeInstance(fragment, NodeType::DOCUMENT_FRAGMENT_NODE, DocumentFragment::classId(), "DocumentFragment") {
   setNodeFlag(DocumentFragmentInstance::NodeFlag::IsDocumentFragment);
-  foundation::UICommandBuffer::instance(m_contextId)->addCommand(m_eventTargetId, UICommand::createDocumentFragment, nativeEventTarget);
+  m_context->uiCommandBuffer()->addCommand(m_eventTargetId, UICommand::createDocumentFragment, nativeEventTarget);
 }
 }  // namespace kraken::binding::qjs

bridge/bindings/qjs/dom/element.cc

@@ -223,7 +223,7 @@ JSValue Element::setAttribute(JSContext* ctx, JSValue this_val, int argc, JSValu
   std::unique_ptr<NativeString> args_01 = stringToNativeString(name);
   std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, attributeValue);
 
-  ::foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
+  element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
 
   JS_FreeValue(ctx, attributeValue);
 
@@ -275,7 +275,7 @@ JSValue Element::removeAttribute(JSContext* ctx, JSValue this_val, int argc, JSV
     JS_FreeValue(ctx, targetValue);
 
     std::unique_ptr<NativeString> args_01 = stringToNativeString(name);
-    ::foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::removeProperty, *args_01, nullptr);
+    element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::removeProperty, *args_01, nullptr);
   }
 
   return JS_NULL;
@@ -406,7 +406,7 @@ IMPL_PROPERTY_SETTER(Element, className)(JSContext* ctx, JSValue this_val, int a
   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]);
-  ::foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
+  element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
   return JS_NULL;
 }
 
@@ -847,7 +847,7 @@ ElementInstance::ElementInstance(Element* element, std::string tagName, bool sho
 
   if (shouldAddUICommand) {
     std::unique_ptr<NativeString> args_01 = stringToNativeString(tagName);
-    ::foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(m_eventTargetId, UICommand::createElement, *args_01, nativeEventTarget);
+    element->m_context->uiCommandBuffer()->addCommand(m_eventTargetId, UICommand::createElement, *args_01, nativeEventTarget);
   }
 }
 

bridge/bindings/qjs/dom/elements/image_element.cc

@@ -33,7 +33,7 @@ IMPL_PROPERTY_SETTER(ImageElement, width)(JSContext* ctx, JSValue this_val, int
   std::string key = "width";
   std::unique_ptr<NativeString> args_01 = stringToNativeString(key);
   std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, argv[0]);
-  foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
+  element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
   return JS_NULL;
 }
 IMPL_PROPERTY_GETTER(ImageElement, height)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
@@ -46,7 +46,7 @@ IMPL_PROPERTY_SETTER(ImageElement, height)(JSContext* ctx, JSValue this_val, int
   std::string key = "height";
   std::unique_ptr<NativeString> args_01 = stringToNativeString(key);
   std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, argv[0]);
-  foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
+  element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
   return JS_NULL;
 }
 IMPL_PROPERTY_GETTER(ImageElement, naturalWidth)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
@@ -69,7 +69,7 @@ IMPL_PROPERTY_SETTER(ImageElement, src)(JSContext* ctx, JSValue this_val, int ar
   std::string key = "src";
   std::unique_ptr<NativeString> args_01 = stringToNativeString(key);
   std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, argv[0]);
-  foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
+  element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
   return JS_NULL;
 }
 IMPL_PROPERTY_GETTER(ImageElement, loading)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
@@ -82,7 +82,7 @@ IMPL_PROPERTY_SETTER(ImageElement, loading)(JSContext* ctx, JSValue this_val, in
   std::string key = "loading";
   std::unique_ptr<NativeString> args_01 = stringToNativeString(key);
   std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, argv[0]);
-  foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
+  element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
   return JS_NULL;
 }
 

bridge/bindings/qjs/dom/event_target.cc

@@ -75,7 +75,7 @@ JSValue EventTarget::addEventListener(JSContext* ctx, JSValue this_val, int argc
     NativeString args_01{};
     buildUICommandArgs(ctx, eventTypeValue, args_01);
 
-    foundation::UICommandBuffer::instance(contextId)->addCommand(eventTargetInstance->m_eventTargetId, UICommand::addEvent, args_01, nullptr);
+    eventTargetInstance->m_context->uiCommandBuffer()->addCommand(eventTargetInstance->m_eventTargetId, UICommand::addEvent, args_01, nullptr);
   }
 
   eventTargetInstance->m_eventListenerMap.add(eventType, JS_DupValue(ctx, callback));
@@ -120,7 +120,7 @@ JSValue EventTarget::removeEventListener(JSContext* ctx, JSValue this_val, int a
     NativeString args_01{};
     buildUICommandArgs(ctx, eventTypeValue, args_01);
 
-    foundation::UICommandBuffer::instance(contextId)->addCommand(eventTargetInstance->m_eventTargetId, UICommand::removeEvent, args_01, nullptr);
+    eventTargetInstance->m_context->uiCommandBuffer()->addCommand(eventTargetInstance->m_eventTargetId, UICommand::removeEvent, args_01, nullptr);
   }
 
   JS_FreeAtom(ctx, eventType);
@@ -262,7 +262,7 @@ JSClassID EventTargetInstance::classId() {
 }
 
 EventTargetInstance::~EventTargetInstance() {
-  foundation::UICommandBuffer::instance(m_contextId)->addCommand(m_eventTargetId, UICommand::disposeEventTarget, nullptr, false);
+  m_context->uiCommandBuffer()->addCommand(m_eventTargetId, UICommand::disposeEventTarget, nullptr, false);
   getDartMethod()->flushUICommand();
   delete nativeEventTarget;
 }
@@ -365,7 +365,7 @@ int EventTargetInstance::setProperty(JSContext* ctx, JSValue obj, JSAtom atom, J
     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);
-      foundation::UICommandBuffer::instance(eventTarget->m_contextId)->addCommand(eventTarget->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
+      eventTarget->m_context->uiCommandBuffer()->addCommand(eventTarget->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
     }
   }
 
@@ -418,7 +418,7 @@ void EventTargetInstance::setAttributesEventHandler(JSString* p, JSValue value)
     int32_t contextId = m_context->getContextId();
     std::unique_ptr<NativeString> args_01 = atomToNativeString(m_ctx, atom);
     int32_t type = JS_IsFunction(m_ctx, value) ? UICommand::addEvent : UICommand::removeEvent;
-    foundation::UICommandBuffer::instance(contextId)->addCommand(m_eventTargetId, type, *args_01, nullptr);
+    m_context->uiCommandBuffer()->addCommand(m_eventTargetId, type, *args_01, nullptr);
   }
 
   JS_FreeAtom(m_ctx, atom);

bridge/bindings/qjs/dom/node.cc

@@ -279,7 +279,7 @@ JSValue Node::copyNodeValue(JSContext* ctx, NodeInstance* node) {
 
     std::string newNodeEventTargetId = std::to_string(newElement->m_eventTargetId);
     std::unique_ptr<NativeString> args_01 = stringToNativeString(newNodeEventTargetId);
-    foundation::UICommandBuffer::instance(newElement->context()->getContextId())->addCommand(element->m_eventTargetId, UICommand::cloneNode, *args_01, nullptr);
+    element->m_context->uiCommandBuffer()->addCommand(element->m_eventTargetId, UICommand::cloneNode, *args_01, nullptr);
 
     return newElement->jsObject;
   } else if (node->nodeType == TEXT_NODE) {
@@ -425,7 +425,7 @@ void NodeInstance::internalAppendChild(NodeInstance* node) {
   std::unique_ptr<NativeString> args_01 = stringToNativeString(nodeEventTargetId);
   std::unique_ptr<NativeString> args_02 = stringToNativeString(position);
 
-  foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(m_eventTargetId, UICommand::insertAdjacentNode, *args_01, *args_02, nullptr);
+  m_context->uiCommandBuffer()->addCommand(m_eventTargetId, UICommand::insertAdjacentNode, *args_01, *args_02, nullptr);
 }
 void NodeInstance::internalRemove() {
   if (JS_IsNull(parentNode))
@@ -441,7 +441,7 @@ void NodeInstance::internalClearChild() {
     auto* node = static_cast<NodeInstance*>(JS_GetOpaque(v, Node::classId(v)));
     node->removeParentNode();
     node->_notifyNodeRemoved(this);
-    foundation::UICommandBuffer::instance(node->m_context->getContextId())->addCommand(node->m_eventTargetId, UICommand::removeNode, nullptr);
+    node->m_context->uiCommandBuffer()->addCommand(node->m_eventTargetId, UICommand::removeNode, nullptr);
     JS_FreeValue(m_ctx, v);
   }
 
@@ -454,7 +454,7 @@ NodeInstance* NodeInstance::internalRemoveChild(NodeInstance* node) {
     arraySpliceValue(m_ctx, childNodes, idx, 1);
     node->removeParentNode();
     node->_notifyNodeRemoved(this);
-    foundation::UICommandBuffer::instance(node->m_context->getContextId())->addCommand(node->m_eventTargetId, UICommand::removeNode, nullptr);
+    node->m_context->uiCommandBuffer()->addCommand(node->m_eventTargetId, UICommand::removeNode, nullptr);
   }
 
   return node;
@@ -487,7 +487,7 @@ JSValue NodeInstance::internalInsertBefore(NodeInstance* node, NodeInstance* ref
       std::unique_ptr<NativeString> args_01 = stringToNativeString(nodeEventTargetId);
       std::unique_ptr<NativeString> args_02 = stringToNativeString(position);
 
-      foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(referenceNode->m_eventTargetId, UICommand::insertAdjacentNode, *args_01, *args_02, nullptr);
+      m_context->uiCommandBuffer()->addCommand(referenceNode->m_eventTargetId, UICommand::insertAdjacentNode, *args_01, *args_02, nullptr);
     }
   }
 
@@ -519,9 +519,9 @@ JSValue NodeInstance::internalReplaceChild(NodeInstance* newChild, NodeInstance*
   std::unique_ptr<NativeString> args_01 = stringToNativeString(newChildEventTargetId);
   std::unique_ptr<NativeString> args_02 = stringToNativeString(position);
 
-  foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(oldChild->m_eventTargetId, UICommand::insertAdjacentNode, *args_01, *args_02, nullptr);
+  m_context->uiCommandBuffer()->addCommand(oldChild->m_eventTargetId, UICommand::insertAdjacentNode, *args_01, *args_02, nullptr);
 
-  foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(oldChild->m_eventTargetId, UICommand::removeNode, nullptr);
+  m_context->uiCommandBuffer()->addCommand(oldChild->m_eventTargetId, UICommand::removeNode, nullptr);
 
   return oldChild->jsObject;
 }

bridge/bindings/qjs/dom/style_declaration.cc

@@ -125,7 +125,7 @@ bool StyleDeclarationInstance::internalSetProperty(std::string& name, JSValue va
   if (ownerEventTarget != nullptr) {
     std::unique_ptr<NativeString> args_01 = stringToNativeString(name);
     std::unique_ptr<NativeString> args_02 = jsValueToNativeString(m_ctx, value);
-    foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(ownerEventTarget->eventTargetId(), UICommand::setStyle, *args_01, *args_02, nullptr);
+    m_context->uiCommandBuffer()->addCommand(ownerEventTarget->eventTargetId(), UICommand::setStyle, *args_01, *args_02, nullptr);
   }
 
   return true;
@@ -143,7 +143,7 @@ void StyleDeclarationInstance::internalRemoveProperty(std::string& name) {
   if (ownerEventTarget != nullptr) {
     std::unique_ptr<NativeString> args_01 = stringToNativeString(name);
     std::unique_ptr<NativeString> args_02 = jsValueToNativeString(m_ctx, JS_NULL);
-    foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(ownerEventTarget->eventTargetId(), UICommand::setStyle, *args_01, *args_02, nullptr);
+    m_context->uiCommandBuffer()->addCommand(ownerEventTarget->eventTargetId(), UICommand::setStyle, *args_01, *args_02, nullptr);
   }
 }
 

bridge/bindings/qjs/dom/text_node.cc

@@ -63,7 +63,7 @@ IMPL_PROPERTY_GETTER(TextNode, nodeName)(JSContext* ctx, JSValue this_val, int a
 TextNodeInstance::TextNodeInstance(TextNode* textNode, JSValue text) : NodeInstance(textNode, NodeType::TEXT_NODE, TextNode::classId(), "TextNode") {
   m_data = jsValueToStdString(m_ctx, text);
   std::unique_ptr<NativeString> args_01 = stringToNativeString(m_data);
-  foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(m_eventTargetId, UICommand::createTextNode, *args_01, nativeEventTarget);
+  m_context->uiCommandBuffer()->addCommand(m_eventTargetId, UICommand::createTextNode, *args_01, nativeEventTarget);
 }
 
 TextNodeInstance::~TextNodeInstance() {}
@@ -81,6 +81,6 @@ void TextNodeInstance::internalSetTextContent(JSValue content) {
   std::string key = "data";
   std::unique_ptr<NativeString> args_01 = stringToNativeString(key);
   std::unique_ptr<NativeString> args_02 = jsValueToNativeString(m_ctx, content);
-  foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
+  m_context->uiCommandBuffer()->addCommand(m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);
 }
 }  // namespace kraken::binding::qjs

bridge/bindings/qjs/executing_context.h

@@ -17,6 +17,7 @@
 #include <mutex>
 #include <unordered_map>
 #include "bindings/qjs/bom/dom_timer_coordinator.h"
+#include "foundation/ui_command_buffer.h"
 #include "garbage_collected.h"
 #include "js_context_macros.h"
 #include "kraken_foundation.h"
@@ -94,6 +95,7 @@ class ExecutionContext {
   DOMTimerCoordinator* timers();
 
   FORCE_INLINE DocumentInstance* document() { return m_document; };
+  FORCE_INLINE foundation::UICommandBuffer* uiCommandBuffer() { return &m_commandBuffer; };
 
   void trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func);
 
@@ -129,6 +131,7 @@ class ExecutionContext {
   DocumentInstance* m_document{nullptr};
   DOMTimerCoordinator m_timers;
   ExecutionContextGCTracker* m_gcTracker{nullptr};
+  foundation::UICommandBuffer m_commandBuffer{contextId};
 };
 
 // The read object's method or properties via Proxy, we should redirect this_val from Proxy into target property of

bridge/foundation/ui_command_buffer.cc

@@ -3,6 +3,7 @@
  * Author: Kraken Team.
  */
 
+#include "ui_command_buffer.h"
 #include "dart_methods.h"
 #include "include/kraken_bridge.h"
 
@@ -55,16 +56,6 @@ void UICommandBuffer::addCommand(int32_t id, int32_t type, NativeString& args_01
   queue.emplace_back(item);
 }
 
-UICommandBuffer* UICommandBuffer::instance(int32_t contextId) {
-  static std::unordered_map<int32_t, UICommandBuffer*> instanceMap;
-
-  if (instanceMap.count(contextId) == 0) {
-    instanceMap[contextId] = new UICommandBuffer(contextId);
-  }
-
-  return instanceMap[contextId];
-}
-
 UICommandItem* UICommandBuffer::data() {
   return queue.data();
 }

bridge/foundation/ui_command_buffer.h

@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2020 Alibaba Inc. All rights reserved.
+ * Author: Kraken Team.
+ */
+
+#ifndef KRAKENBRIDGE_FOUNDATION_UI_COMMAND_BUFFER_H_
+#define KRAKENBRIDGE_FOUNDATION_UI_COMMAND_BUFFER_H_
+
+#include "include/kraken_bridge.h"
+
+namespace foundation {
+
+class UICommandBuffer {
+ public:
+  UICommandBuffer() = delete;
+  explicit UICommandBuffer(int32_t contextId);
+  void addCommand(int32_t id, int32_t type, void* nativePtr, bool batchedUpdate);
+  void addCommand(int32_t id, int32_t type, void* nativePtr);
+  void addCommand(int32_t id, int32_t type, NativeString& args_01, NativeString& args_02, void* nativePtr);
+  void addCommand(int32_t id, int32_t type, NativeString& args_01, void* nativePtr);
+  UICommandItem* data();
+  int64_t size();
+  void clear();
+
+ private:
+  int32_t contextId;
+  std::atomic<bool> update_batched{false};
+  std::vector<UICommandItem> queue;
+};
+
+}  // namespace foundation
+
+#endif  // KRAKENBRIDGE_FOUNDATION_UI_COMMAND_BUFFER_H_

bridge/include/kraken_foundation.h

@@ -79,26 +79,6 @@ class UICommandCallbackQueue {
   std::vector<CallbackItem> queue;
 };
 
-class UICommandBuffer {
- public:
-  UICommandBuffer() = delete;
-  explicit UICommandBuffer(int32_t contextId);
-  static KRAKEN_EXPORT UICommandBuffer* instance(int32_t contextId);
-
-  KRAKEN_EXPORT void addCommand(int32_t id, int32_t type, void* nativePtr, bool batchedUpdate);
-  KRAKEN_EXPORT void addCommand(int32_t id, int32_t type, void* nativePtr);
-  KRAKEN_EXPORT void addCommand(int32_t id, int32_t type, NativeString& args_01, NativeString& args_02, void* nativePtr);
-  KRAKEN_EXPORT void addCommand(int32_t id, int32_t type, NativeString& args_01, void* nativePtr);
-  KRAKEN_EXPORT UICommandItem* data();
-  KRAKEN_EXPORT int64_t size();
-  KRAKEN_EXPORT void clear();
-
- private:
-  int32_t contextId;
-  std::atomic<bool> update_batched{false};
-  std::vector<UICommandItem> queue;
-};
-
 typedef int LogSeverity;
 
 // Default log levels. Negative values can be used for verbose log levels.