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

@@ -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<int>(v8::NewStringType::kNormal),
+    kInternalizedString = static_cast<int>(v8::NewStringType::kInternalized)
+  };
 
   /** Allocates a new string from UTF-8 data.*/
-  static Local<String> NewFromUtf8(Isolate* isolate, const char* data,
-                                   NewStringType type = kNormalString,
-                                   int length = -1);
+  static V8_DEPRECATE_SOON(
+      "Use maybe version",
+      Local<String> 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<String> NewFromUtf8(Isolate* isolate, const char* data,
+                                        v8::NewStringType type,
+                                        int length = -1);
 
   /** Allocates a new string from Latin-1 data.*/
-  static Local<String> NewFromOneByte(
-      Isolate* isolate,
-      const uint8_t* data,
-      NewStringType type = kNormalString,
-      int length = -1);
+  static V8_DEPRECATE_SOON(
+      "Use maybe version",
+      Local<String> 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<String> NewFromOneByte(Isolate* isolate,
+                                           const uint8_t* data,
+                                           v8::NewStringType type,
+                                           int length = -1);
 
   /** Allocates a new string from UTF-16 data.*/
-  static Local<String> NewFromTwoByte(
-      Isolate* isolate,
-      const uint16_t* data,
-      NewStringType type = kNormalString,
-      int length = -1);
+  static V8_DEPRECATE_SOON(
+      "Use maybe version",
+      Local<String> 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<String> 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<String> NewExternal(Isolate* isolate,
-                                   ExternalStringResource* resource);
+  static V8_DEPRECATE_SOON(
+      "Use maybe version",
+      Local<String> NewExternal(Isolate* isolate,
+                                ExternalStringResource* resource));
+  static MaybeLocal<String> 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<String> NewExternal(Isolate* isolate,
-                                   ExternalOneByteStringResource* resource);
+  static V8_DEPRECATE_SOON(
+      "Use maybe version",
+      Local<String> NewExternal(Isolate* isolate,
+                                ExternalOneByteStringResource* resource));
+  static MaybeLocal<String> NewExternalOneByte(
+      Isolate* isolate, ExternalOneByteStringResource* resource);
 
   /**
    * Associate an external string resource with this string by transforming it

src/api.cc

@@ -5671,9 +5671,9 @@ inline int StringLength(const uint16_t* string) {
 
 MUST_USE_RESULT
 inline i::MaybeHandle<i::String> NewString(i::Factory* factory,
-                                           String::NewStringType type,
+                                           v8::NewStringType type,
                                            i::Vector<const char> 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<i::String> NewString(i::Factory* factory,
 
 MUST_USE_RESULT
 inline i::MaybeHandle<i::String> NewString(i::Factory* factory,
-                                           String::NewStringType type,
+                                           v8::NewStringType type,
                                            i::Vector<const uint8_t> 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<i::String> NewString(i::Factory* factory,
 
 MUST_USE_RESULT
 inline i::MaybeHandle<i::String> NewString(i::Factory* factory,
-                                           String::NewStringType type,
+                                           v8::NewStringType type,
                                            i::Vector<const uint16_t> string) {
-  if (type == String::kInternalizedString) {
+  if (type == v8::NewStringType::kInternalized) {
     return factory->InternalizeTwoByteString(string);
   }
   return factory->NewStringFromTwoByte(string);
 }
 
 
-template<typename Char>
-inline Local<String> 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 <typename Char>
+inline MaybeLocal<String> NewString(Isolate* v8_isolate, const char* location,
+                                    const char* env, const Char* data,
+                                    v8::NewStringType type, int length) {
   i::Isolate* isolate = reinterpret_cast<internal::Isolate*>(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<String>();
   ENTER_V8(isolate);
-  if (length == -1) length = StringLength(data);
-  EXCEPTION_PREAMBLE(isolate);
-  i::Handle<i::String> result;
-  has_pending_exception =
-      !NewString(isolate->factory(), type, i::Vector<const Char>(data, length))
-           .ToHandle(&result);
-  EXCEPTION_BAILOUT_CHECK(isolate, Local<String>());
+  LOG_API(isolate, env);
+  if (length < 0) length = StringLength(data);
+  i::Handle<i::String> result =
+      NewString(isolate->factory(), type, i::Vector<const Char>(data, length))
+          .ToHandleChecked();
   return Utils::ToLocal(result);
 }
 
@@ -5732,12 +5729,17 @@ Local<String> 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<v8::NewStringType>(type), length),
+      String);
+}
+
+
+MaybeLocal<String> 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> 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<v8::NewStringType>(type), length),
+      String);
+}
+
+
+MaybeLocal<String> 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> 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<v8::NewStringType>(type), length),
+      String);
+}
+
+
+MaybeLocal<String> 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<String> v8::String::Concat(Handle<String> left, Handle<String> right) {
   i::Handle<i::String> 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<i::String> 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<String> v8::String::Concat(Handle<String> left, Handle<String> right) {
 }
 
 
-static i::MaybeHandle<i::String> NewExternalStringHandle(
-    i::Isolate* isolate, v8::String::ExternalStringResource* resource) {
-  return isolate->factory()->NewExternalStringFromTwoByte(resource);
+MaybeLocal<String> 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<size_t>(i::String::kMaxLength)) {
+    return MaybeLocal<String>();
+  }
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  ENTER_V8(i_isolate);
+  LOG_API(i_isolate, "String::NewExternalTwoByte");
+  i::Handle<i::String> string = i_isolate->factory()
+                                    ->NewExternalStringFromTwoByte(resource)
+                                    .ToHandleChecked();
+  i_isolate->heap()->external_string_table()->AddString(*string);
+  return Utils::ToLocal(string);
 }
 
 
-static i::MaybeHandle<i::String> NewExternalOneByteStringHandle(
-    i::Isolate* isolate, v8::String::ExternalOneByteStringResource* resource) {
-  return isolate->factory()->NewExternalStringFromOneByte(resource);
+Local<String> v8::String::NewExternal(
+    Isolate* isolate, v8::String::ExternalStringResource* resource) {
+  RETURN_TO_LOCAL_UNCHECKED(NewExternalTwoByte(isolate, resource), String);
 }
 
 
-Local<String> v8::String::NewExternal(
-    Isolate* isolate,
-    v8::String::ExternalStringResource* resource) {
+MaybeLocal<String> 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<size_t>(i::String::kMaxLength)) {
+    return MaybeLocal<String>();
+  }
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
-  LOG_API(i_isolate, "String::NewExternal");
   ENTER_V8(i_isolate);
-  CHECK(resource && resource->data());
-  EXCEPTION_PREAMBLE(i_isolate);
-  i::Handle<i::String> string;
-  has_pending_exception =
-      !NewExternalStringHandle(i_isolate, resource).ToHandle(&string);
-  EXCEPTION_BAILOUT_CHECK(i_isolate, Local<String>());
+  LOG_API(i_isolate, "String::NewExternalOneByte");
+  i::Handle<i::String> string = i_isolate->factory()
+                                    ->NewExternalStringFromOneByte(resource)
+                                    .ToHandleChecked();
   i_isolate->heap()->external_string_table()->AddString(*string);
   return Utils::ToLocal(string);
 }
 
 
+Local<String> 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<i::String> obj = Utils::OpenHandle(this);
   i::Isolate* isolate = obj->GetIsolate();
@@ -5839,22 +5873,6 @@ bool v8::String::MakeExternal(v8::String::ExternalStringResource* resource) {
 }
 
 
-Local<String> v8::String::NewExternal(
-    Isolate* isolate, v8::String::ExternalOneByteStringResource* resource) {
-  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
-  LOG_API(i_isolate, "String::NewExternal");
-  ENTER_V8(i_isolate);
-  CHECK(resource && resource->data());
-  EXCEPTION_PREAMBLE(i_isolate);
-  i::Handle<i::String> string;
-  has_pending_exception =
-      !NewExternalOneByteStringHandle(i_isolate, resource).ToHandle(&string);
-  EXCEPTION_BAILOUT_CHECK(i_isolate, Local<String>());
-  i_isolate->heap()->external_string_table()->AddString(*string);
-  return Utils::ToLocal(string);
-}
-
-
 bool v8::String::MakeExternal(
     v8::String::ExternalOneByteStringResource* resource) {
   i::Handle<i::String> obj = Utils::OpenHandle(this);

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<v8::String> str = v8::String::NewExternal(CcTest::isolate(), &r);
+    v8::Local<v8::String> 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<v8::String> str = v8::String::NewExternal(CcTest::isolate(), &r);
+    v8::Local<v8::String> 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<char*>(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<uint8_t*>(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<uint16_t*>(buffer);
     CHECK(v8::String::NewFromTwoByte(isolate, data, v8::String::kNormalString,
                                      length).IsEmpty());
-    CHECK(try_catch.HasCaught());
+    CHECK(!try_catch.HasCaught());
   }
   free(buffer);
 }