[intl] Fix Intl Locale Info

1. Add test to ensure none of the array contains undefined
2. Calculate the fixed array size by considering the code may filter out
some itmes returned by ICU.
3. Add test to check there are no undefined.
4. Add test to check the locale.timeZones return sorted array.
5. Also refactor the JSArray generation code.

Bug: v8:11871
Change-Id: I8ad4a779d137d9b7e2deead7a1aa38e599e1af2e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2953517
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75162}

src/objects/js-locale.cc

@@ -460,32 +460,28 @@ MaybeHandle<JSLocale> JSLocale::Minimize(Isolate* isolate,
   return Construct(isolate, result);
 }
 
-template <typename T>
-MaybeHandle<JSArray> GetKeywordValuesFromLocale(
-    Isolate* isolate, const char* key, const char* unicode_key,
-    const icu::Locale& locale,
-    const std::map<std::string, std::string>& substitutions) {
-  Factory* factory = isolate->factory();
+MaybeHandle<JSArray> ToJSArray(
+    Isolate* isolate, icu::StringEnumeration* enumeration,
+    const std::map<std::string, std::string>& substitutions, bool may_remove) {
   UErrorCode status = U_ZERO_ERROR;
-  std::string ext =
-      locale.getUnicodeKeywordValue<std::string>(unicode_key, status);
-  if (!ext.empty()) {
-    Handle<FixedArray> fixed_array = factory->NewFixedArray(1);
-    Handle<String> str = factory->NewStringFromAsciiChecked(ext.c_str());
-    fixed_array->set(0, *str);
-    return factory->NewJSArrayWithElements(fixed_array);
+  Factory* factory = isolate->factory();
+
+  int32_t count = 0;
+  if (may_remove) {
+    // If we may remove items, then we need to go one pass first to count how
+    // many items we will insert before we allocate the fixed array.
+    for (const char* item = enumeration->next(nullptr, status);
+         U_SUCCESS(status) && item != nullptr;
+         item = enumeration->next(nullptr, status)) {
+      auto mapped = substitutions.find(item);
+      if ((mapped == substitutions.end()) ||
+          (*(mapped->second.c_str()) != '\0')) {
+        count++;
       }
-  status = U_ZERO_ERROR;
-  std::unique_ptr<icu::StringEnumeration> enumeration(
-      T::getKeywordValuesForLocale(key, locale, true, status));
-  if (U_FAILURE(status)) {
-    THROW_NEW_ERROR(isolate, NewRangeError(MessageTemplate::kIcuError),
-                    JSArray);
     }
-  int32_t count = enumeration->count(status);
-  if (U_FAILURE(status)) {
-    THROW_NEW_ERROR(isolate, NewRangeError(MessageTemplate::kIcuError),
-                    JSArray);
+    enumeration->reset(status);
+  } else {
+    count = enumeration->count(status);
   }
   Handle<FixedArray> fixed_array = factory->NewFixedArray(count);
 
@@ -503,6 +499,7 @@ MaybeHandle<JSArray> GetKeywordValuesFromLocale(
     Handle<String> str = factory->NewStringFromAsciiChecked(item);
     fixed_array->set(index++, *str);
   }
+  CHECK(index == count);
   if (U_FAILURE(status)) {
     THROW_NEW_ERROR(isolate, NewRangeError(MessageTemplate::kIcuError),
                     JSArray);
@@ -510,13 +507,38 @@ MaybeHandle<JSArray> GetKeywordValuesFromLocale(
   return factory->NewJSArrayWithElements(fixed_array);
 }
 
+template <typename T>
+MaybeHandle<JSArray> GetKeywordValuesFromLocale(
+    Isolate* isolate, const char* key, const char* unicode_key,
+    const icu::Locale& locale,
+    const std::map<std::string, std::string>& substitutions, bool may_remove) {
+  Factory* factory = isolate->factory();
+  UErrorCode status = U_ZERO_ERROR;
+  std::string ext =
+      locale.getUnicodeKeywordValue<std::string>(unicode_key, status);
+  if (!ext.empty()) {
+    Handle<FixedArray> fixed_array = factory->NewFixedArray(1);
+    Handle<String> str = factory->NewStringFromAsciiChecked(ext.c_str());
+    fixed_array->set(0, *str);
+    return factory->NewJSArrayWithElements(fixed_array);
+  }
+  status = U_ZERO_ERROR;
+  std::unique_ptr<icu::StringEnumeration> enumeration(
+      T::getKeywordValuesForLocale(key, locale, true, status));
+  if (U_FAILURE(status)) {
+    THROW_NEW_ERROR(isolate, NewRangeError(MessageTemplate::kIcuError),
+                    JSArray);
+  }
+  return ToJSArray(isolate, enumeration.get(), substitutions, may_remove);
+}
+
 MaybeHandle<JSArray> JSLocale::Calendars(Isolate* isolate,
                                          Handle<JSLocale> locale) {
   icu::Locale icu_locale(*(locale->icu_locale().raw()));
   const std::map<std::string, std::string> substitutions(
       {{"gregorian", "gregory"}, {"ethiopic-amete-alem", "ethioaa"}});
-  return GetKeywordValuesFromLocale<icu::Calendar>(isolate, "calendar", "ca",
-                                                   icu_locale, substitutions);
+  return GetKeywordValuesFromLocale<icu::Calendar>(
+      isolate, "calendar", "ca", icu_locale, substitutions, false);
 }
 
 MaybeHandle<JSArray> JSLocale::Collations(Isolate* isolate,
@@ -524,8 +546,8 @@ MaybeHandle<JSArray> JSLocale::Collations(Isolate* isolate,
   icu::Locale icu_locale(*(locale->icu_locale().raw()));
   const std::map<std::string, std::string> substitutions(
       {{"standard", ""}, {"search", ""}});
-  return GetKeywordValuesFromLocale<icu::Collator>(isolate, "collations", "co",
-                                                   icu_locale, substitutions);
+  return GetKeywordValuesFromLocale<icu::Collator>(
+      isolate, "collations", "co", icu_locale, substitutions, true);
 }
 
 MaybeHandle<JSArray> JSLocale::HourCycles(Isolate* isolate,
@@ -646,7 +668,6 @@ MaybeHandle<Object> JSLocale::TimeZones(Isolate* isolate,
   // Let list be a List of 1 or more time zone identifiers, which must be String
   // values indicating a Zone or Link name of the IANA Time Zone Database,
   // sorted in descending preference of those in common use in region.
-  int32_t index = 0;
   UErrorCode status = U_ZERO_ERROR;
   std::unique_ptr<icu::StringEnumeration> enumeration(
       icu::TimeZone::createTimeZoneIDEnumeration(UCAL_ZONE_TYPE_CANONICAL,
@@ -655,26 +676,8 @@ MaybeHandle<Object> JSLocale::TimeZones(Isolate* isolate,
     THROW_NEW_ERROR(isolate, NewRangeError(MessageTemplate::kIcuError),
                     JSArray);
   }
-  int32_t count = enumeration->count(status);
-  if (U_FAILURE(status)) {
-    THROW_NEW_ERROR(isolate, NewRangeError(MessageTemplate::kIcuError),
-                    JSArray);
-  }
-
-  // Return CreateArrayFromList( list ).
-  Handle<FixedArray> fixed_array = factory->NewFixedArray(count);
-  for (const char* item = enumeration->next(nullptr, status);
-       U_SUCCESS(status) && item != nullptr;
-       item = enumeration->next(nullptr, status)) {
-    Handle<String> str = isolate->factory()->NewStringFromAsciiChecked(item);
-    fixed_array->set(index++, *str);
-  }
-  if (U_FAILURE(status)) {
-    THROW_NEW_ERROR(isolate, NewRangeError(MessageTemplate::kIcuError),
-                    JSArray);
-  }
-
-  return factory->NewJSArrayWithElements(fixed_array);
+  const std::map<std::string, std::string> substitutions({});
+  return ToJSArray(isolate, enumeration.get(), substitutions, false);
 }
 
 MaybeHandle<JSObject> JSLocale::TextInfo(Isolate* isolate,

test/intl/assert.js

@@ -141,6 +141,15 @@ function assertNotNull(value, user_message = '') {
   }
 }
 
+/**
+ * Throws if value is undefined.
+ */
+function assertNotUndefined(value, user_message = '') {
+  if (value === undefined) {
+    fail("not undefined", value, user_message);
+  }
+}
+
 
 /**
  * Runs code() and asserts that it throws the specified exception.

test/intl/locale/locale-info-no-undefined.js

+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony_intl_locale_info
+
+// Check the return array has no undefined
+function checkNoUndefined(l, items) {
+  items.forEach( function(item) {
+    assertNotUndefined(item, l + ": [" +items + "] should not have undefined");
+  });
+}
+function checkLocale(locale) {
+  let l = new Intl.Locale(locale)
+  checkNoUndefined(l, l.calendars);
+  checkNoUndefined(l, l.collations);
+  checkNoUndefined(l, l.hourCycles);
+  checkNoUndefined(l, l.numberingSystems);
+  if (l.region != undefined) {
+    checkNoUndefined(l, l.timeZones);
+  }
+}
+
+checkLocale("ar");
+checkLocale("en");
+checkLocale("fr");
+checkLocale("en-GB");
+checkLocale("en-US");
+checkLocale("zh-TW");
+checkLocale("zh-CN");

test/intl/locale/locale-info-timezones-sorted.js

+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony_intl_locale_info
+
+// Check the return array are sorted
+function checkSortedArray(l, name, items) {
+  assertEquals([...items].sort(), items,
+      "return value of " + l + "." + name + "should be sorted");
+}
+function checkLocale(locale) {
+  let l = new Intl.Locale(locale)
+  checkSortedArray(l, "timeZones", l.timeZones);
+}
+
+checkLocale("ar-EG");
+checkLocale("fr-FR");
+checkLocale("en-GB");
+checkLocale("en-US");
+checkLocale("en-AU");
+checkLocale("en-CA");
+checkLocale("zh-TW");
+checkLocale("zh-CN");
+checkLocale("ja-JP");
+checkLocale("in-IN");
+checkLocale("ru-RU");