From 85a0e8075f433fe92c9a4f2df3c86d14000580d9 Mon Sep 17 00:00:00 2001 From: dcarney Date: Tue, 17 Mar 2015 04:45:35 -0700 Subject: [PATCH] convert String::New functions to maybe R=svenpanne@chromium.org BUG=v8:3929 LOG=n Review URL: https://codereview.chromium.org/1010803008 Cr-Commit-Position: refs/heads/master@{#27236} --- include/v8.h | 75 +++++++++++++++------ src/api.cc | 168 +++++++++++++++++++++++++++--------------------- test/cctest/test-api.cc | 26 +++----- 3 files changed, 160 insertions(+), 109 deletions(-) diff --git a/include/v8.h b/include/v8.h index 10152f6..755a87f 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2031,11 +2031,16 @@ class V8_EXPORT Name : public Primitive { }; +enum class NewStringType { kNormal, kInternalized }; + + /** * A JavaScript string value (ECMA-262, 4.3.17). */ class V8_EXPORT String : public Name { public: + static const int kMaxLength = (1 << 28) - 16; + enum Encoding { UNKNOWN_ENCODING = 0x1, TWO_BYTE_ENCODING = 0x0, @@ -2232,26 +2237,52 @@ class V8_EXPORT String : public Name { V8_INLINE static String* Cast(v8::Value* obj); - enum NewStringType { kNormalString, kInternalizedString }; + // TODO(dcarney): remove with deprecation of New functions. + enum NewStringType { + kNormalString = static_cast(v8::NewStringType::kNormal), + kInternalizedString = static_cast(v8::NewStringType::kInternalized) + }; /** Allocates a new string from UTF-8 data.*/ - static Local NewFromUtf8(Isolate* isolate, const char* data, - NewStringType type = kNormalString, - int length = -1); + static V8_DEPRECATE_SOON( + "Use maybe version", + Local NewFromUtf8(Isolate* isolate, const char* data, + NewStringType type = kNormalString, + int length = -1)); + + /** Allocates a new string from UTF-8 data. Only returns an empty value when + * length > kMaxLength. **/ + static MaybeLocal NewFromUtf8(Isolate* isolate, const char* data, + v8::NewStringType type, + int length = -1); /** Allocates a new string from Latin-1 data.*/ - static Local NewFromOneByte( - Isolate* isolate, - const uint8_t* data, - NewStringType type = kNormalString, - int length = -1); + static V8_DEPRECATE_SOON( + "Use maybe version", + Local NewFromOneByte(Isolate* isolate, const uint8_t* data, + NewStringType type = kNormalString, + int length = -1)); + + /** Allocates a new string from Latin-1 data. Only returns an empty value + * when length > kMaxLength. **/ + static MaybeLocal NewFromOneByte(Isolate* isolate, + const uint8_t* data, + v8::NewStringType type, + int length = -1); /** Allocates a new string from UTF-16 data.*/ - static Local NewFromTwoByte( - Isolate* isolate, - const uint16_t* data, - NewStringType type = kNormalString, - int length = -1); + static V8_DEPRECATE_SOON( + "Use maybe version", + Local NewFromTwoByte(Isolate* isolate, const uint16_t* data, + NewStringType type = kNormalString, + int length = -1)); + + /** Allocates a new string from UTF-16 data. Only returns an empty value when + * length > kMaxLength. **/ + static MaybeLocal NewFromTwoByte(Isolate* isolate, + const uint16_t* data, + v8::NewStringType type, + int length = -1); /** * Creates a new string by concatenating the left and the right strings @@ -2267,8 +2298,12 @@ class V8_EXPORT String : public Name { * should the underlying buffer be deallocated or modified except through the * destructor of the external string resource. */ - static Local NewExternal(Isolate* isolate, - ExternalStringResource* resource); + static V8_DEPRECATE_SOON( + "Use maybe version", + Local NewExternal(Isolate* isolate, + ExternalStringResource* resource)); + static MaybeLocal NewExternalTwoByte( + Isolate* isolate, ExternalStringResource* resource); /** * Associate an external string resource with this string by transforming it @@ -2289,8 +2324,12 @@ class V8_EXPORT String : public Name { * should the underlying buffer be deallocated or modified except through the * destructor of the external string resource. */ - static Local NewExternal(Isolate* isolate, - ExternalOneByteStringResource* resource); + static V8_DEPRECATE_SOON( + "Use maybe version", + Local NewExternal(Isolate* isolate, + ExternalOneByteStringResource* resource)); + static MaybeLocal NewExternalOneByte( + Isolate* isolate, ExternalOneByteStringResource* resource); /** * Associate an external string resource with this string by transforming it diff --git a/src/api.cc b/src/api.cc index 1c27857..fd17edf 100644 --- a/src/api.cc +++ b/src/api.cc @@ -5671,9 +5671,9 @@ inline int StringLength(const uint16_t* string) { MUST_USE_RESULT inline i::MaybeHandle NewString(i::Factory* factory, - String::NewStringType type, + v8::NewStringType type, i::Vector string) { - if (type == String::kInternalizedString) { + if (type == v8::NewStringType::kInternalized) { return factory->InternalizeUtf8String(string); } return factory->NewStringFromUtf8(string); @@ -5682,9 +5682,9 @@ inline i::MaybeHandle NewString(i::Factory* factory, MUST_USE_RESULT inline i::MaybeHandle NewString(i::Factory* factory, - String::NewStringType type, + v8::NewStringType type, i::Vector string) { - if (type == String::kInternalizedString) { + if (type == v8::NewStringType::kInternalized) { return factory->InternalizeOneByteString(string); } return factory->NewStringFromOneByte(string); @@ -5693,35 +5693,32 @@ inline i::MaybeHandle NewString(i::Factory* factory, MUST_USE_RESULT inline i::MaybeHandle NewString(i::Factory* factory, - String::NewStringType type, + v8::NewStringType type, i::Vector string) { - if (type == String::kInternalizedString) { + if (type == v8::NewStringType::kInternalized) { return factory->InternalizeTwoByteString(string); } return factory->NewStringFromTwoByte(string); } -template -inline Local NewString(Isolate* v8_isolate, - const char* location, - const char* env, - const Char* data, - String::NewStringType type, - int length) { +STATIC_ASSERT(v8::String::kMaxLength == i::String::kMaxLength); + + +template +inline MaybeLocal NewString(Isolate* v8_isolate, const char* location, + const char* env, const Char* data, + v8::NewStringType type, int length) { i::Isolate* isolate = reinterpret_cast(v8_isolate); - LOG_API(isolate, env); - if (length == 0) { - return String::Empty(v8_isolate); - } + if (length == 0) return String::Empty(v8_isolate); + // TODO(dcarney): throw a context free exception. + if (length > i::String::kMaxLength) return MaybeLocal(); ENTER_V8(isolate); - if (length == -1) length = StringLength(data); - EXCEPTION_PREAMBLE(isolate); - i::Handle result; - has_pending_exception = - !NewString(isolate->factory(), type, i::Vector(data, length)) - .ToHandle(&result); - EXCEPTION_BAILOUT_CHECK(isolate, Local()); + LOG_API(isolate, env); + if (length < 0) length = StringLength(data); + i::Handle result = + NewString(isolate->factory(), type, i::Vector(data, length)) + .ToHandleChecked(); return Utils::ToLocal(result); } @@ -5732,12 +5729,17 @@ Local String::NewFromUtf8(Isolate* isolate, const char* data, NewStringType type, int length) { - return NewString(isolate, - "v8::String::NewFromUtf8()", - "String::NewFromUtf8", - data, - type, - length); + RETURN_TO_LOCAL_UNCHECKED( + NewString(isolate, "v8::String::NewFromUtf8()", "String::NewFromUtf8", + data, static_cast(type), length), + String); +} + + +MaybeLocal String::NewFromUtf8(Isolate* isolate, const char* data, + v8::NewStringType type, int length) { + return NewString(isolate, "v8::String::NewFromUtf8()", "String::NewFromUtf8", + data, type, length); } @@ -5745,12 +5747,18 @@ Local String::NewFromOneByte(Isolate* isolate, const uint8_t* data, NewStringType type, int length) { - return NewString(isolate, - "v8::String::NewFromOneByte()", - "String::NewFromOneByte", - data, - type, - length); + RETURN_TO_LOCAL_UNCHECKED( + NewString(isolate, "v8::String::NewFromOneByte()", + "String::NewFromOneByte", data, + static_cast(type), length), + String); +} + + +MaybeLocal String::NewFromOneByte(Isolate* isolate, const uint8_t* data, + v8::NewStringType type, int length) { + return NewString(isolate, "v8::String::NewFromOneByte()", + "String::NewFromOneByte", data, type, length); } @@ -5758,20 +5766,27 @@ Local String::NewFromTwoByte(Isolate* isolate, const uint16_t* data, NewStringType type, int length) { - return NewString(isolate, - "v8::String::NewFromTwoByte()", - "String::NewFromTwoByte", - data, - type, - length); + RETURN_TO_LOCAL_UNCHECKED( + NewString(isolate, "v8::String::NewFromTwoByte()", + "String::NewFromTwoByte", data, + static_cast(type), length), + String); +} + + +MaybeLocal String::NewFromTwoByte(Isolate* isolate, + const uint16_t* data, + v8::NewStringType type, int length) { + return NewString(isolate, "v8::String::NewFromTwoByte()", + "String::NewFromTwoByte", data, type, length); } Local v8::String::Concat(Handle left, Handle right) { i::Handle left_string = Utils::OpenHandle(*left); i::Isolate* isolate = left_string->GetIsolate(); - LOG_API(isolate, "String::New(char)"); ENTER_V8(isolate); + LOG_API(isolate, "v8::String::Concat"); i::Handle right_string = Utils::OpenHandle(*right); // If we are steering towards a range error, do not wait for the error to be // thrown, and return the null handle instead. @@ -5784,35 +5799,54 @@ Local v8::String::Concat(Handle left, Handle right) { } -static i::MaybeHandle NewExternalStringHandle( - i::Isolate* isolate, v8::String::ExternalStringResource* resource) { - return isolate->factory()->NewExternalStringFromTwoByte(resource); +MaybeLocal v8::String::NewExternalTwoByte( + Isolate* isolate, v8::String::ExternalStringResource* resource) { + CHECK(resource && resource->data()); + // TODO(dcarney): throw a context free exception. + if (resource->length() > static_cast(i::String::kMaxLength)) { + return MaybeLocal(); + } + i::Isolate* i_isolate = reinterpret_cast(isolate); + ENTER_V8(i_isolate); + LOG_API(i_isolate, "String::NewExternalTwoByte"); + i::Handle string = i_isolate->factory() + ->NewExternalStringFromTwoByte(resource) + .ToHandleChecked(); + i_isolate->heap()->external_string_table()->AddString(*string); + return Utils::ToLocal(string); } -static i::MaybeHandle NewExternalOneByteStringHandle( - i::Isolate* isolate, v8::String::ExternalOneByteStringResource* resource) { - return isolate->factory()->NewExternalStringFromOneByte(resource); +Local v8::String::NewExternal( + Isolate* isolate, v8::String::ExternalStringResource* resource) { + RETURN_TO_LOCAL_UNCHECKED(NewExternalTwoByte(isolate, resource), String); } -Local v8::String::NewExternal( - Isolate* isolate, - v8::String::ExternalStringResource* resource) { +MaybeLocal v8::String::NewExternalOneByte( + Isolate* isolate, v8::String::ExternalOneByteStringResource* resource) { + CHECK(resource && resource->data()); + // TODO(dcarney): throw a context free exception. + if (resource->length() > static_cast(i::String::kMaxLength)) { + return MaybeLocal(); + } i::Isolate* i_isolate = reinterpret_cast(isolate); - LOG_API(i_isolate, "String::NewExternal"); ENTER_V8(i_isolate); - CHECK(resource && resource->data()); - EXCEPTION_PREAMBLE(i_isolate); - i::Handle string; - has_pending_exception = - !NewExternalStringHandle(i_isolate, resource).ToHandle(&string); - EXCEPTION_BAILOUT_CHECK(i_isolate, Local()); + LOG_API(i_isolate, "String::NewExternalOneByte"); + i::Handle string = i_isolate->factory() + ->NewExternalStringFromOneByte(resource) + .ToHandleChecked(); i_isolate->heap()->external_string_table()->AddString(*string); return Utils::ToLocal(string); } +Local v8::String::NewExternal( + Isolate* isolate, v8::String::ExternalOneByteStringResource* resource) { + RETURN_TO_LOCAL_UNCHECKED(NewExternalOneByte(isolate, resource), String); +} + + bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) { i::Handle obj = Utils::OpenHandle(this); i::Isolate* isolate = obj->GetIsolate(); @@ -5839,22 +5873,6 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) { } -Local v8::String::NewExternal( - Isolate* isolate, v8::String::ExternalOneByteStringResource* resource) { - i::Isolate* i_isolate = reinterpret_cast(isolate); - LOG_API(i_isolate, "String::NewExternal"); - ENTER_V8(i_isolate); - CHECK(resource && resource->data()); - EXCEPTION_PREAMBLE(i_isolate); - i::Handle string; - has_pending_exception = - !NewExternalOneByteStringHandle(i_isolate, resource).ToHandle(&string); - EXCEPTION_BAILOUT_CHECK(i_isolate, Local()); - i_isolate->heap()->external_string_table()->AddString(*string); - return Utils::ToLocal(string); -} - - bool v8::String::MakeExternal( v8::String::ExternalOneByteStringResource* resource) { i::Handle obj = Utils::OpenHandle(this); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 7016bf7..d339c15 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -694,28 +694,23 @@ class RandomLengthOneByteResource THREADED_TEST(NewExternalForVeryLongString) { + auto isolate = CcTest::isolate(); { - LocalContext env; - v8::HandleScope scope(env->GetIsolate()); + v8::HandleScope scope(isolate); v8::TryCatch try_catch; RandomLengthOneByteResource r(1 << 30); - v8::Local str = v8::String::NewExternal(CcTest::isolate(), &r); + v8::Local str = v8::String::NewExternal(isolate, &r); CHECK(str.IsEmpty()); - CHECK(try_catch.HasCaught()); - String::Utf8Value exception_value(try_catch.Exception()); - CHECK_EQ(0, strcmp("RangeError: Invalid string length", *exception_value)); + CHECK(!try_catch.HasCaught()); } { - LocalContext env; - v8::HandleScope scope(env->GetIsolate()); + v8::HandleScope scope(isolate); v8::TryCatch try_catch; RandomLengthResource r(1 << 30); - v8::Local str = v8::String::NewExternal(CcTest::isolate(), &r); + v8::Local str = v8::String::NewExternal(isolate, &r); CHECK(str.IsEmpty()); - CHECK(try_catch.HasCaught()); - String::Utf8Value exception_value(try_catch.Exception()); - CHECK_EQ(0, strcmp("RangeError: Invalid string length", *exception_value)); + CHECK(!try_catch.HasCaught()); } } @@ -21618,7 +21613,6 @@ TEST(StreamingScriptWithSourceMappingURLInTheMiddle) { TEST(NewStringRangeError) { v8::Isolate* isolate = CcTest::isolate(); v8::HandleScope handle_scope(isolate); - LocalContext env; const int length = i::String::kMaxLength + 1; const int buffer_size = length * sizeof(uint16_t); void* buffer = malloc(buffer_size); @@ -21629,21 +21623,21 @@ TEST(NewStringRangeError) { char* data = reinterpret_cast(buffer); CHECK(v8::String::NewFromUtf8(isolate, data, v8::String::kNormalString, length).IsEmpty()); - CHECK(try_catch.HasCaught()); + CHECK(!try_catch.HasCaught()); } { v8::TryCatch try_catch; uint8_t* data = reinterpret_cast(buffer); CHECK(v8::String::NewFromOneByte(isolate, data, v8::String::kNormalString, length).IsEmpty()); - CHECK(try_catch.HasCaught()); + CHECK(!try_catch.HasCaught()); } { v8::TryCatch try_catch; uint16_t* data = reinterpret_cast(buffer); CHECK(v8::String::NewFromTwoByte(isolate, data, v8::String::kNormalString, length).IsEmpty()); - CHECK(try_catch.HasCaught()); + CHECK(!try_catch.HasCaught()); } free(buffer); } -- 2.7.4