Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: src/JsValue.cpp

Issue 29812649: Issue 6526 - *ToV8String() return MaybeLocal<> and check Call() return value (Closed) Base URL: https://hg.adblockplus.org/libadblockplus/
Patch Set: Added tests. Renamed CHECKED_TO_LOCAL. Created July 11, 2018, 2:10 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/JsEngine.cpp ('k') | src/Utils.h » ('j') | src/Utils.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/JsValue.cpp
===================================================================
--- a/src/JsValue.cpp
+++ b/src/JsValue.cpp
@@ -178,46 +178,57 @@
AdblockPlus::JsValue AdblockPlus::JsValue::GetProperty(const std::string& name) const
{
if (!IsObject())
throw std::runtime_error("Attempting to get property of a non-object");
const JsContext context(*jsEngine);
- v8::Local<v8::String> property = Utils::ToV8String(jsEngine->GetIsolate(), name);
+ auto isolate = jsEngine->GetIsolate();
+ v8::Local<v8::String> property = CHECKED_TO_LOCAL(
+ isolate, Utils::ToV8String(isolate, name));
v8::Local<v8::Object> obj = v8::Local<v8::Object>::Cast(UnwrapValue());
- return JsValue(jsEngine, obj->Get(property));
+ return JsValue(jsEngine, CHECKED_TO_LOCAL(
+ isolate, obj->Get(isolate->GetCurrentContext(), property)));
}
void AdblockPlus::JsValue::SetProperty(const std::string& name, v8::Local<v8::Value> val)
{
if (!IsObject())
throw std::runtime_error("Attempting to set property on a non-object");
+ auto isolate = jsEngine->GetIsolate();
- v8::Local<v8::String> property = Utils::ToV8String(jsEngine->GetIsolate(), name);
+ v8::Local<v8::String> property = CHECKED_TO_LOCAL(
+ isolate, Utils::ToV8String(isolate, name));
v8::Local<v8::Object> obj = v8::Local<v8::Object>::Cast(UnwrapValue());
obj->Set(property, val);
}
v8::Local<v8::Value> AdblockPlus::JsValue::UnwrapValue() const
{
return v8::Local<v8::Value>::New(jsEngine->GetIsolate(), *value);
}
void AdblockPlus::JsValue::SetProperty(const std::string& name, const std::string& val)
{
const JsContext context(*jsEngine);
- SetProperty(name, Utils::ToV8String(jsEngine->GetIsolate(), val));
+ auto isolate = jsEngine->GetIsolate();
+
+ SetProperty(name, CHECKED_TO_LOCAL(
+ isolate, Utils::ToV8String(jsEngine->GetIsolate(), val)));
}
void AdblockPlus::JsValue::SetStringBufferProperty(const std::string& name, const StringBuffer& val)
{
const JsContext context(*jsEngine);
- SetProperty(name, Utils::StringBufferToV8String(jsEngine->GetIsolate(), val));
+ auto isolate = jsEngine->GetIsolate();
+
+ SetProperty(name, CHECKED_TO_LOCAL(
+ isolate, Utils::StringBufferToV8String(isolate, val)));
}
void AdblockPlus::JsValue::SetProperty(const std::string& name, int64_t val)
{
const JsContext context(*jsEngine);
SetProperty(name, v8::Number::New(jsEngine->GetIsolate(), val));
}
@@ -278,18 +289,18 @@
JsValue JsValue::Call(std::vector<v8::Local<v8::Value>>& args, v8::Local<v8::Object> thisObj) const
{
if (!IsFunction())
throw std::runtime_error("Attempting to call a non-function");
if (!thisObj->IsObject())
throw std::runtime_error("`this` pointer has to be an object");
const JsContext context(*jsEngine);
+ auto isolate = jsEngine->GetIsolate();
- const v8::TryCatch tryCatch(jsEngine->GetIsolate());
+ const v8::TryCatch tryCatch(isolate);
v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(UnwrapValue());
- v8::Local<v8::Value> result = func->Call(thisObj, args.size(),
- args.size() ? &args[0] : nullptr);
- if (tryCatch.HasCaught())
- throw JsError(jsEngine->GetIsolate(), tryCatch.Exception(), tryCatch.Message());
+ auto result = CHECKED_TO_LOCAL_WITH_TRY_CATCH(
+ isolate, func->Call(isolate->GetCurrentContext(),
+ thisObj, args.size(), args.size() ? &args[0] : nullptr), tryCatch);
return JsValue(jsEngine, result);
}
« no previous file with comments | « src/JsEngine.cpp ('k') | src/Utils.h » ('j') | src/Utils.h » ('J')

Powered by Google App Engine
This is Rietveld