[intl] Add GetOption

This patch ports over the spec defined operation `GetOption` from
JavaScript to C++:
https://tc39.github.io/ecma402/#sec-getoption

The JS implementation will be deleted once all it's
users are migrated.

Refactors LocaleConstructor to use this method which fixes some test262
tests. The test262 test status file is updated to reflect this.

Bug: v8:5751, v8:7684
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Ief5eae9b69dcea50062825163ca7658ed20bd0cf
Reviewed-on: https://chromium-review.googlesource.com/1094201
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53652}

src/objects.cc

@@ -18859,6 +18859,57 @@ MaybeHandle<Name> FunctionTemplateInfo::TryGetCachedPropertyName(
   return MaybeHandle<Name>();
 }
 
+#ifdef V8_INTL_SUPPORT
+MaybeHandle<Object> Object::GetOption(
+    Isolate* isolate, Handle<JSReceiver> options, Handle<Name> property,
+    Object::OptionType type, Handle<FixedArray> values, Handle<Object> fallback,
+    Handle<String> services) {
+  // 1. Let value be ? Get(options, property).
+  Handle<Object> value;
+  ASSIGN_RETURN_ON_EXCEPTION(
+      isolate, value, Object::GetPropertyOrElement(options, property), Object);
+
+  // 2. If value is not undefined, then
+  if (!value->IsUndefined(isolate)) {
+    // 2. b. If type is "boolean", then
+    if (type == Object::OptionType::Boolean) {
+      // 2. b. i. Let value be ToBoolean(value).
+      value = isolate->factory()->ToBoolean(value->BooleanValue(isolate));
+    }
+
+    // 2. c. If type is "string", then
+    if (type == Object::OptionType::String) {
+      // 2. c. i. Let value be ? ToString(value).
+      ASSIGN_RETURN_ON_EXCEPTION(isolate, value,
+                                 Object::ToString(isolate, value), Object);
+    }
+
+    // 2. d. if values is not undefined, then
+    if (values->length() > 0) {
+      // 2. d. i. If values does not contain an element equal to value,
+      // throw a RangeError exception.
+      for (int i = 0; i < values->length(); i++) {
+        if (value->StrictEquals(values->get(i))) {
+          // 2. e. return value
+          return value;
+        }
+      }
+
+      THROW_NEW_ERROR(isolate,
+                      NewRangeError(MessageTemplate::kValueOutOfRange, value,
+                                    services, property),
+                      Object);
+    }
+
+    // 2. e. return value
+    return value;
+  }
+
+  // 3. Else, return fallback.
+  return fallback;
+}
+#endif  // V8_INTL_SUPPORT
+
 // Force instantiation of template instances class.
 // Please note this list is compiler dependent.
 // Keep this at the end of this file

src/objects.h

@@ -1540,6 +1540,23 @@ class Object {
   // and length.
   bool IterationHasObservableEffects();
 
+  enum class OptionType : bool { String, Boolean };
+
+#ifdef V8_INTL_SUPPORT
+  // ECMA402 9.2.10. GetOption( options, property, type, type, values, fallback)
+  // ecma402/#sec-getoption
+  //
+  // Instead of passing undefined for the values argument as the spec
+  // defines, pass in an empty fixed array.
+  //
+  // service is a string denoting the type of Intl object; used when
+  // printing the error message.
+  V8_WARN_UNUSED_RESULT static MaybeHandle<Object> GetOption(
+      Isolate* isolate, Handle<JSReceiver> options, Handle<Name> property,
+      Object::OptionType type, Handle<FixedArray> values,
+      Handle<Object> fallback, Handle<String> service);
+#endif  // V8_INTL_SUPPORT
+
   DECL_VERIFIER(Object)
 #ifdef VERIFY_HEAP
   // Verify a pointer is a valid object pointer.

src/objects/js-locale.cc

@@ -31,30 +31,6 @@ namespace v8 {
 namespace internal {
 
 namespace {
-
-// Extracts value of a given property key in the Object.
-Maybe<bool> ExtractStringSetting(Isolate* isolate, Handle<JSReceiver> options,
-                                 const char* key, icu::UnicodeString* setting) {
-  v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
-  Handle<String> str = isolate->factory()->NewStringFromAsciiChecked(key);
-
-  // JSReceiver::GetProperty could throw an exception and return an empty
-  // MaybeHandle<Object>().
-  // Returns Nothing<bool> on exception.
-  Handle<Object> object;
-  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
-      isolate, object, JSReceiver::GetProperty(options, str), Nothing<bool>());
-
-  if (object->IsString()) {
-    v8::String::Utf8Value utf8_string(
-        v8_isolate, v8::Utils::ToLocal(Handle<String>::cast(object)));
-    *setting = icu::UnicodeString::fromUTF8(*utf8_string);
-    return Just(true);
-  }
-
-  return Just(false);
-}
-
 // Inserts tags from options into locale string.
 Maybe<bool> InsertOptionsIntoLocale(Isolate* isolate,
                                     Handle<JSReceiver> options,
@@ -70,22 +46,29 @@ Maybe<bool> InsertOptionsIntoLocale(Isolate* isolate,
                                  {"numeric", "kn"},
                                  {"numberingSystem", "nu"}}};
 
+  Handle<Object> undefined = isolate->factory()->undefined_value();
+  Handle<FixedArray> empty_array = isolate->factory()->empty_fixed_array();
+  Handle<String> service =
+      isolate->factory()->NewStringFromAsciiChecked("locale");
+
   for (const auto& option_to_bcp47 : kOptionToUnicodeTagMap) {
-    UErrorCode status = U_ZERO_ERROR;
-    icu::UnicodeString value_unicode;
+    Handle<String> key_str =
+        isolate->factory()->NewStringFromAsciiChecked(option_to_bcp47.first);
 
-    Maybe<bool> found = ExtractStringSetting(
-        isolate, options, option_to_bcp47.first, &value_unicode);
-    // Return on exception.
-    MAYBE_RETURN(found, Nothing<bool>());
-    if (!found.FromJust()) {
+    Handle<Object> value_obj;
+    ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+        isolate, value_obj,
+        Object::GetOption(isolate, options, key_str, Object::OptionType::String,
+                          empty_array, undefined, service),
+        Nothing<bool>());
+
+    if (value_obj->IsUndefined(isolate)) {
       // Skip this key, user didn't specify it in options.
       continue;
     }
-    DCHECK(found.FromJust());
 
-    std::string value_string;
-    value_unicode.toUTF8String(value_string);
+    Handle<String> value_str = Handle<String>::cast(value_obj);
+    std::unique_ptr<char[]> value_c_str = value_str->ToCString();
 
     // Convert bcp47 key and value into legacy ICU format so we can use
     // uloc_setKeywordValue.
@@ -93,7 +76,8 @@ Maybe<bool> InsertOptionsIntoLocale(Isolate* isolate,
     if (!key) return Just(false);
 
     // Overwrite existing, or insert new key-value to the locale string.
-    const char* value = uloc_toLegacyType(key, value_string.c_str());
+    const char* value = uloc_toLegacyType(key, value_c_str.get());
+    UErrorCode status = U_ZERO_ERROR;
     if (value) {
       // TODO(cira): ICU puts artificial limit on locale length, while BCP47
       // doesn't. Switch to C++ API when it's ready.

test/cctest/test-intl.cc

@@ -5,6 +5,8 @@
 #ifdef V8_INTL_SUPPORT
 
 #include "src/builtins/builtins-intl.h"
+#include "src/lookup.h"
+#include "src/objects-inl.h"
 #include "test/cctest/cctest.h"
 
 namespace v8 {
@@ -113,6 +115,82 @@ TEST(FlattenRegionsToParts) {
       });
 }
 
+TEST(GetOptions) {
+  LocalContext env;
+  Isolate* isolate = CcTest::i_isolate();
+  v8::Isolate* v8_isolate = env->GetIsolate();
+  v8::HandleScope handle_scope(v8_isolate);
+
+  Handle<JSObject> options = isolate->factory()->NewJSObjectWithNullProto();
+  Handle<String> key = isolate->factory()->NewStringFromAsciiChecked("foo");
+  Handle<String> service =
+      isolate->factory()->NewStringFromAsciiChecked("service");
+  Handle<Object> undefined = isolate->factory()->undefined_value();
+  Handle<FixedArray> empty_fixed_array =
+      isolate->factory()->empty_fixed_array();
+
+  // No value found
+  Handle<Object> result =
+      Object::GetOption(isolate, options, key, Object::OptionType::String,
+                        empty_fixed_array, undefined, service)
+          .ToHandleChecked();
+  CHECK(result->IsUndefined(isolate));
+
+  // Value found
+  v8::internal::LookupIterator it(options, key);
+  CHECK(Object::SetProperty(&it, Handle<Smi>(Smi::FromInt(42), isolate),
+                            LanguageMode::kStrict,
+                            AllocationMemento::MAY_BE_STORE_FROM_KEYED)
+            .FromJust());
+  result = Object::GetOption(isolate, options, key, Object::OptionType::String,
+                             empty_fixed_array, undefined, service)
+               .ToHandleChecked();
+  CHECK(result->IsString());
+  Handle<String> expected_str =
+      isolate->factory()->NewStringFromAsciiChecked("42");
+  CHECK(String::Equals(expected_str, Handle<String>::cast(result)));
+
+  result = Object::GetOption(isolate, options, key, Object::OptionType::Boolean,
+                             empty_fixed_array, undefined, service)
+               .ToHandleChecked();
+  CHECK(result->IsBoolean());
+  CHECK(result->IsTrue(isolate));
+
+  // No expected value in values array
+  Handle<FixedArray> values = isolate->factory()->NewFixedArray(1);
+  {
+    CHECK(!Object::GetOption(isolate, options, key, Object::OptionType::String,
+                             values, undefined, service)
+               .ToHandle(&result));
+    CHECK(isolate->has_pending_exception());
+    isolate->clear_pending_exception();
+  }
+
+  // Add expected value to values array
+  values->set(0, *expected_str);
+  result = Object::GetOption(isolate, options, key, Object::OptionType::String,
+                             values, undefined, service)
+               .ToHandleChecked();
+  CHECK(result->IsString());
+  CHECK(String::Equals(expected_str, Handle<String>::cast(result)));
+
+  // Test boolean values
+  CHECK(Object::SetProperty(&it, isolate->factory()->ToBoolean(false),
+                            LanguageMode::kStrict,
+                            AllocationMemento::MAY_BE_STORE_FROM_KEYED)
+            .FromJust());
+  result = Object::GetOption(isolate, options, key, Object::OptionType::String,
+                             empty_fixed_array, undefined, service)
+               .ToHandleChecked();
+  CHECK(result->IsString());
+
+  result = Object::GetOption(isolate, options, key, Object::OptionType::Boolean,
+                             empty_fixed_array, undefined, service)
+               .ToHandleChecked();
+  CHECK(result->IsBoolean());
+  CHECK(result->IsFalse(isolate));
+}
+
 }  // namespace internal
 }  // namespace v8
 

test/test262/test262.status

@@ -453,10 +453,8 @@
   'intl402/Locale/constructor-locale-object': [FAIL],
   'intl402/Locale/constructor-non-iana-canon': [FAIL],
   'intl402/Locale/constructor-options-casefirst-invalid': [FAIL],
-  'intl402/Locale/constructor-options-casefirst-valid': [FAIL],
   'intl402/Locale/constructor-options-collation-valid': [FAIL],
   'intl402/Locale/constructor-options-hourcycle-invalid': [FAIL],
-  'intl402/Locale/constructor-options-hourcycle-valid': [FAIL],
   'intl402/Locale/constructor-options-language-grandfathered': [FAIL],
   'intl402/Locale/constructor-options-language-invalid': [FAIL],
   'intl402/Locale/constructor-options-language-valid': [FAIL],