Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix report error cause stack overflow. #1164

Merged
merged 7 commits into from Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions bridge/bindings/qjs/bom/window.cc
Expand Up @@ -25,6 +25,13 @@ void bindWindow(ExecutionContext* context) {
context->defineGlobalProperty("__window__", window->jsObject);
}

JSValue ensureWindowIsGlobal(EventTargetInstance* target) {
if (target == target->context()->window()) {
return target->context()->global();
}
return target->jsObject;
}

JSClassID Window::kWindowClassId{0};

Window::Window(ExecutionContext* context) : EventTarget(context, "Window") {
Expand Down
4 changes: 4 additions & 0 deletions bridge/bindings/qjs/bom/window.h
Expand Up @@ -59,6 +59,10 @@ class Window : public EventTarget {
friend WindowInstance;
};

// Hack: m_context->window() are not real global object, which are strict equal to globalThis.
// But we configure m_context->window() to simulate globalThis and can access all global properties and methods.
JSValue ensureWindowIsGlobal(EventTargetInstance* target);

class WindowInstance : public EventTargetInstance {
public:
WindowInstance() = delete;
Expand Down
10 changes: 7 additions & 3 deletions bridge/bindings/qjs/dom/event.cc
Expand Up @@ -4,6 +4,7 @@
*/

#include "event.h"
#include "bindings/qjs/bom/window.h"
#include "bindings/qjs/qjs_patch.h"
#include "custom_event.h"
#include "event_target.h"
Expand Down Expand Up @@ -66,27 +67,30 @@ IMPL_PROPERTY_GETTER(Event, defaultPrevented)(JSContext* ctx, JSValue this_val,

IMPL_PROPERTY_GETTER(Event, target)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto* eventInstance = static_cast<EventInstance*>(JS_GetOpaque(this_val, Event::kEventClassID));

if (eventInstance->nativeEvent->target != nullptr) {
auto instance = reinterpret_cast<EventTargetInstance*>(eventInstance->nativeEvent->target);
return JS_DupValue(ctx, instance->jsObject);
return JS_DupValue(ctx, ensureWindowIsGlobal(instance));
}
return JS_NULL;
}

IMPL_PROPERTY_GETTER(Event, srcElement)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto* eventInstance = static_cast<EventInstance*>(JS_GetOpaque(this_val, Event::kEventClassID));

if (eventInstance->nativeEvent->target != nullptr) {
auto instance = reinterpret_cast<EventTargetInstance*>(eventInstance->nativeEvent->target);
return JS_DupValue(ctx, instance->jsObject);
return JS_DupValue(ctx, ensureWindowIsGlobal(instance));
}
return JS_NULL;
}

IMPL_PROPERTY_GETTER(Event, currentTarget)(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
auto* eventInstance = static_cast<EventInstance*>(JS_GetOpaque(this_val, Event::kEventClassID));

if (eventInstance->nativeEvent->currentTarget != nullptr) {
auto instance = reinterpret_cast<EventTargetInstance*>(eventInstance->nativeEvent->currentTarget);
return JS_DupValue(ctx, instance->jsObject);
return JS_DupValue(ctx, ensureWindowIsGlobal(instance));
}
return JS_NULL;
}
Expand Down
1 change: 1 addition & 0 deletions bridge/bindings/qjs/dom/event_target.cc
Expand Up @@ -149,6 +149,7 @@ JSValue EventTarget::dispatchEvent(JSContext* ctx, JSValue this_val, int argc, J

JSValue eventValue = argv[0];
auto eventInstance = reinterpret_cast<EventInstance*>(JS_GetOpaque(eventValue, EventTarget::classId(eventValue)));
eventInstance->nativeEvent->target = eventTargetInstance;
return JS_NewBool(ctx, eventTargetInstance->dispatchEvent(eventInstance));
}

Expand Down
35 changes: 31 additions & 4 deletions bridge/bindings/qjs/executing_context.cc
Expand Up @@ -270,6 +270,31 @@ void ExecutionContext::reportError(JSValueConst error) {
JS_FreeCString(m_ctx, type);
}

void ExecutionContext::reportErrorEvent(EventInstance* errorEvent) {
JSValue error = JS_GetPropertyStr(m_ctx, errorEvent->jsObject, "error");
reportError(error);
JS_FreeValue(m_ctx, error);
}

void ExecutionContext::dispatchErrorEvent(EventInstance* errorEvent) {
if (m_inDispatchErrorEvent_) {
return;
}

dispatchErrorEventInternal(errorEvent);
reportErrorEvent(errorEvent);
}

void ExecutionContext::dispatchErrorEventInternal(EventInstance* errorEvent) {
if (m_window == nullptr)
return;

assert(!m_inDispatchErrorEvent_);
m_inDispatchErrorEvent_ = true;
m_window->dispatchEvent(errorEvent);
m_inDispatchErrorEvent_ = false;
}

void ExecutionContext::drainPendingPromiseJobs() {
// should executing pending promise jobs.
JSContext* pctx;
Expand Down Expand Up @@ -306,7 +331,7 @@ void ExecutionContext::dispatchGlobalErrorEvent(ExecutionContext* context, JSVal
auto* window = static_cast<WindowInstance*>(JS_GetOpaque(context->global(), Window::classId()));

{
JSValue ErrorEventValue = JS_GetPropertyStr(ctx, context->global(), "ErrorEvent");
JSValue errorEventConstructor = JS_GetPropertyStr(ctx, context->global(), "ErrorEvent");
JSValue errorType = JS_NewString(ctx, "error");
JSValue errorInit = JS_NewObject(ctx);
JS_SetPropertyStr(ctx, errorInit, "error", JS_DupValue(ctx, error));
Expand All @@ -315,16 +340,17 @@ void ExecutionContext::dispatchGlobalErrorEvent(ExecutionContext* context, JSVal
JS_SetPropertyStr(ctx, errorInit, "filename", JS_GetPropertyStr(ctx, error, "fileName"));
JS_SetPropertyStr(ctx, errorInit, "colno", JS_NewUint32(ctx, 0));
JSValue arguments[] = {errorType, errorInit};
JSValue errorEventValue = JS_CallConstructor(context->ctx(), ErrorEventValue, 2, arguments);
JSValue errorEventValue = JS_CallConstructor(context->ctx(), errorEventConstructor, 2, arguments);
if (JS_IsException(errorEventValue)) {
context->handleException(&errorEventValue);
return;
}

auto* errorEvent = static_cast<EventInstance*>(JS_GetOpaque(errorEventValue, Event::kEventClassID));
window->dispatchEvent(errorEvent);
errorEvent->nativeEvent->target = window;
context->dispatchErrorEvent(errorEvent);

JS_FreeValue(ctx, ErrorEventValue);
JS_FreeValue(ctx, errorEventConstructor);
JS_FreeValue(ctx, errorEventValue);
JS_FreeValue(ctx, errorType);
JS_FreeValue(ctx, errorInit);
Expand Down Expand Up @@ -352,6 +378,7 @@ static void dispatchPromiseRejectionEvent(const char* eventType, ExecutionContex
}

auto* rejectEvent = static_cast<EventInstance*>(JS_GetOpaque(rejectEventValue, Event::kEventClassID));
rejectEvent->nativeEvent->target = window;
window->dispatchEvent(rejectEvent);

JS_FreeValue(ctx, errorType);
Expand Down
6 changes: 6 additions & 0 deletions bridge/bindings/qjs/executing_context.h
Expand Up @@ -33,6 +33,7 @@ static std::once_flag kinitJSClassIDFlag;
class WindowInstance;
class DocumentInstance;
class ExecutionContext;
class EventInstance;
struct DOMTimerCallbackContext;

std::string jsAtomToStdString(JSContext* ctx, JSAtom atom);
Expand Down Expand Up @@ -96,6 +97,7 @@ class ExecutionContext {
DOMTimerCoordinator* timers();

FORCE_INLINE DocumentInstance* document() { return m_document; };
FORCE_INLINE WindowInstance* window() { return m_window; }
FORCE_INLINE foundation::UICommandBuffer* uiCommandBuffer() { return &m_commandBuffer; };

void trace(JSRuntime* rt, JSValue val, JS_MarkFunc* mark_func);
Expand All @@ -119,6 +121,9 @@ class ExecutionContext {
static void dispatchGlobalErrorEvent(ExecutionContext* context, JSValueConst error);

void reportError(JSValueConst error);
void reportErrorEvent(EventInstance* errorEvent);
void dispatchErrorEvent(EventInstance* errorEvent);
void dispatchErrorEventInternal(EventInstance* errorEvent);

private:
static void promiseRejectTracker(JSContext* ctx, JSValueConst promise, JSValueConst reason, JS_BOOL is_handled, void* opaque);
Expand All @@ -129,6 +134,7 @@ class ExecutionContext {
JSValue globalObject{JS_NULL};
bool ctxInvalid_{false};
JSContext* m_ctx{nullptr};
bool m_inDispatchErrorEvent_{false};
friend WindowInstance;
friend DocumentInstance;
WindowInstance* m_window{nullptr};
Expand Down
39 changes: 36 additions & 3 deletions bridge/bindings/qjs/js_context_test.cc
Expand Up @@ -26,6 +26,20 @@ TEST(Context, evalWithError) {
EXPECT_EQ(errorHandlerExecuted, true);
}

TEST(Context, recursionThrowError) {
static bool errorHandlerExecuted = false;
auto errorHandler = [](int32_t contextId, const char* errmsg) { errorHandlerExecuted = true; };
auto bridge = TEST_init(errorHandler);
const char* code =
"addEventListener('error', (evt) => {\n"
" console.log('tagName', evt.target.tagName());\n"
"});\n"
"\n"
"throw Error('foo');";
bridge->evaluateScript(code, strlen(code), "file://", 0);
EXPECT_EQ(errorHandlerExecuted, true);
}

TEST(Context, unrejectPromiseError) {
static bool errorHandlerExecuted = false;
auto errorHandler = [](int32_t contextId, const char* errmsg) {
Expand All @@ -49,6 +63,25 @@ TEST(Context, unrejectPromiseError) {
EXPECT_EQ(errorHandlerExecuted, true);
}

TEST(Context, globalErrorHandlerTargetReturnToWindow) {
static bool logCalled = false;
auto errorHandler = [](int32_t contextId, const char* errmsg) {};
auto bridge = TEST_init(errorHandler);
kraken::KrakenPage::consoleMessageHandler = [](void* ctx, const std::string& message, int logLevel) {
logCalled = true;

EXPECT_STREQ(message.c_str(), "true");
};

std::string code = R"(
window.addEventListener('error', (e) => { console.log(e.target === window) });
throw new Error('1234');
)";
bridge->evaluateScript(code.c_str(), code.size(), "file://", 0);
EXPECT_EQ(logCalled, true);
kraken::KrakenPage::consoleMessageHandler = nullptr;
}

TEST(Context, unrejectPromiseWillTriggerUnhandledRejectionEvent) {
static bool errorHandlerExecuted = false;
static bool logCalled = false;
Expand All @@ -62,15 +95,15 @@ TEST(Context, unrejectPromiseWillTriggerUnhandledRejectionEvent) {
};
auto bridge = TEST_init(errorHandler);
static int logIndex = 0;
static std::string logs[] = {"error event cannot read property 'forceNullError' of null", "unhandled event {promise: Promise {...}, reason: Error {...}}"};
static std::string logs[] = {"error event cannot read property 'forceNullError' of null", "unhandled event {promise: Promise {...}, reason: Error {...}} true"};
kraken::KrakenPage::consoleMessageHandler = [](void* ctx, const std::string& message, int logLevel) {
logCalled = true;
EXPECT_STREQ(logs[logIndex++].c_str(), message.c_str());
};

std::string code = R"(
window.onunhandledrejection = (e) => {
console.log('unhandled event', e);
console.log('unhandled event', e, e.target === window);
};
window.onerror = (e) => {
console.log('error event', e);
Expand Down Expand Up @@ -222,7 +255,7 @@ TEST(Context, unrejectPromiseErrorWithMultipleContext) {
bridge->evaluateScript(code, strlen(code), "file://", 0);
bridge2->evaluateScript(code, strlen(code), "file://", 0);
EXPECT_EQ(errorHandlerExecuted, true);
EXPECT_EQ(errorCalledCount, 2);
EXPECT_EQ(errorCalledCount, 4);
}

TEST(Context, accessGetUICommandItemsAfterDisposed) {
Expand Down
6 changes: 3 additions & 3 deletions integration_tests/pubspec.yaml
Expand Up @@ -27,9 +27,9 @@ dependencies:
sdk: flutter
kraken: 0.8.0-dev.1
image: ^3.0.2
kraken_video_player: ^2.0.0-dev.0
kraken_websocket: ^2.0.0
kraken_webview: ^2.0.0-dev.0
kraken_video_player: 2.0.0
kraken_websocket: 2.0.0
kraken_webview: 2.0.0
waterfall_flow: ^3.0.1

dev_dependencies:
Expand Down