Skip to content

GitLab

T
tangled
Commit cc7538d0 authored by Dan Elphick's avatar Dan Elphick Committed by Commit Bot
Browse files
Options

[cleanup] Fix uses of deprecated v8::Object::Set/Get 

Replace uses of Get/Set with Context versions that return a
Maybe/MaybeLocal and check their return value mostly using
ToLocalChecked or Check.

Bug: v8:7283, v8:8562
Change-Id: Id8614f6e599cad628f3de8614d5a600d2148eb76
Reviewed-on: https://chromium-review.googlesource.com/c/1449676
Commit-Queue: Dan Elphick <delphick@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Dan Elphick <delphick@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628313}
parent 74c7050a
Hide whitespace changes
Inline Side-by-side
Showing with 170 additions and 130 deletions
content/renderer/pepper/v8_var_converter_unittest.cc
View file @ cc7538d0
...@@ -120,7 +120,8 @@ bool Equals(const PP_Var& var, ...@@ -120,7 +120,8 @@ bool Equals(const PP_Var& var,
if (v8_array->Length() != array_var->elements().size()) if (v8_array->Length() != array_var->elements().size())
return false; return false;
for (uint32_t i = 0; i < v8_array->Length(); ++i) { for (uint32_t i = 0; i < v8_array->Length(); ++i) {
v8::Local<v8::Value> child_v8 = v8_array->Get(i); v8::Local<v8::Value> child_v8 =
v8_array->Get(context, i).ToLocalChecked();
if (!Equals(array_var->elements()[i].get(), child_v8, visited_ids)) if (!Equals(array_var->elements()[i].get(), child_v8, visited_ids))
return false; return false;
} }
...@@ -394,7 +395,7 @@ TEST_F(V8VarConverterTest, Cycles) { ...@@ -394,7 +395,7 @@ TEST_F(V8VarConverterTest, Cycles) {
ASSERT_FALSE(FromV8ValueSync(object, context, &var_result)); ASSERT_FALSE(FromV8ValueSync(object, context, &var_result));
// Array with self reference. // Array with self reference.
array->Set(0, array); array->Set(context, 0, array).Check();
ASSERT_FALSE(FromV8ValueSync(array, context, &var_result)); ASSERT_FALSE(FromV8ValueSync(array, context, &var_result));
} }
} }
......
content/renderer/v8_value_converter_impl.cc
View file @ cc7538d0
...@@ -470,7 +470,7 @@ std::unique_ptr<base::Value> V8ValueConverterImpl::FromV8Array( ...@@ -470,7 +470,7 @@ std::unique_ptr<base::Value> V8ValueConverterImpl::FromV8Array(
// Only fields with integer keys are carried over to the ListValue. // Only fields with integer keys are carried over to the ListValue.
for (uint32_t i = 0; i < val->Length(); ++i) { for (uint32_t i = 0; i < val->Length(); ++i) {
v8::TryCatch try_catch(isolate); v8::TryCatch try_catch(isolate);
v8::Local<v8::Value> child_v8 = val->Get(i); v8::Local<v8::Value> child_v8;
v8::MaybeLocal<v8::Value> maybe_child = v8::MaybeLocal<v8::Value> maybe_child =
val->Get(isolate->GetCurrentContext(), i); val->Get(isolate->GetCurrentContext(), i);
if (try_catch.HasCaught() || !maybe_child.ToLocal(&child_v8)) { if (try_catch.HasCaught() || !maybe_child.ToLocal(&child_v8)) {
......
content/renderer/v8_value_converter_impl_unittest.cc
View file @ cc7538d0
...@@ -75,17 +75,17 @@ class V8ValueConverterImplTest : public testing::Test { ...@@ -75,17 +75,17 @@ class V8ValueConverterImplTest : public testing::Test {
} }
std::string GetString(v8::Local<v8::Object> value, const std::string& key) { std::string GetString(v8::Local<v8::Object> value, const std::string& key) {
v8::Local<v8::String> temp = v8::Local<v8::Value> temp;
value if (!value
->Get(v8::String::NewFromUtf8(isolate_, key.c_str(), ->Get(isolate_->GetCurrentContext(),
v8::NewStringType::kInternalized) v8::String::NewFromUtf8(isolate_, key.c_str(),
.ToLocalChecked()) v8::NewStringType::kInternalized)
.As<v8::String>(); .ToLocalChecked())
if (temp.IsEmpty()) { .ToLocal(&temp)) {
ADD_FAILURE(); ADD_FAILURE();
return std::string(); return std::string();
} }
v8::String::Utf8Value utf8(isolate_, temp); v8::String::Utf8Value utf8(isolate_, temp.As<v8::String>());
return std::string(*utf8, utf8.length()); return std::string(*utf8, utf8.length());
} }
...@@ -99,36 +99,36 @@ class V8ValueConverterImplTest : public testing::Test { ...@@ -99,36 +99,36 @@ class V8ValueConverterImplTest : public testing::Test {
} }
std::string GetString(v8::Local<v8::Array> value, uint32_t index) { std::string GetString(v8::Local<v8::Array> value, uint32_t index) {
v8::Local<v8::String> temp = value->Get(index).As<v8::String>(); v8::Local<v8::Value> temp;
if (temp.IsEmpty()) { if (!value->Get(isolate_->GetCurrentContext(), index).ToLocal(&temp)) {
ADD_FAILURE(); ADD_FAILURE();
return std::string(); return std::string();
} }
v8::String::Utf8Value utf8(isolate_, temp); v8::String::Utf8Value utf8(isolate_, temp.As<v8::String>());
return std::string(*utf8, utf8.length()); return std::string(*utf8, utf8.length());
} }
int32_t GetInt(v8::Local<v8::Object> value, const std::string& key) { int32_t GetInt(v8::Local<v8::Object> value, const std::string& key) {
v8::Local<v8::Int32> temp = v8::Local<v8::Value> temp;
value if (!value
->Get(v8::String::NewFromUtf8(isolate_, key.c_str(), ->Get(isolate_->GetCurrentContext(),
v8::NewStringType::kInternalized) v8::String::NewFromUtf8(isolate_, key.c_str(),
.ToLocalChecked()) v8::NewStringType::kInternalized)
.As<v8::Int32>(); .ToLocalChecked())
if (temp.IsEmpty()) { .ToLocal(&temp)) {
ADD_FAILURE(); ADD_FAILURE();
return -1; return -1;
} }
return temp->Value(); return temp.As<v8::Int32>()->Value();
} }
int32_t GetInt(v8::Local<v8::Object> value, uint32_t index) { int32_t GetInt(v8::Local<v8::Object> value, uint32_t index) {
v8::Local<v8::Int32> temp = value->Get(index).As<v8::Int32>(); v8::Local<v8::Value> temp;
if (temp.IsEmpty()) { if (!value->Get(isolate_->GetCurrentContext(), index).ToLocal(&temp)) {
ADD_FAILURE(); ADD_FAILURE();
return -1; return -1;
} }
return temp->Value(); return temp.As<v8::Int32>()->Value();
} }
bool IsNull(base::DictionaryValue* value, const std::string& key) { bool IsNull(base::DictionaryValue* value, const std::string& key) {
...@@ -141,11 +141,13 @@ class V8ValueConverterImplTest : public testing::Test { ...@@ -141,11 +141,13 @@ class V8ValueConverterImplTest : public testing::Test {
} }
bool IsNull(v8::Local<v8::Object> value, const std::string& key) { bool IsNull(v8::Local<v8::Object> value, const std::string& key) {
v8::Local<v8::Value> child = v8::Local<v8::Value> child;
value->Get(v8::String::NewFromUtf8(isolate_, key.c_str(), if (!value
->Get(isolate_->GetCurrentContext(),
v8::String::NewFromUtf8(isolate_, key.c_str(),
v8::NewStringType::kInternalized) v8::NewStringType::kInternalized)
.ToLocalChecked()); .ToLocalChecked())
if (child.IsEmpty()) { .ToLocal(&child)) {
ADD_FAILURE(); ADD_FAILURE();
return false; return false;
} }
...@@ -162,8 +164,8 @@ class V8ValueConverterImplTest : public testing::Test { ...@@ -162,8 +164,8 @@ class V8ValueConverterImplTest : public testing::Test {
} }
bool IsNull(v8::Local<v8::Array> value, uint32_t index) { bool IsNull(v8::Local<v8::Array> value, uint32_t index) {
v8::Local<v8::Value> child = value->Get(index); v8::Local<v8::Value> child;
if (child.IsEmpty()) { if (!value->Get(isolate_->GetCurrentContext(), index).ToLocal(&child)) {
ADD_FAILURE(); ADD_FAILURE();
return false; return false;
} }
...@@ -187,10 +189,13 @@ class V8ValueConverterImplTest : public testing::Test { ...@@ -187,10 +189,13 @@ class V8ValueConverterImplTest : public testing::Test {
} }
v8::Local<v8::Object> object(v8::Object::New(isolate_)); v8::Local<v8::Object> object(v8::Object::New(isolate_));
object->Set(v8::String::NewFromUtf8(isolate_, "test", object
v8::NewStringType::kInternalized) ->Set(context,
.ToLocalChecked(), v8::String::NewFromUtf8(isolate_, "test",
val); v8::NewStringType::kInternalized)
.ToLocalChecked(),
val)
.Check();
std::unique_ptr<base::DictionaryValue> dictionary( std::unique_ptr<base::DictionaryValue> dictionary(
base::DictionaryValue::From(converter.FromV8Value(object, context))); base::DictionaryValue::From(converter.FromV8Value(object, context)));
ASSERT_TRUE(dictionary.get()); ASSERT_TRUE(dictionary.get());
...@@ -205,7 +210,7 @@ class V8ValueConverterImplTest : public testing::Test { ...@@ -205,7 +210,7 @@ class V8ValueConverterImplTest : public testing::Test {
} }
v8::Local<v8::Array> array(v8::Array::New(isolate_)); v8::Local<v8::Array> array(v8::Array::New(isolate_));
array->Set(0, val); array->Set(context, 0, val).Check();
std::unique_ptr<base::ListValue> list( std::unique_ptr<base::ListValue> list(
base::ListValue::From(converter.FromV8Value(array, context))); base::ListValue::From(converter.FromV8Value(array, context)));
ASSERT_TRUE(list.get()); ASSERT_TRUE(list.get());
...@@ -277,85 +282,104 @@ TEST_F(V8ValueConverterImplTest, BasicRoundTrip) { ...@@ -277,85 +282,104 @@ TEST_F(V8ValueConverterImplTest, BasicRoundTrip) {
EXPECT_EQ(static_cast<const base::DictionaryValue&>(*original_root).size(), EXPECT_EQ(static_cast<const base::DictionaryValue&>(*original_root).size(),
v8_object->GetPropertyNames(context).ToLocalChecked()->Length()); v8_object->GetPropertyNames(context).ToLocalChecked()->Length());
EXPECT_TRUE(v8_object
->Get(v8::String::NewFromUtf8(
isolate_, "null", v8::NewStringType::kInternalized)
.ToLocalChecked())
->IsNull());
EXPECT_TRUE(v8_object
->Get(v8::String::NewFromUtf8(
isolate_, "true", v8::NewStringType::kInternalized)
.ToLocalChecked())
->IsTrue());
EXPECT_TRUE(v8_object
->Get(v8::String::NewFromUtf8(
isolate_, "false", v8::NewStringType::kInternalized)
.ToLocalChecked())
->IsFalse());
EXPECT_TRUE( EXPECT_TRUE(
v8_object v8_object
->Get(v8::String::NewFromUtf8(isolate_, "positive-int", ->Get(context, v8::String::NewFromUtf8(
v8::NewStringType::kInternalized) isolate_, "null", v8::NewStringType::kInternalized)
.ToLocalChecked()) .ToLocalChecked())
->IsInt32()); .ToLocalChecked()
->IsNull());
EXPECT_TRUE( EXPECT_TRUE(
v8_object v8_object
->Get(v8::String::NewFromUtf8(isolate_, "negative-int", ->Get(context, v8::String::NewFromUtf8(
v8::NewStringType::kInternalized) isolate_, "true", v8::NewStringType::kInternalized)
.ToLocalChecked()) .ToLocalChecked())
->IsInt32()); .ToLocalChecked()
->IsTrue());
EXPECT_TRUE(v8_object EXPECT_TRUE(v8_object
->Get(v8::String::NewFromUtf8( ->Get(context,
isolate_, "zero", v8::NewStringType::kInternalized) v8::String::NewFromUtf8(
isolate_, "false", v8::NewStringType::kInternalized)
.ToLocalChecked()) .ToLocalChecked())
.ToLocalChecked()
->IsFalse());
EXPECT_TRUE(v8_object
->Get(context, v8::String::NewFromUtf8(
isolate_, "positive-int",
v8::NewStringType::kInternalized)
.ToLocalChecked())
.ToLocalChecked()
->IsInt32());
EXPECT_TRUE(v8_object
->Get(context, v8::String::NewFromUtf8(
isolate_, "negative-int",
v8::NewStringType::kInternalized)
.ToLocalChecked())
.ToLocalChecked()
->IsInt32()); ->IsInt32());
EXPECT_TRUE( EXPECT_TRUE(
v8_object v8_object
->Get(v8::String::NewFromUtf8(isolate_, "double", ->Get(context, v8::String::NewFromUtf8(
v8::NewStringType::kInternalized) isolate_, "zero", v8::NewStringType::kInternalized)
.ToLocalChecked()) .ToLocalChecked())
->IsNumber()); .ToLocalChecked()
EXPECT_TRUE( ->IsInt32());
v8_object
->Get(v8::String::NewFromUtf8(isolate_, "big-integral-double",
v8::NewStringType::kInternalized)
.ToLocalChecked())
->IsNumber());
EXPECT_TRUE(
v8_object
->Get(v8::String::NewFromUtf8(isolate_, "string",
v8::NewStringType::kInternalized)
.ToLocalChecked())
->IsString());
EXPECT_TRUE(
v8_object
->Get(v8::String::NewFromUtf8(isolate_, "empty-string",
v8::NewStringType::kInternalized)
.ToLocalChecked())
->IsString());
EXPECT_TRUE(
v8_object
->Get(v8::String::NewFromUtf8(isolate_, "dictionary",
v8::NewStringType::kInternalized)
.ToLocalChecked())
->IsObject());
EXPECT_TRUE(
v8_object
->Get(v8::String::NewFromUtf8(isolate_, "empty-dictionary",
v8::NewStringType::kInternalized)
.ToLocalChecked())
->IsObject());
EXPECT_TRUE(v8_object EXPECT_TRUE(v8_object
->Get(v8::String::NewFromUtf8( ->Get(context, v8::String::NewFromUtf8(
isolate_, "list", v8::NewStringType::kInternalized) isolate_, "double",
.ToLocalChecked()) v8::NewStringType::kInternalized)
->IsArray()); .ToLocalChecked())
.ToLocalChecked()
->IsNumber());
EXPECT_TRUE(v8_object
->Get(context, v8::String::NewFromUtf8(
isolate_, "big-integral-double",
v8::NewStringType::kInternalized)
.ToLocalChecked())
.ToLocalChecked()
->IsNumber());
EXPECT_TRUE(v8_object
->Get(context, v8::String::NewFromUtf8(
isolate_, "string",
v8::NewStringType::kInternalized)
.ToLocalChecked())
.ToLocalChecked()
->IsString());
EXPECT_TRUE(v8_object
->Get(context, v8::String::NewFromUtf8(
isolate_, "empty-string",
v8::NewStringType::kInternalized)
.ToLocalChecked())
.ToLocalChecked()
->IsString());
EXPECT_TRUE(v8_object
->Get(context, v8::String::NewFromUtf8(
isolate_, "dictionary",
v8::NewStringType::kInternalized)
.ToLocalChecked())
.ToLocalChecked()
->IsObject());
EXPECT_TRUE(v8_object
->Get(context, v8::String::NewFromUtf8(
isolate_, "empty-dictionary",
v8::NewStringType::kInternalized)
.ToLocalChecked())
.ToLocalChecked()
->IsObject());
EXPECT_TRUE( EXPECT_TRUE(
v8_object v8_object
->Get(v8::String::NewFromUtf8(isolate_, "empty-list", ->Get(context, v8::String::NewFromUtf8(
v8::NewStringType::kInternalized) isolate_, "list", v8::NewStringType::kInternalized)
.ToLocalChecked()) .ToLocalChecked())
.ToLocalChecked()
->IsArray()); ->IsArray());
EXPECT_TRUE(v8_object
->Get(context, v8::String::NewFromUtf8(
isolate_, "empty-list",
v8::NewStringType::kInternalized)
.ToLocalChecked())
.ToLocalChecked()
->IsArray());
std::unique_ptr<base::Value> new_root( std::unique_ptr<base::Value> new_root(
converter.FromV8Value(v8_object, context)); converter.FromV8Value(v8_object, context));
...@@ -400,7 +424,7 @@ TEST_F(V8ValueConverterImplTest, ObjectExceptions) { ...@@ -400,7 +424,7 @@ TEST_F(V8ValueConverterImplTest, ObjectExceptions) {
v8::Local<v8::String> bar = v8::Local<v8::String> bar =
v8::String::NewFromUtf8(isolate_, "bar", v8::NewStringType::kInternalized) v8::String::NewFromUtf8(isolate_, "bar", v8::NewStringType::kInternalized)
.ToLocalChecked(); .ToLocalChecked();
object->Set(bar, bar); object->Set(context, bar, bar).Check();
// Converting from v8 value should replace the foo property with null. // Converting from v8 value should replace the foo property with null.
V8ValueConverterImpl converter; V8ValueConverterImpl converter;
...@@ -690,15 +714,21 @@ TEST_F(V8ValueConverterImplTest, RecursiveObjects) { ...@@ -690,15 +714,21 @@ TEST_F(V8ValueConverterImplTest, RecursiveObjects) {
v8::Local<v8::Object> object = v8::Object::New(isolate_).As<v8::Object>(); v8::Local<v8::Object> object = v8::Object::New(isolate_).As<v8::Object>();
ASSERT_FALSE(object.IsEmpty()); ASSERT_FALSE(object.IsEmpty());
object->Set( object
v8::String::NewFromUtf8(isolate_, "foo", v8::NewStringType::kInternalized) ->Set(context,
.ToLocalChecked(), v8::String::NewFromUtf8(isolate_, "foo",
v8::String::NewFromUtf8(isolate_, "bar", v8::NewStringType::kNormal) v8::NewStringType::kInternalized)
.ToLocalChecked()); .ToLocalChecked(),
object->Set( v8::String::NewFromUtf8(isolate_, "bar", v8::NewStringType::kNormal)
v8::String::NewFromUtf8(isolate_, "obj", v8::NewStringType::kInternalized) .ToLocalChecked())
.ToLocalChecked(), .Check();
object); object
->Set(context,
v8::String::NewFromUtf8(isolate_, "obj",
v8::NewStringType::kInternalized)
.ToLocalChecked(),
object)
.Check();
std::unique_ptr<base::DictionaryValue> object_result( std::unique_ptr<base::DictionaryValue> object_result(
base::DictionaryValue::From(converter.FromV8Value(object, context))); base::DictionaryValue::From(converter.FromV8Value(object, context)));
...@@ -708,10 +738,12 @@ TEST_F(V8ValueConverterImplTest, RecursiveObjects) { ...@@ -708,10 +738,12 @@ TEST_F(V8ValueConverterImplTest, RecursiveObjects) {
v8::Local<v8::Array> array = v8::Array::New(isolate_).As<v8::Array>(); v8::Local<v8::Array> array = v8::Array::New(isolate_).As<v8::Array>();
ASSERT_FALSE(array.IsEmpty()); ASSERT_FALSE(array.IsEmpty());
array->Set(0, array
v8::String::NewFromUtf8(isolate_, "1", v8::NewStringType::kNormal) ->Set(context, 0,
.ToLocalChecked()); v8::String::NewFromUtf8(isolate_, "1", v8::NewStringType::kNormal)
array->Set(1, array); .ToLocalChecked())
.Check();
array->Set(context, 1, array).Check();
std::unique_ptr<base::ListValue> list_result( std::unique_ptr<base::ListValue> list_result(
base::ListValue::From(converter.FromV8Value(array, context))); base::ListValue::From(converter.FromV8Value(array, context)));
...@@ -842,10 +874,14 @@ TEST_F(V8ValueConverterImplTest, ObjectsWithClashingIdentityHash) { ...@@ -842,10 +874,14 @@ TEST_F(V8ValueConverterImplTest, ObjectsWithClashingIdentityHash) {
// Create the v8::Object to be converted. // Create the v8::Object to be converted.
v8::Local<v8::Array> root(v8::Array::New(isolate_, 4)); v8::Local<v8::Array> root(v8::Array::New(isolate_, 4));
root->Set(0, v8::Local<v8::Object>(v8::Object::New(isolate_))); root->Set(context, 0, v8::Local<v8::Object>(v8::Object::New(isolate_)))
root->Set(1, v8::Local<v8::Object>(v8::Object::New(isolate_))); .Check();
root->Set(2, v8::Local<v8::Object>(v8::Array::New(isolate_, 0))); root->Set(context, 1, v8::Local<v8::Object>(v8::Object::New(isolate_)))
root->Set(3, v8::Local<v8::Object>(v8::Array::New(isolate_, 0))); .Check();
root->Set(context, 2, v8::Local<v8::Object>(v8::Array::New(isolate_, 0)))
.Check();
root->Set(context, 3, v8::Local<v8::Object>(v8::Array::New(isolate_, 0)))
.Check();
// The expected base::Value result. // The expected base::Value result.
std::unique_ptr<base::Value> expected = std::unique_ptr<base::Value> expected =
...@@ -868,7 +904,7 @@ TEST_F(V8ValueConverterImplTest, DetectCycles) { ...@@ -868,7 +904,7 @@ TEST_F(V8ValueConverterImplTest, DetectCycles) {
// Create a recursive array. // Create a recursive array.
v8::Local<v8::Array> recursive_array(v8::Array::New(isolate_, 1)); v8::Local<v8::Array> recursive_array(v8::Array::New(isolate_, 1));
recursive_array->Set(0, recursive_array); recursive_array->Set(context, 0, recursive_array).Check();
// The first repetition should be trimmed and replaced by a null value. // The first repetition should be trimmed and replaced by a null value.
base::ListValue expected_list; base::ListValue expected_list;
...@@ -884,13 +920,14 @@ TEST_F(V8ValueConverterImplTest, DetectCycles) { ...@@ -884,13 +920,14 @@ TEST_F(V8ValueConverterImplTest, DetectCycles) {
// Now create a recursive object // Now create a recursive object
const std::string key("key"); const std::string key("key");
v8::Local<v8::Object> recursive_object(v8::Object::New(isolate_)); v8::Local<v8::Object> recursive_object(v8::Object::New(isolate_));
v8::TryCatch try_catch(isolate_); recursive_object
recursive_object->Set( ->Set(context,
v8::String::NewFromUtf8(isolate_, key.c_str(), v8::String::NewFromUtf8(isolate_, key.c_str(),
v8::NewStringType::kInternalized, key.length()) v8::NewStringType::kInternalized,
.ToLocalChecked(), key.length())
recursive_object); .ToLocalChecked(),
ASSERT_FALSE(try_catch.HasCaught()); recursive_object)
.Check();
// The first repetition should be trimmed and replaced by a null value. // The first repetition should be trimmed and replaced by a null value.
base::DictionaryValue expected_dictionary; base::DictionaryValue expected_dictionary;
...@@ -981,10 +1018,12 @@ TEST_F(V8ValueConverterImplTest, MaxRecursionDepth) { ...@@ -981,10 +1018,12 @@ TEST_F(V8ValueConverterImplTest, MaxRecursionDepth) {
v8::Local<v8::Object> leaf = deep_object; v8::Local<v8::Object> leaf = deep_object;
for (int i = 0; i < kDepth; ++i) { for (int i = 0; i < kDepth; ++i) {
v8::Local<v8::Object> new_object = v8::Object::New(isolate_); v8::Local<v8::Object> new_object = v8::Object::New(isolate_);
leaf->Set(v8::String::NewFromUtf8(isolate_, kKey, leaf->Set(context,
v8::String::NewFromUtf8(isolate_, kKey,
v8::NewStringType::kInternalized) v8::NewStringType::kInternalized)
.ToLocalChecked(), .ToLocalChecked(),
new_object); new_object)
.Check();
leaf = new_object; leaf = new_object;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment