[Intl] Clean up SupportedLocalesOf and GetAvailableLocales

As per
https://unicode-org.atlassian.net/browse/ICU-20009?focusedCommentId=62380&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-62380
we can just use DateFormat::GetAvailableLocales for RelativeTimeFormat
removing a lot of code.

This patch also cleans up the code to be in line with the rest of V8.

Bug: v8:5751
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: I8df5b0e5a9a05c426aaa4f7fb9c67d7947301478
Reviewed-on: https://chromium-review.googlesource.com/c/1237298
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56455}

src/builtins/builtins-intl.cc

@@ -130,44 +130,24 @@ BUILTIN(StringPrototypeNormalizeIntl) {
                    result.length())));
 }
 
-namespace {
-
-MaybeHandle<JSObject> SupportedLocalesOfCommon(Isolate* isolate,
-                                               const char* service_in,
-                                               BuiltinArguments args) {
-  Factory* factory = isolate->factory();
-  Handle<String> service = factory->NewStringFromAsciiChecked(service_in);
-  Handle<Object> locales = args.atOrUndefined(isolate, 1);
-  Handle<Object> options = args.atOrUndefined(isolate, 2);
-
-  MaybeHandle<JSObject> result =
-      Intl::SupportedLocalesOf(isolate, service, locales, options);
-  Handle<JSObject> elements;
-  ASSIGN_RETURN_ON_EXCEPTION(isolate, elements, result, JSObject);
-  return elements;
-}
-
-}  // namespace
-
 BUILTIN(V8BreakIteratorSupportedLocalesOf) {
   HandleScope scope(isolate);
-  // 1. If NewTarget is defined, throw a TypeError exception.
-  if (!args.new_target()->IsUndefined(isolate)) {  // [[Call]]
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate,
-        NewTypeError(MessageTemplate::kOrdinaryFunctionCalledAsConstructor,
-                     isolate->factory()->NewStringFromStaticChars(
-                         "Intl.v8BreakIterator")));
-  }
+  Handle<Object> locales = args.atOrUndefined(isolate, 1);
+  Handle<Object> options = args.atOrUndefined(isolate, 2);
 
   RETURN_RESULT_OR_FAILURE(
-      isolate, SupportedLocalesOfCommon(isolate, "breakiterator", args));
+      isolate, Intl::SupportedLocalesOf(isolate, ICUService::kBreakIterator,
+                                        locales, options));
 }
 
 BUILTIN(NumberFormatSupportedLocalesOf) {
   HandleScope scope(isolate);
+  Handle<Object> locales = args.atOrUndefined(isolate, 1);
+  Handle<Object> options = args.atOrUndefined(isolate, 2);
+
   RETURN_RESULT_OR_FAILURE(
-      isolate, SupportedLocalesOfCommon(isolate, "numberformat", args));
+      isolate, Intl::SupportedLocalesOf(isolate, ICUService::kNumberFormat,
+                                        locales, options));
 }
 
 BUILTIN(NumberFormatPrototypeFormatToParts) {
@@ -204,8 +184,12 @@ BUILTIN(DateTimeFormatPrototypeResolvedOptions) {
 
 BUILTIN(DateTimeFormatSupportedLocalesOf) {
   HandleScope scope(isolate);
+  Handle<Object> locales = args.atOrUndefined(isolate, 1);
+  Handle<Object> options = args.atOrUndefined(isolate, 2);
+
   RETURN_RESULT_OR_FAILURE(
-      isolate, SupportedLocalesOfCommon(isolate, "dateformat", args));
+      isolate, Intl::SupportedLocalesOf(isolate, ICUService::kDateFormat,
+                                        locales, options));
 }
 
 BUILTIN(DateTimeFormatPrototypeFormatToParts) {
@@ -543,8 +527,12 @@ BUILTIN(ListFormatPrototypeResolvedOptions) {
 
 BUILTIN(ListFormatSupportedLocalesOf) {
   HandleScope scope(isolate);
+  Handle<Object> locales = args.atOrUndefined(isolate, 1);
+  Handle<Object> options = args.atOrUndefined(isolate, 2);
+
   RETURN_RESULT_OR_FAILURE(
-      isolate, SupportedLocalesOfCommon(isolate, "listformat", args));
+      isolate, Intl::SupportedLocalesOf(isolate, ICUService::kListFormatter,
+                                        locales, options));
 }
 
 namespace {
@@ -646,8 +634,13 @@ BUILTIN(LocalePrototypeMinimize) {
 
 BUILTIN(RelativeTimeFormatSupportedLocalesOf) {
   HandleScope scope(isolate);
+  Handle<Object> locales = args.atOrUndefined(isolate, 1);
+  Handle<Object> options = args.atOrUndefined(isolate, 2);
+
   RETURN_RESULT_OR_FAILURE(
-      isolate, SupportedLocalesOfCommon(isolate, "relativetimeformat", args));
+      isolate,
+      Intl::SupportedLocalesOf(isolate, ICUService::kRelativeDateTimeFormatter,
+                               locales, options));
 }
 
 BUILTIN(RelativeTimeFormatPrototypeFormat) {
@@ -905,8 +898,12 @@ BUILTIN(PluralRulesPrototypeSelect) {
 
 BUILTIN(PluralRulesSupportedLocalesOf) {
   HandleScope scope(isolate);
+  Handle<Object> locales = args.atOrUndefined(isolate, 1);
+  Handle<Object> options = args.atOrUndefined(isolate, 2);
+
   RETURN_RESULT_OR_FAILURE(
-      isolate, SupportedLocalesOfCommon(isolate, "pluralrules", args));
+      isolate, Intl::SupportedLocalesOf(isolate, ICUService::kPluralRules,
+                                        locales, options));
 }
 
 BUILTIN(CollatorConstructor) {
@@ -951,8 +948,12 @@ BUILTIN(CollatorPrototypeResolvedOptions) {
 
 BUILTIN(CollatorSupportedLocalesOf) {
   HandleScope scope(isolate);
-  RETURN_RESULT_OR_FAILURE(isolate,
-                           SupportedLocalesOfCommon(isolate, "collator", args));
+  Handle<Object> locales = args.atOrUndefined(isolate, 1);
+  Handle<Object> options = args.atOrUndefined(isolate, 2);
+
+  RETURN_RESULT_OR_FAILURE(
+      isolate, Intl::SupportedLocalesOf(isolate, ICUService::kCollator, locales,
+                                        options));
 }
 
 BUILTIN(CollatorPrototypeCompare) {
@@ -1047,8 +1048,12 @@ BUILTIN(SegmenterConstructor) {
 
 BUILTIN(SegmenterSupportedLocalesOf) {
   HandleScope scope(isolate);
+  Handle<Object> locales = args.atOrUndefined(isolate, 1);
+  Handle<Object> options = args.atOrUndefined(isolate, 2);
+
   RETURN_RESULT_OR_FAILURE(
-      isolate, SupportedLocalesOfCommon(isolate, "segmenter", args));
+      isolate, Intl::SupportedLocalesOf(isolate, ICUService::kSegmenter,
+                                        locales, options));
 }
 
 BUILTIN(SegmenterPrototypeResolvedOptions) {

src/intl.h

@@ -23,13 +23,12 @@ class TimeZone;
 namespace v8 {
 namespace internal {
 
-enum class IcuService {
+enum class ICUService {
   kBreakIterator,
   kCollator,
   kDateFormat,
   kNumberFormat,
   kPluralRules,
-  kResourceBundle,
   kRelativeDateTimeFormatter,
   kListFormatter,
   kSegmenter

src/objects/intl-objects.h

@@ -61,13 +61,11 @@ class Intl {
   static bool IsObjectOfType(Isolate* isolate, Handle<Object> object,
                              Intl::Type expected_type);
 
-  static IcuService StringToIcuService(Handle<String> service);
-
   // Gets the ICU locales for a given service. If there is a locale with a
   // script tag then the locales also include a locale without the script; eg,
   // pa_Guru_IN (language=Panjabi, script=Gurmukhi, country-India) would include
   // pa_IN.
-  static std::set<std::string> GetAvailableLocales(const IcuService& service);
+  static std::set<std::string> GetAvailableLocales(ICUService service);
 
   // Get the name of the numbering system from locale.
   // ICU doesn't expose numbering system in any way, so we have to assume that
@@ -78,18 +76,12 @@ class Intl {
   static V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> AvailableLocalesOf(
       Isolate* isolate, Handle<String> service);
 
-  static MaybeHandle<JSObject> SupportedLocalesOf(Isolate* isolate,
-                                                  Handle<String> service,
-                                                  Handle<Object> locales_in,
+  static V8_WARN_UNUSED_RESULT MaybeHandle<JSObject> SupportedLocalesOf(
+      Isolate* isolate, ICUService service, Handle<Object> locales_in,
       Handle<Object> options_in);
 
   static std::string DefaultLocale(Isolate* isolate);
 
-  // If locale has a script tag then return true and the locale without the
-  // script else return false and an empty string
-  static bool RemoveLocaleScriptTag(const std::string& icu_locale,
-                                    std::string* locale_less_script);
-
   // The ResolveLocale abstract operation compares a BCP 47 language
   // priority list requestedLocales against the locales in
   // availableLocales and determines the best available language to

test/cctest/test-intl.cc

@@ -208,52 +208,26 @@ TEST(GetBoolOption) {
   }
 }
 
-bool ScriptTagWasRemoved(std::string locale, std::string expected) {
-  std::string without_script_tag;
-  bool didShorten = Intl::RemoveLocaleScriptTag(locale, &without_script_tag);
-  return didShorten && expected == without_script_tag;
-}
-
-bool ScriptTagWasNotRemoved(std::string locale) {
-  std::string without_script_tag;
-  bool didShorten = Intl::RemoveLocaleScriptTag(locale, &without_script_tag);
-  return !didShorten && without_script_tag.empty();
-}
-
-TEST(RemoveLocaleScriptTag) {
-  CHECK(ScriptTagWasRemoved("aa_Bbbb_CC", "aa_CC"));
-  CHECK(ScriptTagWasRemoved("aaa_Bbbb_CC", "aaa_CC"));
-
-  CHECK(ScriptTagWasNotRemoved("aa"));
-  CHECK(ScriptTagWasNotRemoved("aaa"));
-  CHECK(ScriptTagWasNotRemoved("aa_CC"));
-  CHECK(ScriptTagWasNotRemoved("aa_Bbb_CC"));
-  CHECK(ScriptTagWasNotRemoved("aa_1bbb_CC"));
-}
-
 TEST(GetAvailableLocales) {
   std::set<std::string> locales;
 
-  locales = Intl::GetAvailableLocales(IcuService::kBreakIterator);
+  locales = Intl::GetAvailableLocales(ICUService::kBreakIterator);
   CHECK(locales.count("en-US"));
   CHECK(!locales.count("abcdefg"));
 
-  locales = Intl::GetAvailableLocales(IcuService::kCollator);
-  CHECK(locales.count("en-US"));
-
-  locales = Intl::GetAvailableLocales(IcuService::kDateFormat);
+  locales = Intl::GetAvailableLocales(ICUService::kCollator);
   CHECK(locales.count("en-US"));
 
-  locales = Intl::GetAvailableLocales(IcuService::kNumberFormat);
+  locales = Intl::GetAvailableLocales(ICUService::kDateFormat);
   CHECK(locales.count("en-US"));
 
-  locales = Intl::GetAvailableLocales(IcuService::kPluralRules);
+  locales = Intl::GetAvailableLocales(ICUService::kNumberFormat);
   CHECK(locales.count("en-US"));
 
-  locales = Intl::GetAvailableLocales(IcuService::kResourceBundle);
+  locales = Intl::GetAvailableLocales(ICUService::kPluralRules);
   CHECK(locales.count("en-US"));
 
-  locales = Intl::GetAvailableLocales(IcuService::kRelativeDateTimeFormatter);
+  locales = Intl::GetAvailableLocales(ICUService::kRelativeDateTimeFormatter);
   CHECK(locales.count("en-US"));
 }