Fix locale of Intl.Collator..resolvedOptions

Bug: v8:7481, v8:9084, v8:8664
Change-Id: Iccbf78bf11a4e8ca5d105772fa5f654fbe6542cd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2410791
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69951}

src/objects/intl-objects.tq

@@ -154,4 +154,5 @@ extern class JSV8BreakIterator extends JSObject {
 extern class JSCollator extends JSObject {
   icu_collator: Foreign;  // Managed<icu::Collator>
   bound_compare: Undefined|JSFunction;
+  locale: String;
 }

src/objects/js-collator.cc

@@ -196,8 +196,26 @@ Handle<JSObject> JSCollator::ResolvedOptions(Isolate* isolate,
   //    [[Collation]]            "collation"
   //    [[Numeric]]              "numeric"              kn
   //    [[CaseFirst]]            "caseFirst"            kf
-  CreateDataPropertyForOptions(
-      isolate, options, isolate->factory()->locale_string(), locale.c_str());
+
+  // If the collator return the locale differ from what got requested, we stored
+  // it in the collator->locale. Otherwise, we just use the one from the
+  // collator.
+  if (collator->locale().length() != 0) {
+    // Get the locale from collator->locale() since we know in some cases
+    // collator won't be able to return the requested one, such as zh_CN.
+    Handle<String> locale_from_collator(collator->locale(), isolate);
+    Maybe<bool> maybe = JSReceiver::CreateDataProperty(
+        isolate, options, isolate->factory()->locale_string(),
+        locale_from_collator, Just(kDontThrow));
+    DCHECK(maybe.FromJust());
+    USE(maybe);
+  } else {
+    // Just return from the collator for most of the cases that we can recover
+    // from the collator.
+    CreateDataPropertyForOptions(
+        isolate, options, isolate->factory()->locale_string(), locale.c_str());
+  }
+
   CreateDataPropertyForOptions(isolate, options,
                                isolate->factory()->usage_string(), usage);
   CreateDataPropertyForOptions(
@@ -424,6 +442,9 @@ MaybeHandle<JSCollator> JSCollator::New(Isolate* isolate, Handle<Map> map,
   }
   DCHECK(U_SUCCESS(status));
 
+  icu::Locale collator_locale(
+      icu_collator->getLocale(ULOC_VALID_LOCALE, status));
+
   // 22. If relevantExtensionKeys contains "kn", then
   //     a. Set collator.[[Numeric]] to ! SameValue(r.[[kn]], "true").
   //
@@ -524,8 +545,12 @@ MaybeHandle<JSCollator> JSCollator::New(Isolate* isolate, Handle<Map> map,
   // Now all properties are ready, so we can allocate the result object.
   Handle<JSCollator> collator = Handle<JSCollator>::cast(
       isolate->factory()->NewFastOrSlowJSObjectFromMap(map));
+  // We only need to do so if it is different from the collator would return.
+  Handle<String> locale_str = isolate->factory()->NewStringFromAsciiChecked(
+      (collator_locale != icu_locale) ? r.locale.c_str() : "");
   DisallowHeapAllocation no_gc;
   collator->set_icu_collator(*managed_collator);
+  collator->set_locale(*locale_str);
 
   // 29. Return collator.
   return collator;

test/intl/intl.status

@@ -29,10 +29,6 @@
 [ALWAYS, {
 # TODO(jochen): The following test is flaky.
   'overrides/caching': [PASS, FAIL],
-
-  # https://code.google.com/p/v8/issues/detail?id=7481
-  'collator/check-kf-option': [FAIL],
-  'collator/check-kn-option': [FAIL],
 }],  # ALWAYS
 
 ['variant == no_wasm_traps', {

test/intl/regress-9084.js

+// Copyright 2020 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.
+
+["zh", "zh-CN", "zh-Hans", "zh-Hans-CN"].forEach((l) => {
+  assertEquals(
+      l,
+      (new Intl.Collator(
+          l, {localeMatcher: "lookup"} )).resolvedOptions().locale);
+});

test/mjsunit/regress/regress-6288.js

@@ -5,9 +5,9 @@
 // Environment Variables: LC_ALL=pt-BR.UTF8
 
 // The data files packaged with d8 currently have Brazillian Portuguese
-// DateTimeFormat but not Collation
+// DateTimeFormat and Collation
 
 if (this.Intl) {
-  assertEquals('pt', Intl.Collator().resolvedOptions().locale);
+  assertEquals('pt-BR', Intl.Collator().resolvedOptions().locale);
   assertEquals('pt-BR', Intl.DateTimeFormat().resolvedOptions().locale);
 }

test/test262/test262.status

@@ -62,10 +62,6 @@
   # https://bugs.chromium.org/p/v8/issues/detail?id=4709
   'language/expressions/assignment/fn-name-lhs-cover': [FAIL],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=9084
-  'intl402/supportedLocalesOf-consistent-with-resolvedOptions': [FAIL],
-  'intl402/fallback-locales-are-supported': [FAIL],
-
   # https://code.google.com/p/v8/issues/detail?id=4251
   'language/expressions/postfix-increment/S11.3.1_A5_T1': [FAIL],
   'language/expressions/postfix-increment/S11.3.1_A5_T2': [FAIL],
@@ -447,9 +443,6 @@
   'language/expressions/call/eval-spread-empty-leading': [FAIL],
   'language/expressions/call/eval-spread-empty-trailing': [FAIL],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=8664
-  'intl402/Collator/missing-unicode-ext-value-defaults-to-true': [FAIL],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=7472
   'intl402/NumberFormat/currency-digits': [FAIL],