Skip to content

Commit 7da257d

Browse files
andycallandycall
authored
Merge pull request #836 from openkraken/fix/proxy_element
fix: fix read property if element wrapped by Proxy.
2 parents 9c05f90 + 892c7c8 commit 7da257d

File tree

7 files changed

+166
-23
lines changed

7 files changed

+166
-23
lines changed

‎bridge/bindings/qjs/dom/element.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -862,9 +862,9 @@ ElementInstance::ElementInstance(Element *element, std::string tagName, bool sho
862862
m_style = static_cast<StyleDeclarationInstance *>(JS_GetOpaque(style, CSSStyleDeclaration::kCSSStyleDeclarationClassId));
863863

864864
JS_DefinePropertyValueStr(m_ctx, instanceObject, "style", m_style->instanceObject,
865-
JS_PROP_NORMAL | JS_PROP_ENUMERABLE);
865+
JS_PROP_C_W_E);
866866
JS_DefinePropertyValueStr(m_ctx, instanceObject, "attributes", m_attributes->jsObject,
867-
JS_PROP_NORMAL | JS_PROP_ENUMERABLE);
867+
JS_PROP_C_W_E);
868868

869869
if (shouldAddUICommand) {
870870
NativeString *args_01 = stringToNativeString(tagName);

‎bridge/bindings/qjs/host_object_test.cc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,36 @@ TEST(HostObject, defineProperty) {
6767
EXPECT_EQ(errorCalled, false);
6868
}
6969

70+
71+
TEST(ObjectProperty, worksWithProxy) {
72+
bool static logCalled = false;
73+
bool static errorCalled = false;
74+
kraken::JSBridge::consoleMessageHandler = [](void *ctx, const std::string &message, int logLevel) {
75+
logCalled = true;
76+
EXPECT_STREQ(message.c_str(), "0");
77+
};
78+
auto *bridge = new kraken::JSBridge(0, [](int32_t contextId, const char* errmsg) {
79+
KRAKEN_LOG(VERBOSE) << errmsg;
80+
errorCalled = true;
81+
});
82+
auto &context = bridge->getContext();
83+
auto *sampleObject = new SampleObject(context.get());
84+
JSValue object = sampleObject->jsObject;
85+
context->defineGlobalProperty("o", object);
86+
std::string code = std::string(R"(
87+
let p = new Proxy(o, {
88+
get(target, key, receiver) {
89+
return Reflect.get(target, key, receiver);
90+
}
91+
});
92+
console.log(p.foo);
93+
)");
94+
bridge->evaluateScript(code.c_str(), code.size(), "vm://", 0);
95+
delete bridge;
96+
EXPECT_EQ(logCalled, true);
97+
EXPECT_EQ(errorCalled, false);
98+
}
99+
70100
TEST(HostObject, defineFunction) {
71101
bool static logCalled = false;
72102
bool static errorCalled = false;

‎bridge/bindings/qjs/js_context.h

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@
66
#ifndef KRAKENBRIDGE_JS_CONTEXT_H
77
#define KRAKENBRIDGE_JS_CONTEXT_H
88

9+
#include "js_context_macros.h"
910
#include "kraken_foundation.h"
11+
#include "qjs_patch.h"
1012
#include <memory>
11-
#include <unordered_map>
12-
#include <quickjs/quickjs.h>
1313
#include <quickjs/list.h>
14-
#include "js_context_macros.h"
14+
#include <quickjs/quickjs.h>
15+
#include <unordered_map>
1516

1617
using QjsContext = JSContext;
1718
using JSExceptionHandler = std::function<void(int32_t contextId, const char *message)>;
@@ -66,10 +67,10 @@ class JSContext {
6667
bool handleException(JSValue *exc);
6768
void drainPendingPromiseJobs();
6869
void defineGlobalProperty(const char *prop, JSValueConst value);
69-
uint8_t *dumpByteCode(const char* code, uint32_t codeLength, const char *sourceURL, size_t* bytecodeLength);
70+
uint8_t *dumpByteCode(const char *code, uint32_t codeLength, const char *sourceURL, size_t *bytecodeLength);
7071

7172
std::chrono::time_point<std::chrono::system_clock> timeOrigin;
72-
std::unordered_map<std::string, void*> constructorMap;
73+
std::unordered_map<std::string, void *> constructorMap;
7374

7475
int32_t uniqueId;
7576
struct list_head node_job_list;
@@ -86,7 +87,6 @@ class JSContext {
8687
static JSClassID kHostExoticObjectClassId;
8788

8889
private:
89-
9090
static void promiseRejectTracker(QjsContext *ctx, JSValueConst promise, JSValueConst reason, JS_BOOL is_handled,
9191
void *opaque);
9292
void dispatchGlobalErrorEvent(JSValueConst error);
@@ -104,26 +104,46 @@ class JSContext {
104104
WindowInstance *m_window{nullptr};
105105
};
106106

107+
// The read object's method or properties via Proxy, we should redirect this_val from Proxy into target property of
108+
// proxy object.
109+
static JSValue handleCallThisOnProxy(QjsContext *ctx, JSValueConst this_val, int argc, JSValueConst *argv, int data_len,
110+
JSValueConst *data) {
111+
JSValue f = data[0];
112+
JSValue result;
113+
if (JS_IsProxy(this_val)) {
114+
result = JS_Call(ctx, f, JS_GetProxyTarget(this_val), argc, argv);
115+
} else {
116+
result = JS_Call(ctx, f, this_val, argc, argv);
117+
}
118+
return result;
119+
}
120+
107121
class ObjectProperty {
108122
KRAKEN_DISALLOW_COPY_ASSIGN_AND_MOVE(ObjectProperty);
109123

110124
public:
111125
ObjectProperty() = delete;
112-
explicit ObjectProperty(JSContext *context, JSValueConst thisObject, const char *property,
113-
JSCFunction getterFunction, JSCFunction setterFunction) {
126+
explicit ObjectProperty(JSContext *context, JSValueConst thisObject, const char *property, JSCFunction getterFunction,
127+
JSCFunction setterFunction) {
114128
JSValue ge = JS_NewCFunction(context->ctx(), getterFunction, "get", 0);
115129
JSValue se = JS_NewCFunction(context->ctx(), setterFunction, "set", 1);
130+
131+
JSValue pge = JS_NewCFunctionData(context->ctx(), handleCallThisOnProxy, 0, 0, 1, &ge);
132+
JSValue pse = JS_NewCFunctionData(context->ctx(), handleCallThisOnProxy, 1, 0, 1, &se);
133+
134+
JS_FreeValue(context->ctx(), ge);
135+
JS_FreeValue(context->ctx(), se);
136+
116137
JSAtom key = JS_NewAtom(context->ctx(), property);
117-
JS_DefinePropertyGetSet(context->ctx(), thisObject, key, ge, se,
118-
JS_PROP_C_W_E);
138+
JS_DefinePropertyGetSet(context->ctx(), thisObject, key, pge, pse, JS_PROP_C_W_E);
119139
JS_FreeAtom(context->ctx(), key);
120140
};
121141
explicit ObjectProperty(JSContext *context, JSValueConst thisObject, const char *property,
122142
JSCFunction getterFunction) {
123143
JSValue get = JS_NewCFunction(context->ctx(), getterFunction, "get", 0);
124144
JSAtom key = JS_NewAtom(context->ctx(), property);
125145
JS_DefineProperty(context->ctx(), thisObject, key, JS_UNDEFINED, get, JS_UNDEFINED,
126-
JS_PROP_HAS_CONFIGURABLE | JS_PROP_ENUMERABLE | JS_PROP_HAS_GET);
146+
JS_PROP_HAS_CONFIGURABLE | JS_PROP_ENUMERABLE | JS_PROP_HAS_GET);
127147
JS_FreeAtom(context->ctx(), key);
128148
}
129149
};
@@ -133,26 +153,29 @@ class ObjectFunction {
133153

134154
public:
135155
ObjectFunction() = delete;
136-
explicit ObjectFunction(JSContext *context, JSValueConst thisObject, const char *functionName,
137-
JSCFunction function, int argc) {
156+
explicit ObjectFunction(JSContext *context, JSValueConst thisObject, const char *functionName, JSCFunction function,
157+
int argc) {
138158
JSValue f = JS_NewCFunction(context->ctx(), function, functionName, argc);
159+
JSValue pf = JS_NewCFunctionData(context->ctx(), handleCallThisOnProxy, argc, 0, 1, &f);
139160
JSAtom key = JS_NewAtom(context->ctx(), functionName);
140161

162+
JS_FreeValue(context->ctx(), f);
163+
141164
// We should avoid overwrite exist property functions.
142165
#ifdef DEBUG
143-
assert_m(JS_HasProperty(context->ctx(), thisObject, key) == 0, (std::string("Found exist function property: ") + std::string(functionName)).c_str());
166+
assert_m(JS_HasProperty(context->ctx(), thisObject, key) == 0,
167+
(std::string("Found exist function property: ") + std::string(functionName)).c_str());
144168
#endif
145169

146-
JS_DefinePropertyValue(context->ctx(), thisObject, key, f,
147-
JS_PROP_ENUMERABLE);
170+
JS_DefinePropertyValue(context->ctx(), thisObject, key, pf, JS_PROP_ENUMERABLE);
148171
JS_FreeAtom(context->ctx(), key);
149172
};
150173
};
151174

152175
class JSValueHolder {
153176
public:
154177
JSValueHolder() = delete;
155-
explicit JSValueHolder(QjsContext *ctx, JSValue value): m_value(value), m_ctx(ctx) {};
178+
explicit JSValueHolder(QjsContext *ctx, JSValue value) : m_value(value), m_ctx(ctx){};
156179
~JSValueHolder() {
157180
JS_FreeValue(m_ctx, m_value);
158181
}
@@ -162,7 +185,10 @@ class JSValueHolder {
162185
}
163186
m_value = JS_DupValue(m_ctx, value);
164187
};
165-
inline JSValue value() const { return JS_DupValue(m_ctx, m_value); }
188+
inline JSValue value() const {
189+
return JS_DupValue(m_ctx, m_value);
190+
}
191+
166192
private:
167193
QjsContext *m_ctx{nullptr};
168194
JSValue m_value{JS_NULL};
@@ -184,7 +210,6 @@ void arraySpliceValue(QjsContext *ctx, JSValue array, uint32_t start, uint32_t d
184210
void arraySpliceValue(QjsContext *ctx, JSValue array, uint32_t start, uint32_t deleteCount, JSValue replacedValue);
185211
JSValue objectGetKeys(QjsContext *ctx, JSValue obj);
186212

187-
188213
} // namespace kraken::binding::qjs
189214

190215
#endif // KRAKENBRIDGE_JS_CONTEXT_H

‎bridge/bindings/qjs/qjs_patch.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ typedef enum {
1414
JS_GC_PHASE_REMOVE_CYCLES,
1515
} JSGCPhaseEnum;
1616

17+
typedef struct JSProxyData {
18+
JSValue target;
19+
JSValue handler;
20+
uint8_t is_func;
21+
uint8_t is_revoked;
22+
} JSProxyData;
23+
1724
typedef enum {
1825
JS_GC_OBJ_TYPE_JS_OBJECT,
1926
JS_GC_OBJ_TYPE_FUNCTION_BYTECODE,
@@ -298,3 +305,14 @@ JSClassID JSValueGetClassId(JSValue obj) {
298305
p = JS_VALUE_GET_OBJ(obj);
299306
return p->class_id;
300307
}
308+
309+
bool JS_IsProxy(JSValue value) {
310+
if (!JS_IsObject(value)) return false;
311+
JSObject *p = JS_VALUE_GET_OBJ(value);
312+
return p->class_id == JS_CLASS_PROXY;
313+
}
314+
315+
JSValue JS_GetProxyTarget(JSValue value) {
316+
JSObject *p = JS_VALUE_GET_OBJ(value);
317+
return p->u.proxy_data->target;
318+
}

‎bridge/bindings/qjs/qjs_patch.h

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,83 @@ struct JSString {
2828
} u;
2929
};
3030

31+
32+
enum {
33+
/* classid tag */ /* union usage | properties */
34+
JS_CLASS_OBJECT = 1, /* must be first */
35+
JS_CLASS_ARRAY, /* u.array | length */
36+
JS_CLASS_ERROR,
37+
JS_CLASS_NUMBER, /* u.object_data */
38+
JS_CLASS_STRING, /* u.object_data */
39+
JS_CLASS_BOOLEAN, /* u.object_data */
40+
JS_CLASS_SYMBOL, /* u.object_data */
41+
JS_CLASS_ARGUMENTS, /* u.array | length */
42+
JS_CLASS_MAPPED_ARGUMENTS, /* | length */
43+
JS_CLASS_DATE, /* u.object_data */
44+
JS_CLASS_MODULE_NS,
45+
JS_CLASS_C_FUNCTION, /* u.cfunc */
46+
JS_CLASS_BYTECODE_FUNCTION, /* u.func */
47+
JS_CLASS_BOUND_FUNCTION, /* u.bound_function */
48+
JS_CLASS_C_FUNCTION_DATA, /* u.c_function_data_record */
49+
JS_CLASS_GENERATOR_FUNCTION, /* u.func */
50+
JS_CLASS_FOR_IN_ITERATOR, /* u.for_in_iterator */
51+
JS_CLASS_REGEXP, /* u.regexp */
52+
JS_CLASS_ARRAY_BUFFER, /* u.array_buffer */
53+
JS_CLASS_SHARED_ARRAY_BUFFER, /* u.array_buffer */
54+
JS_CLASS_UINT8C_ARRAY, /* u.array (typed_array) */
55+
JS_CLASS_INT8_ARRAY, /* u.array (typed_array) */
56+
JS_CLASS_UINT8_ARRAY, /* u.array (typed_array) */
57+
JS_CLASS_INT16_ARRAY, /* u.array (typed_array) */
58+
JS_CLASS_UINT16_ARRAY, /* u.array (typed_array) */
59+
JS_CLASS_INT32_ARRAY, /* u.array (typed_array) */
60+
JS_CLASS_UINT32_ARRAY, /* u.array (typed_array) */
61+
#ifdef CONFIG_BIGNUM
62+
JS_CLASS_BIG_INT64_ARRAY, /* u.array (typed_array) */
63+
JS_CLASS_BIG_UINT64_ARRAY, /* u.array (typed_array) */
64+
#endif
65+
JS_CLASS_FLOAT32_ARRAY, /* u.array (typed_array) */
66+
JS_CLASS_FLOAT64_ARRAY, /* u.array (typed_array) */
67+
JS_CLASS_DATAVIEW, /* u.typed_array */
68+
#ifdef CONFIG_BIGNUM
69+
JS_CLASS_BIG_INT, /* u.object_data */
70+
JS_CLASS_BIG_FLOAT, /* u.object_data */
71+
JS_CLASS_FLOAT_ENV, /* u.float_env */
72+
JS_CLASS_BIG_DECIMAL, /* u.object_data */
73+
JS_CLASS_OPERATOR_SET, /* u.operator_set */
74+
#endif
75+
JS_CLASS_MAP, /* u.map_state */
76+
JS_CLASS_SET, /* u.map_state */
77+
JS_CLASS_WEAKMAP, /* u.map_state */
78+
JS_CLASS_WEAKSET, /* u.map_state */
79+
JS_CLASS_MAP_ITERATOR, /* u.map_iterator_data */
80+
JS_CLASS_SET_ITERATOR, /* u.map_iterator_data */
81+
JS_CLASS_ARRAY_ITERATOR, /* u.array_iterator_data */
82+
JS_CLASS_STRING_ITERATOR, /* u.array_iterator_data */
83+
JS_CLASS_REGEXP_STRING_ITERATOR, /* u.regexp_string_iterator_data */
84+
JS_CLASS_GENERATOR, /* u.generator_data */
85+
JS_CLASS_PROXY, /* u.proxy_data */
86+
JS_CLASS_PROMISE, /* u.promise_data */
87+
JS_CLASS_PROMISE_RESOLVE_FUNCTION, /* u.promise_function_data */
88+
JS_CLASS_PROMISE_REJECT_FUNCTION, /* u.promise_function_data */
89+
JS_CLASS_ASYNC_FUNCTION, /* u.func */
90+
JS_CLASS_ASYNC_FUNCTION_RESOLVE, /* u.async_function_data */
91+
JS_CLASS_ASYNC_FUNCTION_REJECT, /* u.async_function_data */
92+
JS_CLASS_ASYNC_FROM_SYNC_ITERATOR, /* u.async_from_sync_iterator_data */
93+
JS_CLASS_ASYNC_GENERATOR_FUNCTION, /* u.func */
94+
JS_CLASS_ASYNC_GENERATOR, /* u.async_generator_data */
95+
96+
JS_CLASS_INIT_COUNT, /* last entry for predefined classes */
97+
};
98+
3199
#ifdef __cplusplus
32100
extern "C" {
33101
#endif
34102

35103
uint16_t *JS_ToUnicode(JSContext *ctx, JSValueConst value, uint32_t *length);
36104
JSValue JS_NewUnicodeString(JSRuntime *runtime, JSContext *ctx, const uint16_t *code, uint32_t length);
37105
JSClassID JSValueGetClassId(JSValue);
106+
bool JS_IsProxy(JSValue value);
107+
JSValue JS_GetProxyTarget(JSValue value);
38108

39109
#ifdef __cplusplus
40110
}

‎integration_tests/snapshots/dom/elements/img.ts.ff77830b2.png

466 Bytes
Loading

‎integration_tests/specs/dom/elements/img.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,8 @@ describe('Tags img', () => {
319319
await snapshot(img);
320320
done();
321321
});
322-
// Delay 600ms to play gif.
323-
}, 600);
322+
// Delay 200ms to play gif.
323+
}, 200);
324324
};
325325

326326
document.body.appendChild(img);

0 commit comments

Comments
 (0)