Skip to content

Commit fe59853

Browse files
matiwinnetouCommit Bot
matiwinnetou
authored and
Commit Bot
committed
Pass Isolate pointer to String::Utf8Value/Value constructors
As part of J2V8 development (https://github.com/eclipsesource/J2V8), we realized that we had a subtle bug in how Isolate scope was created and it's lifetime managed, see: eclipsesource/J2V8#313. Mentioned above bug was fixed, however, what we also noticed is that V8 API has been constantly and slowly moving to such an API, in which one has to pass Isolate explicitly to methods and/or constructors. We found two more places that might have been overlooked. This contribution adds passing of Isolate pointer explicitly to constructors of String::Utf8Value and String::Value classes. Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.v8:v8_linux_noi18n_rel_ng Change-Id: I61984285f152aba5ca922100cf3df913a9cb2cea Reviewed-on: https://chromium-review.googlesource.com/593309 Commit-Queue: Adam Klein <adamk@chromium.org> Reviewed-by: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#47656}
1 parent 0006670 commit fe59853

36 files changed

Lines changed: 656 additions & 514 deletions

‎AUTHORS‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ Luis Reis <luis.m.reis@gmail.com>
9090
Luke Zarko <lukezarko@gmail.com>
9191
Maciej Małecki <me@mmalecki.com>
9292
Marcin Cieślak <saper@marcincieslak.com>
93+
Mateusz Czeladka <mateusz.szczap@gmail.com>
9394
Mathias Bynens <mathias@qiwi.be>
9495
Matt Hanselman <mjhanselman@gmail.com>
9596
Matthew Sporleder <msporleder@gmail.com>
@@ -133,4 +134,4 @@ Wiktor Garbacz <wiktor.garbacz@gmail.com>
133134
Yu Yin <xwafish@gmail.com>
134135
Zac Hansen <xaxxon@gmail.com>
135136
Zhongping Wang <kewpie.w.zp@gmail.com>
136-
柳荣一 <admin@web-tinker.com>
137+
柳荣一 <admin@web-tinker.com>

‎include/v8.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2768,6 +2768,7 @@ class V8_EXPORT String : public Name {
27682768
class V8_EXPORT Utf8Value {
27692769
public:
27702770
explicit Utf8Value(Local<v8::Value> obj);
2771+
Utf8Value(Isolate* isolate, Local<v8::Value> obj);
27712772
~Utf8Value();
27722773
char* operator*() { return str_; }
27732774
const char* operator*() const { return str_; }
@@ -2791,6 +2792,7 @@ class V8_EXPORT String : public Name {
27912792
class V8_EXPORT Value {
27922793
public:
27932794
explicit Value(Local<v8::Value> obj);
2795+
Value(Isolate* isolate, Local<v8::Value> obj);
27942796
~Value();
27952797
uint16_t* operator*() { return str_; }
27962798
const uint16_t* operator*() const { return str_; }

‎samples/hello-world.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ int main(int argc, char* argv[]) {
4848
Local<Value> result = script->Run(context).ToLocalChecked();
4949

5050
// Convert the result to an UTF8 string and print it.
51-
String::Utf8Value utf8(result);
51+
String::Utf8Value utf8(isolate, result);
5252
printf("%s\n", *utf8);
5353
}
5454

‎samples/process.cc‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,10 @@ class JsHttpRequestProcessor : public HttpRequestProcessor {
144144

145145
static void LogCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
146146
if (args.Length() < 1) return;
147-
HandleScope scope(args.GetIsolate());
147+
Isolate* isolate = args.GetIsolate();
148+
HandleScope scope(isolate);
148149
Local<Value> arg = args[0];
149-
String::Utf8Value value(arg);
150+
String::Utf8Value value(isolate, arg);
150151
HttpRequestProcessor::Log(*value);
151152
}
152153

@@ -221,7 +222,7 @@ bool JsHttpRequestProcessor::ExecuteScript(Local<String> script) {
221222
// Compile the script and check for errors.
222223
Local<Script> compiled_script;
223224
if (!Script::Compile(context, script).ToLocal(&compiled_script)) {
224-
String::Utf8Value error(try_catch.Exception());
225+
String::Utf8Value error(GetIsolate(), try_catch.Exception());
225226
Log(*error);
226227
// The script failed to compile; bail out.
227228
return false;
@@ -231,11 +232,12 @@ bool JsHttpRequestProcessor::ExecuteScript(Local<String> script) {
231232
Local<Value> result;
232233
if (!compiled_script->Run(context).ToLocal(&result)) {
233234
// The TryCatch above is still in effect and will have caught the error.
234-
String::Utf8Value error(try_catch.Exception());
235+
String::Utf8Value error(GetIsolate(), try_catch.Exception());
235236
Log(*error);
236237
// Running the script failed; bail out.
237238
return false;
238239
}
240+
239241
return true;
240242
}
241243

@@ -295,12 +297,11 @@ bool JsHttpRequestProcessor::Process(HttpRequest* request) {
295297
v8::Local<v8::Function>::New(GetIsolate(), process_);
296298
Local<Value> result;
297299
if (!process->Call(context, context->Global(), argc, argv).ToLocal(&result)) {
298-
String::Utf8Value error(try_catch.Exception());
300+
String::Utf8Value error(GetIsolate(), try_catch.Exception());
299301
Log(*error);
300302
return false;
301-
} else {
302-
return true;
303303
}
304+
return true;
304305
}
305306

306307

@@ -366,8 +367,8 @@ map<string, string>* JsHttpRequestProcessor::UnwrapMap(Local<Object> obj) {
366367

367368
// Convert a JavaScript string to a std::string. To not bother too
368369
// much with string encodings we just use ascii.
369-
string ObjectToString(Local<Value> value) {
370-
String::Utf8Value utf8_value(value);
370+
string ObjectToString(v8::Isolate* isolate, Local<Value> value) {
371+
String::Utf8Value utf8_value(isolate, value);
371372
return string(*utf8_value);
372373
}
373374

@@ -380,7 +381,7 @@ void JsHttpRequestProcessor::MapGet(Local<Name> name,
380381
map<string, string>* obj = UnwrapMap(info.Holder());
381382

382383
// Convert the JavaScript string to a std::string.
383-
string key = ObjectToString(Local<String>::Cast(name));
384+
string key = ObjectToString(info.GetIsolate(), Local<String>::Cast(name));
384385

385386
// Look up the value if it exists using the standard STL ideom.
386387
map<string, string>::iterator iter = obj->find(key);
@@ -405,8 +406,8 @@ void JsHttpRequestProcessor::MapSet(Local<Name> name, Local<Value> value_obj,
405406
map<string, string>* obj = UnwrapMap(info.Holder());
406407

407408
// Convert the key and value to std::strings.
408-
string key = ObjectToString(Local<String>::Cast(name));
409-
string value = ObjectToString(value_obj);
409+
string key = ObjectToString(info.GetIsolate(), Local<String>::Cast(name));
410+
string value = ObjectToString(info.GetIsolate(), value_obj);
410411

411412
// Update the map.
412413
(*obj)[key] = value;

‎samples/shell.cc‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ void Print(const v8::FunctionCallbackInfo<v8::Value>& args) {
147147
} else {
148148
printf(" ");
149149
}
150-
v8::String::Utf8Value str(args[i]);
150+
v8::String::Utf8Value str(args.GetIsolate(), args[i]);
151151
const char* cstr = ToCString(str);
152152
printf("%s", cstr);
153153
}
@@ -166,7 +166,7 @@ void Read(const v8::FunctionCallbackInfo<v8::Value>& args) {
166166
v8::NewStringType::kNormal).ToLocalChecked());
167167
return;
168168
}
169-
v8::String::Utf8Value file(args[0]);
169+
v8::String::Utf8Value file(args.GetIsolate(), args[0]);
170170
if (*file == NULL) {
171171
args.GetIsolate()->ThrowException(
172172
v8::String::NewFromUtf8(args.GetIsolate(), "Error loading file",
@@ -180,17 +180,17 @@ void Read(const v8::FunctionCallbackInfo<v8::Value>& args) {
180180
v8::NewStringType::kNormal).ToLocalChecked());
181181
return;
182182
}
183+
183184
args.GetReturnValue().Set(source);
184185
}
185186

186-
187187
// The callback that is invoked by v8 whenever the JavaScript 'load'
188188
// function is called. Loads, compiles and executes its argument
189189
// JavaScript file.
190190
void Load(const v8::FunctionCallbackInfo<v8::Value>& args) {
191191
for (int i = 0; i < args.Length(); i++) {
192192
v8::HandleScope handle_scope(args.GetIsolate());
193-
v8::String::Utf8Value file(args[i]);
193+
v8::String::Utf8Value file(args.GetIsolate(), args[i]);
194194
if (*file == NULL) {
195195
args.GetIsolate()->ThrowException(
196196
v8::String::NewFromUtf8(args.GetIsolate(), "Error loading file",
@@ -361,7 +361,7 @@ bool ExecuteString(v8::Isolate* isolate, v8::Local<v8::String> source,
361361
if (print_result && !result->IsUndefined()) {
362362
// If all went well and the result wasn't undefined then print
363363
// the returned value.
364-
v8::String::Utf8Value str(result);
364+
v8::String::Utf8Value str(isolate, result);
365365
const char* cstr = ToCString(str);
366366
printf("%s\n", cstr);
367367
}
@@ -373,7 +373,7 @@ bool ExecuteString(v8::Isolate* isolate, v8::Local<v8::String> source,
373373

374374
void ReportException(v8::Isolate* isolate, v8::TryCatch* try_catch) {
375375
v8::HandleScope handle_scope(isolate);
376-
v8::String::Utf8Value exception(try_catch->Exception());
376+
v8::String::Utf8Value exception(isolate, try_catch->Exception());
377377
const char* exception_string = ToCString(exception);
378378
v8::Local<v8::Message> message = try_catch->Message();
379379
if (message.IsEmpty()) {
@@ -382,14 +382,15 @@ void ReportException(v8::Isolate* isolate, v8::TryCatch* try_catch) {
382382
fprintf(stderr, "%s\n", exception_string);
383383
} else {
384384
// Print (filename):(line number): (message).
385-
v8::String::Utf8Value filename(message->GetScriptOrigin().ResourceName());
385+
v8::String::Utf8Value filename(isolate,
386+
message->GetScriptOrigin().ResourceName());
386387
v8::Local<v8::Context> context(isolate->GetCurrentContext());
387388
const char* filename_string = ToCString(filename);
388389
int linenum = message->GetLineNumber(context).FromJust();
389390
fprintf(stderr, "%s:%i: %s\n", filename_string, linenum, exception_string);
390391
// Print line of source code.
391392
v8::String::Utf8Value sourceline(
392-
message->GetSourceLine(context).ToLocalChecked());
393+
isolate, message->GetSourceLine(context).ToLocalChecked());
393394
const char* sourceline_string = ToCString(sourceline);
394395
fprintf(stderr, "%s\n", sourceline_string);
395396
// Print wavy underline (GetUnderline is deprecated).
@@ -406,7 +407,7 @@ void ReportException(v8::Isolate* isolate, v8::TryCatch* try_catch) {
406407
if (try_catch->StackTrace(context).ToLocal(&stack_trace_string) &&
407408
stack_trace_string->IsString() &&
408409
v8::Local<v8::String>::Cast(stack_trace_string)->Length() > 0) {
409-
v8::String::Utf8Value stack_trace(stack_trace_string);
410+
v8::String::Utf8Value stack_trace(isolate, stack_trace_string);
410411
const char* stack_trace_string = ToCString(stack_trace);
411412
fprintf(stderr, "%s\n", stack_trace_string);
412413
}

‎src/api.cc‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9181,44 +9181,46 @@ bool MicrotasksScope::IsRunningMicrotasks(Isolate* v8Isolate) {
91819181
return isolate->IsRunningMicrotasks();
91829182
}
91839183

9184-
String::Utf8Value::Utf8Value(v8::Local<v8::Value> obj)
9184+
String::Utf8Value::Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> obj)
91859185
: str_(NULL), length_(0) {
91869186
if (obj.IsEmpty()) return;
9187-
i::Isolate* isolate = i::Isolate::Current();
9188-
Isolate* v8_isolate = reinterpret_cast<Isolate*>(isolate);
9189-
ENTER_V8_DO_NOT_USE(isolate);
9190-
i::HandleScope scope(isolate);
9191-
Local<Context> context = v8_isolate->GetCurrentContext();
9192-
TryCatch try_catch(v8_isolate);
9187+
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
9188+
ENTER_V8_DO_NOT_USE(i_isolate);
9189+
i::HandleScope scope(i_isolate);
9190+
Local<Context> context = isolate->GetCurrentContext();
9191+
TryCatch try_catch(isolate);
91939192
Local<String> str;
91949193
if (!obj->ToString(context).ToLocal(&str)) return;
91959194
i::Handle<i::String> i_str = Utils::OpenHandle(*str);
9196-
length_ = v8::Utf8Length(*i_str, isolate);
9195+
length_ = v8::Utf8Length(*i_str, i_isolate);
91979196
str_ = i::NewArray<char>(length_ + 1);
91989197
str->WriteUtf8(str_);
91999198
}
92009199

9200+
String::Utf8Value::Utf8Value(v8::Local<v8::Value> obj)
9201+
: String::Utf8Value::Utf8Value(Isolate::GetCurrent(), obj) {}
92019202

92029203
String::Utf8Value::~Utf8Value() {
92039204
i::DeleteArray(str_);
92049205
}
92059206

9206-
9207-
String::Value::Value(v8::Local<v8::Value> obj) : str_(NULL), length_(0) {
9207+
String::Value::Value(v8::Isolate* isolate, v8::Local<v8::Value> obj)
9208+
: str_(NULL), length_(0) {
92089209
if (obj.IsEmpty()) return;
9209-
i::Isolate* isolate = i::Isolate::Current();
9210-
Isolate* v8_isolate = reinterpret_cast<Isolate*>(isolate);
9211-
ENTER_V8_DO_NOT_USE(isolate);
9212-
i::HandleScope scope(isolate);
9213-
Local<Context> context = v8_isolate->GetCurrentContext();
9214-
TryCatch try_catch(v8_isolate);
9210+
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
9211+
ENTER_V8_DO_NOT_USE(i_isolate);
9212+
i::HandleScope scope(i_isolate);
9213+
Local<Context> context = isolate->GetCurrentContext();
9214+
TryCatch try_catch(isolate);
92159215
Local<String> str;
92169216
if (!obj->ToString(context).ToLocal(&str)) return;
92179217
length_ = str->Length();
92189218
str_ = i::NewArray<uint16_t>(length_ + 1);
92199219
str->Write(str_);
92209220
}
92219221

9222+
String::Value::Value(v8::Local<v8::Value> obj)
9223+
: String::Value::Value(v8::Isolate::GetCurrent(), obj) {}
92229224

92239225
String::Value::~Value() {
92249226
i::DeleteArray(str_);

‎src/d8-console.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ void WriteToFile(FILE* file, Isolate* isolate,
2525
return;
2626
}
2727

28-
v8::String::Utf8Value str(str_obj);
28+
v8::String::Utf8Value str(isolate, str_obj);
2929
int n = static_cast<int>(fwrite(*str, sizeof(**str), str.length(), file));
3030
if (n != str.length()) {
3131
printf("Error in fwrite\n");
@@ -77,7 +77,7 @@ void D8Console::Time(const debug::ConsoleCallArguments& args,
7777
Shell::ReportException(isolate_, &try_catch);
7878
return;
7979
}
80-
v8::String::Utf8Value utf8(label);
80+
v8::String::Utf8Value utf8(isolate_, label);
8181
std::string string(*utf8);
8282
auto find = timers_.find(string);
8383
if (find != timers_.end()) {
@@ -104,7 +104,7 @@ void D8Console::TimeEnd(const debug::ConsoleCallArguments& args,
104104
Shell::ReportException(isolate_, &try_catch);
105105
return;
106106
}
107-
v8::String::Utf8Value utf8(label);
107+
v8::String::Utf8Value utf8(isolate_, label);
108108
std::string string(*utf8);
109109
auto find = timers_.find(string);
110110
if (find != timers_.end()) {

‎src/d8-posix.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class ExecArgs {
165165
exec_args_[0] = NULL;
166166
}
167167
bool Init(Isolate* isolate, Local<Value> arg0, Local<Array> command_args) {
168-
String::Utf8Value prog(arg0);
168+
String::Utf8Value prog(isolate, arg0);
169169
if (*prog == NULL) {
170170
const char* message =
171171
"os.system(): String conversion of program name failed";
@@ -183,7 +183,7 @@ class ExecArgs {
183183
Local<Value> arg(
184184
command_args->Get(isolate->GetCurrentContext(),
185185
Integer::New(isolate, j)).ToLocalChecked());
186-
String::Utf8Value utf8_arg(arg);
186+
String::Utf8Value utf8_arg(isolate, arg);
187187
if (*utf8_arg == NULL) {
188188
exec_args_[i] = NULL; // Consistent state for destructor.
189189
const char* message =
@@ -553,7 +553,7 @@ void Shell::ChangeDirectory(const v8::FunctionCallbackInfo<v8::Value>& args) {
553553
.ToLocalChecked());
554554
return;
555555
}
556-
String::Utf8Value directory(args[0]);
556+
String::Utf8Value directory(args.GetIsolate(), args[0]);
557557
if (*directory == NULL) {
558558
const char* message = "os.chdir(): String conversion of argument failed.";
559559
args.GetIsolate()->ThrowException(
@@ -667,7 +667,7 @@ void Shell::MakeDirectory(const v8::FunctionCallbackInfo<v8::Value>& args) {
667667
.ToLocalChecked());
668668
return;
669669
}
670-
String::Utf8Value directory(args[0]);
670+
String::Utf8Value directory(args.GetIsolate(), args[0]);
671671
if (*directory == NULL) {
672672
const char* message = "os.mkdirp(): String conversion of argument failed.";
673673
args.GetIsolate()->ThrowException(
@@ -687,7 +687,7 @@ void Shell::RemoveDirectory(const v8::FunctionCallbackInfo<v8::Value>& args) {
687687
.ToLocalChecked());
688688
return;
689689
}
690-
String::Utf8Value directory(args[0]);
690+
String::Utf8Value directory(args.GetIsolate(), args[0]);
691691
if (*directory == NULL) {
692692
const char* message = "os.rmdir(): String conversion of argument failed.";
693693
args.GetIsolate()->ThrowException(
@@ -707,8 +707,8 @@ void Shell::SetEnvironment(const v8::FunctionCallbackInfo<v8::Value>& args) {
707707
.ToLocalChecked());
708708
return;
709709
}
710-
String::Utf8Value var(args[0]);
711-
String::Utf8Value value(args[1]);
710+
String::Utf8Value var(args.GetIsolate(), args[0]);
711+
String::Utf8Value value(args.GetIsolate(), args[1]);
712712
if (*var == NULL) {
713713
const char* message =
714714
"os.setenv(): String conversion of variable name failed.";
@@ -737,7 +737,7 @@ void Shell::UnsetEnvironment(const v8::FunctionCallbackInfo<v8::Value>& args) {
737737
.ToLocalChecked());
738738
return;
739739
}
740-
String::Utf8Value var(args[0]);
740+
String::Utf8Value var(args.GetIsolate(), args[0]);
741741
if (*var == NULL) {
742742
const char* message =
743743
"os.setenv(): String conversion of variable name failed.";

0 commit comments

Comments
 (0)