[Intl] Make error handling stricter

Bug: v8:5751
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: I5c736e0573babcd37e9ca3b28c737c9d44b3721a
Reviewed-on: https://chromium-review.googlesource.com/1149776
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54690}

src/objects/intl-objects.cc

@@ -94,15 +94,11 @@ bool ExtractBooleanSetting(Isolate* isolate, Handle<JSObject> options,
   return false;
 }
 
-icu::Locale CreateICULocale(Isolate* isolate, Handle<String> bcp47_locale_str,
-                            bool* success) {
-  *success = false;
+icu::Locale CreateICULocale(Isolate* isolate, Handle<String> bcp47_locale_str) {
   v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
   v8::String::Utf8Value bcp47_locale(v8_isolate,
                                      v8::Utils::ToLocal(bcp47_locale_str));
-  if (bcp47_locale.length() == 0) {
-    return icu::Locale();
-  }
+  CHECK_NOT_NULL(*bcp47_locale);
 
   DisallowHeapAllocation no_gc;
 
@@ -111,18 +107,18 @@ icu::Locale CreateICULocale(Isolate* isolate, Handle<String> bcp47_locale_str,
   char icu_result[ULOC_FULLNAME_CAPACITY];
   int icu_length = 0;
 
+  // bcp47_locale_str should be a canonicalized language tag, which
+  // means this shouldn't fail.
   uloc_forLanguageTag(*bcp47_locale, icu_result, ULOC_FULLNAME_CAPACITY,
                       &icu_length, &status);
-  if (U_FAILURE(status) || icu_length == 0) {
-    return icu::Locale();
-  }
+  CHECK(U_SUCCESS(status));
+  CHECK_LT(0, icu_length);
 
   icu::Locale icu_locale(icu_result);
   if (icu_locale.isBogus()) {
-    return icu::Locale();
+    FATAL("Failed to create ICU locale, are ICU data files missing?");
   }
 
-  *success = true;
   return icu_locale;
 }
 
@@ -857,9 +853,8 @@ void SetResolvedBreakIteratorSettings(Isolate* isolate,
 icu::SimpleDateFormat* DateFormat::InitializeDateTimeFormat(
     Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
     Handle<JSObject> resolved) {
-  bool success = false;
-  icu::Locale icu_locale = CreateICULocale(isolate, locale, &success);
-  if (!success) return nullptr;
+  icu::Locale icu_locale = CreateICULocale(isolate, locale);
+  DCHECK(!icu_locale.isBogus());
 
   icu::SimpleDateFormat* date_format =
       CreateICUDateFormat(isolate, icu_locale, options);
@@ -879,6 +874,7 @@ icu::SimpleDateFormat* DateFormat::InitializeDateTimeFormat(
     SetResolvedDateSettings(isolate, icu_locale, date_format, resolved);
   }
 
+  CHECK_NOT_NULL(date_format);
   return date_format;
 }
 
@@ -894,9 +890,8 @@ void DateFormat::DeleteDateFormat(const v8::WeakCallbackInfo<void>& data) {
 icu::DecimalFormat* NumberFormat::InitializeNumberFormat(
     Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
     Handle<JSObject> resolved) {
-  bool success = false;
-  icu::Locale icu_locale = CreateICULocale(isolate, locale, &success);
-  if (!success) return nullptr;
+  icu::Locale icu_locale = CreateICULocale(isolate, locale);
+  DCHECK(!icu_locale.isBogus());
 
   icu::DecimalFormat* number_format =
       CreateICUNumberFormat(isolate, icu_locale, options);
@@ -917,6 +912,7 @@ icu::DecimalFormat* NumberFormat::InitializeNumberFormat(
     SetResolvedNumberSettings(isolate, icu_locale, number_format, resolved);
   }
 
+  CHECK_NOT_NULL(number_format);
   return number_format;
 }
 
@@ -930,14 +926,12 @@ void NumberFormat::DeleteNumberFormat(const v8::WeakCallbackInfo<void>& data) {
   GlobalHandles::Destroy(reinterpret_cast<Object**>(data.GetParameter()));
 }
 
-bool Collator::InitializeCollator(Isolate* isolate,
-                                  Handle<JSObject> collator_holder,
-                                  Handle<String> locale,
-                                  Handle<JSObject> options,
-                                  Handle<JSObject> resolved) {
-  bool success = false;
-  icu::Locale icu_locale = CreateICULocale(isolate, locale, &success);
-  if (!success) return false;
+icu::Collator* Collator::InitializeCollator(Isolate* isolate,
+                                            Handle<String> locale,
+                                            Handle<JSObject> options,
+                                            Handle<JSObject> resolved) {
+  icu::Locale icu_locale = CreateICULocale(isolate, locale);
+  DCHECK(!icu_locale.isBogus());
 
   icu::Collator* collator = CreateICUCollator(isolate, icu_locale, options);
   if (!collator) {
@@ -956,28 +950,24 @@ bool Collator::InitializeCollator(Isolate* isolate,
     SetResolvedCollatorSettings(isolate, icu_locale, collator, resolved);
   }
 
-  Handle<Managed<icu::Collator>> managed =
-      Managed<icu::Collator>::FromRawPtr(isolate, 0, collator);
-  collator_holder->SetEmbedderField(0, *managed);
-
-  return true;
+  CHECK_NOT_NULL(collator);
+  return collator;
 }
 
 icu::Collator* Collator::UnpackCollator(Handle<JSObject> obj) {
   return Managed<icu::Collator>::cast(obj->GetEmbedderField(0))->raw();
 }
 
-bool PluralRules::InitializePluralRules(Isolate* isolate, Handle<String> locale,
+void PluralRules::InitializePluralRules(Isolate* isolate, Handle<String> locale,
                                         Handle<JSObject> options,
                                         Handle<JSObject> resolved,
                                         icu::PluralRules** plural_rules,
                                         icu::DecimalFormat** number_format) {
-  bool success = false;
-  icu::Locale icu_locale = CreateICULocale(isolate, locale, &success);
-  if (!success) return false;
+  icu::Locale icu_locale = CreateICULocale(isolate, locale);
+  DCHECK(!icu_locale.isBogus());
 
-  success = CreateICUPluralRules(isolate, icu_locale, options, plural_rules,
-                                 number_format);
+  bool success = CreateICUPluralRules(isolate, icu_locale, options,
+                                      plural_rules, number_format);
   if (!success) {
     // Remove extensions and try again.
     icu::Locale no_extension_locale(icu_locale.getBaseName());
@@ -996,7 +986,8 @@ bool PluralRules::InitializePluralRules(Isolate* isolate, Handle<String> locale,
                                              *number_format, resolved);
   }
 
-  return success;
+  CHECK_NOT_NULL(*plural_rules);
+  CHECK_NOT_NULL(*number_format);
 }
 
 icu::PluralRules* PluralRules::UnpackPluralRules(Handle<JSObject> obj) {
@@ -1016,9 +1007,8 @@ void PluralRules::DeletePluralRules(const v8::WeakCallbackInfo<void>& data) {
 icu::BreakIterator* V8BreakIterator::InitializeBreakIterator(
     Isolate* isolate, Handle<String> locale, Handle<JSObject> options,
     Handle<JSObject> resolved) {
-  bool success = false;
-  icu::Locale icu_locale = CreateICULocale(isolate, locale, &success);
-  if (!success) return nullptr;
+  icu::Locale icu_locale = CreateICULocale(isolate, locale);
+  DCHECK(!icu_locale.isBogus());
 
   icu::BreakIterator* break_iterator =
       CreateICUBreakIterator(isolate, icu_locale, options);
@@ -1040,6 +1030,7 @@ icu::BreakIterator* V8BreakIterator::InitializeBreakIterator(
                                      resolved);
   }
 
+  CHECK_NOT_NULL(break_iterator);
   return break_iterator;
 }
 

src/objects/intl-objects.h

@@ -120,11 +120,10 @@ class Collator {
  public:
   // Create a collator for the specificied locale and options. Stores the
   // collator in the provided collator_holder.
-  static bool InitializeCollator(Isolate* isolate,
-                                 Handle<JSObject> collator_holder,
-                                 Handle<String> locale,
-                                 Handle<JSObject> options,
-                                 Handle<JSObject> resolved);
+  static icu::Collator* InitializeCollator(Isolate* isolate,
+                                           Handle<String> locale,
+                                           Handle<JSObject> options,
+                                           Handle<JSObject> resolved);
 
   // Unpacks collator object from corresponding JavaScript object.
   static icu::Collator* UnpackCollator(Handle<JSObject> obj);
@@ -141,7 +140,7 @@ class PluralRules {
  public:
   // Create a PluralRules and DecimalFormat for the specificied locale and
   // options. Returns false on an ICU failure.
-  static bool InitializePluralRules(Isolate* isolate, Handle<String> locale,
+  static void InitializePluralRules(Isolate* isolate, Handle<String> locale,
                                     Handle<JSObject> options,
                                     Handle<JSObject> resolved,
                                     icu::PluralRules** plural_rules,

src/objects/js-list-format.cc

@@ -187,7 +187,10 @@ MaybeHandle<JSListFormat> JSListFormat::InitializeListFormat(
   UErrorCode status = U_ZERO_ERROR;
   icu::ListFormatter* formatter = icu::ListFormatter::createInstance(
       icu_locale, GetIcuStyleString(style_enum, type_enum), status);
-  CHECK(U_SUCCESS(status));
+  if (U_FAILURE(status)) {
+    delete formatter;
+    FATAL("Failed to create ICU list formatter, are ICU data files missing?");
+  }
   CHECK_NOT_NULL(formatter);
 
   Handle<Managed<icu::ListFormatter>> managed_formatter =

src/objects/js-relative-time-format.cc

@@ -134,12 +134,12 @@ JSRelativeTimeFormat::InitializeRelativeTimeFormat(
   //     ? Construct(%NumberFormat%, « nfLocale, nfOptions »).
   icu::NumberFormat* number_format =
       icu::NumberFormat::createInstance(icu_locale, UNUM_DECIMAL, status);
-  if (U_FAILURE(status) || number_format == nullptr) {
-    THROW_NEW_ERROR(
-        isolate,
-        NewRangeError(MessageTemplate::kRelativeDateTimeFormatterBadParameters),
-        JSRelativeTimeFormat);
+  if (U_FAILURE(status)) {
+    delete number_format;
+    FATAL("Failed to create ICU number format, are ICU data files missing?");
   }
+  CHECK_NOT_NULL(number_format);
+
   // 23. Perform ! CreateDataPropertyOrThrow(nfOptions, "useGrouping", false).
   number_format->setGroupingUsed(false);
 
@@ -155,13 +155,14 @@ JSRelativeTimeFormat::InitializeRelativeTimeFormat(
       new icu::RelativeDateTimeFormatter(icu_locale, number_format,
                                          getIcuStyle(style_enum),
                                          UDISPCTX_CAPITALIZATION_NONE, status);
-
-  if (U_FAILURE(status) || (icu_formatter == nullptr)) {
-    THROW_NEW_ERROR(
-        isolate,
-        NewRangeError(MessageTemplate::kRelativeDateTimeFormatterBadParameters),
-        JSRelativeTimeFormat);
+  if (U_FAILURE(status)) {
+    delete icu_formatter;
+    FATAL(
+        "Failed to create ICU relative date time formatter, are ICU data files "
+        "missing?");
   }
+  CHECK_NOT_NULL(icu_formatter);
+
   Handle<Managed<icu::RelativeDateTimeFormatter>> managed_formatter =
       Managed<icu::RelativeDateTimeFormatter>::FromRawPtr(isolate, 0,
                                                           icu_formatter);

src/runtime/runtime-intl.cc

@@ -169,8 +169,7 @@ RUNTIME_FUNCTION(Runtime_CreateDateTimeFormat) {
   // Set date time formatter as embedder field of the resulting JS object.
   icu::SimpleDateFormat* date_format =
       DateFormat::InitializeDateTimeFormat(isolate, locale, options, resolved);
-
-  if (!date_format) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(date_format);
 
   local_object->SetEmbedderField(0, reinterpret_cast<Smi*>(date_format));
 
@@ -228,8 +227,7 @@ RUNTIME_FUNCTION(Runtime_CreateNumberFormat) {
   // Set number formatter as embedder field of the resulting JS object.
   icu::DecimalFormat* number_format =
       NumberFormat::InitializeNumberFormat(isolate, locale, options, resolved);
-
-  if (!number_format) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(number_format);
 
   local_object->SetEmbedderField(NumberFormat::kDecimalFormatIndex,
                                  reinterpret_cast<Smi*>(number_format));
@@ -291,10 +289,13 @@ RUNTIME_FUNCTION(Runtime_CreateCollator) {
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, collator_holder,
                                      JSObject::New(constructor, constructor));
 
-  if (!Collator::InitializeCollator(isolate, collator_holder, locale, options,
-                                    resolved)) {
-    return isolate->ThrowIllegalOperation();
-  }
+  icu::Collator* collator =
+      Collator::InitializeCollator(isolate, locale, options, resolved);
+  CHECK_NOT_NULL(collator);
+
+  Handle<Managed<icu::Collator>> managed =
+      Managed<icu::Collator>::FromRawPtr(isolate, 0, collator);
+  collator_holder->SetEmbedderField(0, *managed);
 
   return *collator_holder;
 }
@@ -354,10 +355,10 @@ RUNTIME_FUNCTION(Runtime_CreatePluralRules) {
   // Set pluralRules as internal field of the resulting JS object.
   icu::PluralRules* plural_rules;
   icu::DecimalFormat* decimal_format;
-  bool success = PluralRules::InitializePluralRules(
-      isolate, locale, options, resolved, &plural_rules, &decimal_format);
-
-  if (!success) return isolate->ThrowIllegalOperation();
+  PluralRules::InitializePluralRules(isolate, locale, options, resolved,
+                                     &plural_rules, &decimal_format);
+  CHECK_NOT_NULL(plural_rules);
+  CHECK_NOT_NULL(decimal_format);
 
   local_object->SetEmbedderField(0, reinterpret_cast<Smi*>(plural_rules));
   local_object->SetEmbedderField(1, reinterpret_cast<Smi*>(decimal_format));
@@ -430,6 +431,7 @@ RUNTIME_FUNCTION(Runtime_CreateBreakIterator) {
   // Set break iterator as embedder field of the resulting JS object.
   icu::BreakIterator* break_iterator = V8BreakIterator::InitializeBreakIterator(
       isolate, locale, options, resolved);
+  CHECK_NOT_NULL(break_iterator);
 
   if (!break_iterator) return isolate->ThrowIllegalOperation();