Skip to content

Commit

Permalink
ScriptFunctionCall::call() can return an empty JSValue if the watchdo…
Browse files Browse the repository at this point in the history
…g timer fires, callers should check for this

https://bugs.webkit.org/show_bug.cgi?id=165875

Reviewed by Devin Rousso.

ScriptFunctionCall::call() may return empty JSValue from several places,
the callers now check for emptiness first before accessing the value.

Unfortunately, I don't have a reliable repro which could be converted
to a layout test like the one in 11d211b
even though the symptoms are similar.

* Source/JavaScriptCore/inspector/InjectedScript.cpp:
(Inspector::InjectedScript::wrapObject const):
(Inspector::InjectedScript::wrapJSONString const):
(Inspector::InjectedScript::wrapTable const):
(Inspector::InjectedScript::previewValue const):
(Inspector::InjectedScript::createCommandLineAPIObject const):

Canonical link: https://commits.webkit.org/270739@main
  • Loading branch information
yury-s committed Nov 15, 2023
1 parent 6695e0a commit a4eed62
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 10 deletions.
12 changes: 6 additions & 6 deletions Source/JavaScriptCore/inspector/InjectedScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ RefPtr<Protocol::Runtime::RemoteObject> InjectedScript::wrapObject(JSC::JSValue
wrapFunction.appendArgument(generatePreview);

auto callResult = callFunctionWithEvalEnabled(wrapFunction);
if (!callResult)
if (!callResult || !callResult.value())
return nullptr;

auto resultValue = toInspectorValue(globalObject(), callResult.value());
Expand All @@ -309,7 +309,7 @@ RefPtr<Protocol::Runtime::RemoteObject> InjectedScript::wrapJSONString(const Str
wrapFunction.appendArgument(generatePreview);

auto callResult = callFunctionWithEvalEnabled(wrapFunction);
if (!callResult)
if (!callResult || !callResult.value())
return nullptr;

if (callResult.value().isNull())
Expand Down Expand Up @@ -338,7 +338,7 @@ RefPtr<Protocol::Runtime::RemoteObject> InjectedScript::wrapTable(JSC::JSValue t
wrapFunction.appendArgument(columns);

auto callResult = callFunctionWithEvalEnabled(wrapFunction);
if (!callResult)
if (!callResult || !callResult.value())
return nullptr;

auto resultValue = toInspectorValue(globalObject(), callResult.value());
Expand All @@ -359,7 +359,7 @@ RefPtr<Protocol::Runtime::ObjectPreview> InjectedScript::previewValue(JSC::JSVal
wrapFunction.appendArgument(value);

auto callResult = callFunctionWithEvalEnabled(wrapFunction);
if (!callResult)
if (!callResult || !callResult.value())
return nullptr;

auto resultValue = toInspectorValue(globalObject(), callResult.value());
Expand Down Expand Up @@ -411,7 +411,7 @@ JSC::JSValue InjectedScript::findObjectById(const String& objectId) const

auto callResult = callFunctionWithEvalEnabled(function);
ASSERT(callResult);
if (!callResult)
if (!callResult || !callResult.value())
return { };
return callResult.value();
}
Expand Down Expand Up @@ -449,7 +449,7 @@ JSC::JSObject* InjectedScript::createCommandLineAPIObject(JSC::JSValue callFrame

auto callResult = callFunctionWithEvalEnabled(function);
ASSERT(callResult);
return callResult ? asObject(callResult.value()) : nullptr;
return callResult && callResult.value() ? asObject(callResult.value()) : nullptr;
}

JSC::JSValue InjectedScript::arrayFromVector(Vector<JSC::JSValue>&& vector)
Expand Down
6 changes: 2 additions & 4 deletions Source/JavaScriptCore/inspector/InjectedScriptBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,8 @@ void InjectedScriptBase::makeAsyncCall(ScriptFunctionCall& function, AsyncCallCa
function.appendArgument(JSC::JSValue(jsFunction));

auto result = callFunctionWithEvalEnabled(function);
ASSERT_UNUSED(result, result.value().isUndefined());

ASSERT(result);
if (!result) {
ASSERT_UNUSED(result, result && result.value() && result.value().isUndefined());
if (!result || !result.value()) {
// Since `callback` is moved above, we can't call it if there's an exception while trying to
// execute the `JSNativeStdFunction` inside InjectedScriptSource.js.
jsFunction->function()(globalObject, nullptr);
Expand Down
4 changes: 4 additions & 0 deletions Source/JavaScriptCore/inspector/InjectedScriptModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ void InjectedScriptModule::ensureInjected(InjectedScriptManager* injectedScriptM
WTFLogAlways("Error when calling 'hasInjectedModule' for '%s': %s (%d:%d)\n", name().utf8().data(), error->value().toWTFString(injectedScript.globalObject()).utf8().data(), line, column);
RELEASE_ASSERT_NOT_REACHED();
}
if (!hasInjectedModuleResult.value()) {
WTFLogAlways("VM is terminated when calling 'injectModule' for '%s'\n", name().utf8().data());
RELEASE_ASSERT_NOT_REACHED();
}
if (!hasInjectedModuleResult.value().isBoolean() || !hasInjectedModuleResult.value().asBoolean()) {
ScriptFunctionCall function(injectedScript.globalObject(), injectedScript.injectedScriptObject(), "injectModule"_s, injectedScriptManager->inspectorEnvironment().functionCallHandler());
function.appendArgument(name());
Expand Down

0 comments on commit a4eed62

Please sign in to comment.