Skip to content

Commit

Permalink
fix: fix nativeString memory leaks.
Browse files Browse the repository at this point in the history
  • Loading branch information
@andycall
andycall committed Dec 1, 2021
1 parent 699f313 commit 8661b85
Show file tree
Hide file tree
Showing 22 changed files with 113 additions and 100 deletions.
2 changes: 1 addition & 1 deletion bridge/bindings/qjs/dom/custom_event.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ JSValue CustomEvent::initCustomEvent(QjsContext* ctx, JSValue this_val, int argc
}

JSValue typeValue = argv[0];
eventInstance->nativeEvent->type = jsValueToNativeString(ctx, typeValue);
eventInstance->nativeEvent->type = jsValueToNativeString(ctx, typeValue).release();

if (argc <= 2) {
bool canBubble = JS_ToBool(ctx, argv[1]);
Expand Down
4 changes: 2 additions & 2 deletions bridge/bindings/qjs/dom/document.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ JSValue Document::createEvent(QjsContext* ctx, JSValue this_val, int argc, JSVal
JS_FreeCString(ctx, c_eventType);
std::string eventType = std::string(c_eventType);
if (eventType == "Event") {
NativeString* nativeEventType = jsValueToNativeString(ctx, eventTypeValue);
auto nativeEvent = new NativeEvent{nativeEventType};
std::unique_ptr<NativeString> nativeEventType = jsValueToNativeString(ctx, eventTypeValue);
auto nativeEvent = new NativeEvent{nativeEventType.release()};

auto document = static_cast<DocumentInstance*>(JS_GetOpaque(this_val, Document::classId()));
auto e = Event::buildEventInstance(eventType, document->context(), nativeEvent, false);
Expand Down
12 changes: 6 additions & 6 deletions bridge/bindings/qjs/dom/element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ JSValue Element::setAttribute(QjsContext* ctx, JSValue this_val, int argc, JSVal
element->_didModifyAttribute(name, JS_ATOM_NULL, attributeAtom);
}

NativeString* args_01 = stringToNativeString(name);
NativeString* args_02 = jsValueToNativeString(ctx, attributeString);
std::unique_ptr<NativeString> args_01 = stringToNativeString(name);
std::unique_ptr<NativeString> args_02 = jsValueToNativeString(ctx, attributeString);

::foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::setProperty, *args_01, *args_02, nullptr);

Expand Down Expand Up @@ -267,7 +267,7 @@ JSValue Element::removeAttribute(QjsContext* ctx, JSValue this_val, int argc, JS
element->m_attributes->removeAttribute(name);
element->_didModifyAttribute(name, id, JS_ATOM_NULL);

NativeString* args_01 = stringToNativeString(name);
std::unique_ptr<NativeString> args_01 = stringToNativeString(name);
::foundation::UICommandBuffer::instance(element->m_context->getContextId())->addCommand(element->m_eventTargetId, UICommand::removeProperty, *args_01, nullptr);
}

Expand Down Expand Up @@ -399,8 +399,8 @@ PROP_SETTER(ElementInstance, className)(QjsContext* ctx, JSValue this_val, int a
auto* element = static_cast<ElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
JSAtom atom = JS_ValueToAtom(ctx, argv[0]);
element->m_attributes->setAttribute("class", atom);
NativeString* args_01 = stringToNativeString("class");
NativeString* args_02 = jsValueToNativeString(ctx, 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);
JS_FreeAtom(ctx, atom);
return JS_NULL;
Expand Down Expand Up @@ -846,7 +846,7 @@ ElementInstance::ElementInstance(Element* element, std::string tagName, bool sho
JS_DefinePropertyValueStr(m_ctx, instanceObject, "attributes", m_attributes->jsObject, JS_PROP_C_W_E);

if (shouldAddUICommand) {
NativeString* args_01 = stringToNativeString(tagName);
std::unique_ptr<NativeString> args_01 = stringToNativeString(tagName);
::foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(m_eventTargetId, UICommand::createElement, *args_01, nativeEventTarget);
}
}
Expand Down
16 changes: 8 additions & 8 deletions bridge/bindings/qjs/dom/elements/image_element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ PROP_GETTER(ImageElementInstance, width)(QjsContext* ctx, JSValue this_val, int
PROP_SETTER(ImageElementInstance, width)(QjsContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto* element = static_cast<ImageElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
std::string key = "width";
NativeString* args_01 = stringToNativeString(key);
NativeString* args_02 = jsValueToNativeString(ctx, argv[0]);
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);
return JS_NULL;
}
Expand All @@ -44,8 +44,8 @@ PROP_GETTER(ImageElementInstance, height)(QjsContext* ctx, JSValue this_val, int
PROP_SETTER(ImageElementInstance, height)(QjsContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto* element = static_cast<ImageElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
std::string key = "height";
NativeString* args_01 = stringToNativeString(key);
NativeString* args_02 = jsValueToNativeString(ctx, argv[0]);
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);
return JS_NULL;
}
Expand Down Expand Up @@ -73,8 +73,8 @@ PROP_GETTER(ImageElementInstance, src)(QjsContext* ctx, JSValue this_val, int ar
PROP_SETTER(ImageElementInstance, src)(QjsContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto* element = static_cast<ImageElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
std::string key = "src";
NativeString* args_01 = stringToNativeString(key);
NativeString* args_02 = jsValueToNativeString(ctx, argv[0]);
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);
return JS_NULL;
}
Expand All @@ -86,8 +86,8 @@ PROP_GETTER(ImageElementInstance, loading)(QjsContext* ctx, JSValue this_val, in
PROP_SETTER(ImageElementInstance, loading)(QjsContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto* element = static_cast<ImageElementInstance*>(JS_GetOpaque(this_val, Element::classId()));
std::string key = "loading";
NativeString* args_01 = stringToNativeString(key);
NativeString* args_02 = jsValueToNativeString(ctx, argv[0]);
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);
return JS_NULL;
}
Expand Down
6 changes: 3 additions & 3 deletions bridge/bindings/qjs/dom/event.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ JSValue Event::instanceConstructor(QjsContext* ctx, JSValue func_obj, JSValue th
JSValue eventTypeValue = argv[0];
std::string eventType = jsValueToStdString(ctx, eventTypeValue);

auto* nativeEvent = new NativeEvent{stringToNativeString(eventType)};
auto* nativeEvent = new NativeEvent{stringToNativeString(eventType).release()};
auto* event = Event::buildEventInstance(eventType, m_context, nativeEvent, false);
return event->instanceObject;
}
Expand Down Expand Up @@ -200,7 +200,7 @@ JSValue Event::initEvent(QjsContext* ctx, JSValue this_val, int argc, JSValue* a
}

auto* event = static_cast<EventInstance*>(JS_GetOpaque(this_val, Event::kEventClassID));
event->nativeEvent->type = jsValueToNativeString(ctx, typeValue);
event->nativeEvent->type = jsValueToNativeString(ctx, typeValue).release();

if (!JS_IsNull(bubblesValue)) {
event->nativeEvent->bubbles = JS_IsBool(bubblesValue) ? 1 : 0;
Expand All @@ -218,7 +218,7 @@ EventInstance* EventInstance::fromNativeEvent(Event* event, NativeEvent* nativeE
EventInstance::EventInstance(Event* event, NativeEvent* nativeEvent) : nativeEvent(nativeEvent), Instance(event, "Event", nullptr, Event::kEventClassID, finalizer) {}
EventInstance::EventInstance(Event* jsEvent, JSAtom eventType, JSValue eventInit) : Instance(jsEvent, "Event", nullptr, Event::kEventClassID, finalizer) {
JSValue v = JS_AtomToValue(m_ctx, eventType);
nativeEvent = new NativeEvent{jsValueToNativeString(m_ctx, v)};
nativeEvent = new NativeEvent{jsValueToNativeString(m_ctx, v).release()};
JS_FreeValue(m_ctx, v);

auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch());
Expand Down
8 changes: 4 additions & 4 deletions bridge/bindings/qjs/dom/event_target.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,8 @@ int EventTargetInstance::setProperty(QjsContext* ctx, JSValue obj, JSAtom atom,
JS_SetProperty(ctx, eventTarget->m_properties, atom, JS_DupValue(ctx, value));

if (isJavaScriptExtensionElementInstance(eventTarget->context(), eventTarget->instanceObject) && !p->is_wide_char && p->u.str8[0] != '_') {
NativeString* args_01 = atomToNativeString(ctx, atom);
NativeString* args_02 = jsValueToNativeString(ctx, value);
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);
}
}
Expand All @@ -390,7 +390,7 @@ JSValue EventTargetInstance::callNativeMethods(const char* method, int32_t argc,
std::u16string methodString;
fromUTF8(method, methodString);

NativeString m{reinterpret_cast<const uint16_t*>(methodString.c_str()), static_cast<int32_t>(methodString.size())};
NativeString m{reinterpret_cast<const uint16_t*>(methodString.c_str()), static_cast<uint32_t>(methodString.size())};

NativeValue nativeValue{};
nativeEventTarget->callNativeMethods(nativeEventTarget, &nativeValue, &m, argc, argv);
Expand Down Expand Up @@ -425,7 +425,7 @@ void EventTargetInstance::setPropertyHandler(JSString* p, JSValue value) {
int32_t eventHandlerLen = arrayGetLength(m_ctx, m_eventHandlers);
if (eventHandlerLen == 0) {
int32_t contextId = m_context->getContextId();
NativeString* args_01 = atomToNativeString(m_ctx, atom);
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);
}
Expand Down
2 changes: 1 addition & 1 deletion bridge/bindings/qjs/dom/events/touch_event.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ JSValue TouchEvent::instanceConstructor(QjsContext* ctx, JSValue func_obj, JSVal
}

auto* nativeEvent = new NativeTouchEvent();
nativeEvent->nativeEvent.type = jsValueToNativeString(ctx, eventTypeValue);
nativeEvent->nativeEvent.type = jsValueToNativeString(ctx, eventTypeValue).release();

if (JS_IsObject(eventInit)) {
JSAtom touchesAtom = JS_NewAtom(m_ctx, "touches");
Expand Down
14 changes: 7 additions & 7 deletions bridge/bindings/qjs/dom/node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ JSValue Node::copyNodeValue(QjsContext* ctx, NodeInstance* node) {
ElementInstance::copyNodeProperties(newElement, element);

std::string newNodeEventTargetId = std::to_string(newElement->m_eventTargetId);
NativeString* args_01 = stringToNativeString(newNodeEventTargetId);
std::unique_ptr<NativeString> args_01 = stringToNativeString(newNodeEventTargetId);
foundation::UICommandBuffer::instance(newElement->context()->getContextId())->addCommand(element->m_eventTargetId, UICommand::cloneNode, *args_01, nullptr);

return newElement->instanceObject;
Expand Down Expand Up @@ -443,8 +443,8 @@ void NodeInstance::internalAppendChild(NodeInstance* node) {
std::string nodeEventTargetId = std::to_string(node->m_eventTargetId);
std::string position = std::string("beforeend");

NativeString* args_01 = stringToNativeString(nodeEventTargetId);
NativeString* args_02 = stringToNativeString(position);
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);
}
Expand Down Expand Up @@ -505,8 +505,8 @@ JSValue NodeInstance::internalInsertBefore(NodeInstance* node, NodeInstance* ref
std::string nodeEventTargetId = std::to_string(node->m_eventTargetId);
std::string position = std::string("beforebegin");

NativeString* args_01 = stringToNativeString(nodeEventTargetId);
NativeString* args_02 = stringToNativeString(position);
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);
}
Expand Down Expand Up @@ -537,8 +537,8 @@ JSValue NodeInstance::internalReplaceChild(NodeInstance* newChild, NodeInstance*
std::string newChildEventTargetId = std::to_string(newChild->m_eventTargetId);
std::string position = std::string("afterend");

NativeString* args_01 = stringToNativeString(newChildEventTargetId);
NativeString* args_02 = stringToNativeString(position);
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);

Expand Down
8 changes: 4 additions & 4 deletions bridge/bindings/qjs/dom/style_declaration.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ bool StyleDeclarationInstance::internalSetProperty(std::string& name, JSValue va
properties[name] = jsValueToStdString(m_ctx, value);

if (ownerEventTarget != nullptr) {
NativeString* args_01 = stringToNativeString(name);
NativeString* args_02 = jsValueToNativeString(m_ctx, value);
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);
}

Expand All @@ -141,8 +141,8 @@ void StyleDeclarationInstance::internalRemoveProperty(std::string& name) {
properties.erase(name);

if (ownerEventTarget != nullptr) {
NativeString* args_01 = stringToNativeString(name);
NativeString* args_02 = jsValueToNativeString(m_ctx, JS_NULL);
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);
}
}
Expand Down
6 changes: 3 additions & 3 deletions bridge/bindings/qjs/dom/text_node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ PROP_SETTER(TextNodeInstance, nodeName)(QjsContext* ctx, JSValue this_val, int a

TextNodeInstance::TextNodeInstance(TextNode* textNode, JSValue text)
: NodeInstance(textNode, NodeType::TEXT_NODE, DocumentInstance::instance(Document::instance(textNode->m_context)), TextNode::classId(), "TextNode"), m_data(JS_DupValue(m_ctx, text)) {
NativeString* args_01 = jsValueToNativeString(m_ctx, m_data);
std::unique_ptr<NativeString> args_01 = jsValueToNativeString(m_ctx, m_data);
foundation::UICommandBuffer::instance(m_context->getContextId())->addCommand(m_eventTargetId, UICommand::createTextNode, *args_01, nativeEventTarget);
}

Expand All @@ -91,8 +91,8 @@ void TextNodeInstance::internalSetTextContent(JSValue content) {
m_data = JS_DupValue(m_ctx, content);

std::string key = "data";
NativeString* args_01 = stringToNativeString(key);
NativeString* args_02 = jsValueToNativeString(m_ctx, content);
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);
}
} // namespace kraken::binding::qjs
19 changes: 9 additions & 10 deletions bridge/bindings/qjs/js_context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ void JSContext::promiseRejectTracker(QjsContext* ctx, JSValue promise, JSValue r
context->dispatchGlobalPromiseRejectionEvent(promise, reason);
}

NativeString* jsValueToNativeString(QjsContext* ctx, JSValue value) {
std::unique_ptr<NativeString> jsValueToNativeString(QjsContext* ctx, JSValue value) {
bool isValueString = true;
if (JS_IsNull(value)) {
value = JS_NewString(ctx, "");
Expand All @@ -344,15 +344,14 @@ NativeString* jsValueToNativeString(QjsContext* ctx, JSValue value) {

uint32_t length;
uint16_t* buffer = JS_ToUnicode(ctx, value, &length);
NativeString tmp{};
tmp.string = buffer;
tmp.length = length;
NativeString* cloneString = tmp.clone();
std::unique_ptr<NativeString> ptr = std::make_unique<NativeString>();
ptr->string = buffer;
ptr->length = length;

if (!isValueString) {
JS_FreeValue(ctx, value);
}
return cloneString;
return ptr;
}

void buildUICommandArgs(QjsContext* ctx, JSValue key, NativeString& args_01) {
Expand All @@ -365,18 +364,18 @@ void buildUICommandArgs(QjsContext* ctx, JSValue key, NativeString& args_01) {
args_01.length = length;
}

NativeString* stringToNativeString(const std::string& string) {
std::unique_ptr<NativeString> stringToNativeString(const std::string& string) {
std::u16string utf16;
fromUTF8(string, utf16);
NativeString tmp{};
tmp.string = reinterpret_cast<const uint16_t*>(utf16.c_str());
tmp.length = utf16.size();
return tmp.clone();
return std::unique_ptr<NativeString>(tmp.clone());
}

NativeString* atomToNativeString(QjsContext* ctx, JSAtom atom) {
std::unique_ptr<NativeString> atomToNativeString(QjsContext* ctx, JSAtom atom) {
JSValue stringValue = JS_AtomToString(ctx, atom);
NativeString* string = jsValueToNativeString(ctx, stringValue);
std::unique_ptr<NativeString> string = jsValueToNativeString(ctx, stringValue);
JS_FreeValue(ctx, stringValue);
return string;
}
Expand Down
22 changes: 18 additions & 4 deletions bridge/bindings/qjs/js_context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,19 +193,33 @@ class JSValueHolder {
};

std::unique_ptr<JSContext> createJSContext(int32_t contextId, const JSExceptionHandler& handler, void* owner);
NativeString* jsValueToNativeString(QjsContext* ctx, JSValue value);

// Convert to string and return a full copy of NativeString from JSValue.
std::unique_ptr<NativeString> jsValueToNativeString(QjsContext* ctx, JSValue value);

void buildUICommandArgs(QjsContext* ctx, JSValue key, NativeString& args_01);
NativeString* stringToNativeString(const std::string& string);
NativeString* atomToNativeString(QjsContext* ctx, JSAtom atom);

// Encode utf-8 to utf-16, and return a full copy of NativeString.
std::unique_ptr<NativeString> stringToNativeString(const std::string& string);

// Return a full copy of NativeString form JSAtom.
std::unique_ptr<NativeString> atomToNativeString(QjsContext* ctx, JSAtom atom);

// Convert to string and return a full copy of std::string from JSValue.
std::string jsValueToStdString(QjsContext* ctx, JSValue& value);

// Return a full copy of std::string form JSAtom.
std::string jsAtomToStdString(QjsContext* ctx, JSAtom atom);
void extractErrorInfo(JSValueConst error);

// JS array operation utilities.
void arrayPushValue(QjsContext* ctx, JSValue array, JSValue val);
void arrayInsert(QjsContext* ctx, JSValue array, uint32_t start, JSValue targetValue);
int32_t arrayGetLength(QjsContext* ctx, JSValue array);
int32_t arrayFindIdx(QjsContext* ctx, JSValue array, JSValue target);
void arraySpliceValue(QjsContext* ctx, JSValue array, uint32_t start, uint32_t deleteCount);
void arraySpliceValue(QjsContext* ctx, JSValue array, uint32_t start, uint32_t deleteCount, JSValue replacedValue);

// JS object operation utilities.
JSValue objectGetKeys(QjsContext* ctx, JSValue obj);

} // namespace kraken::binding::qjs
Expand Down

0 comments on commit 8661b85

Please sign in to comment.