Revert "[Intl] move localeCompare to C++"

This reverts commit 51ad234f.

Reason for revert: Speculative revert. Suspect for layout test change:
https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8-Blink%20Linux%2064/25165

See also:
https://github.com/v8/v8/wiki/Blink-layout-tests

The bots currently have another problem with webkit_unit_tests and
don't run the layout tests on every revision. Hence the speculative
revert.

Original change's description:
> [Intl] move localeCompare to C++
> 
> 
> Bug: v8:7958
> Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
> Change-Id: I84a27dda5205c8581a7ffe37213d685cc49974fa
> Reviewed-on: https://chromium-review.googlesource.com/1144644
> Commit-Queue: Frank Tang <ftang@chromium.org>
> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#54740}

TBR=jshin@chromium.org,gsathya@chromium.org,bstell.chromium.org@gmail.com,ftang@chromium.org

Change-Id: Iceab3316bd1eacfe449c8182cbb3027acb56047e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7958
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1152767Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54747}

src/bootstrapper.cc

@@ -2030,13 +2030,8 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
                           Builtins::kStringPrototypeLastIndexOf, 1, false);
     SimpleInstallFunction(isolate_, prototype, "link",
                           Builtins::kStringPrototypeLink, 1, true);
-#ifdef V8_INTL_SUPPORT
-    SimpleInstallFunction(isolate_, prototype, "localeCompare",
-                          Builtins::kStringPrototypeLocaleCompare, 1, false);
-#else
     SimpleInstallFunction(isolate_, prototype, "localeCompare",
                           Builtins::kStringPrototypeLocaleCompare, 1, true);
-#endif  // V8_INTL_SUPPORT
     SimpleInstallFunction(isolate_, prototype, "match",
                           Builtins::kStringPrototypeMatch, 1, true);
 #ifdef V8_INTL_SUPPORT

src/builtins/builtins-string.cc

@@ -7,9 +7,6 @@
 #include "src/conversions.h"
 #include "src/counters.h"
 #include "src/objects-inl.h"
-#ifdef V8_INTL_SUPPORT
-#include "src/objects/intl-objects.h"
-#endif
 #include "src/regexp/regexp-utils.h"
 #include "src/string-builder.h"
 #include "src/string-case.h"
@@ -193,18 +190,10 @@ BUILTIN(StringPrototypeLastIndexOf) {
 //
 // This function is implementation specific.  For now, we do not
 // do anything locale specific.
+// If internationalization is enabled, then intl.js will override this function
+// and provide the proper functionality, so this is just a fallback.
 BUILTIN(StringPrototypeLocaleCompare) {
   HandleScope handle_scope(isolate);
-#ifdef V8_INTL_SUPPORT
-  TO_THIS_STRING(str1, "String.prototype.localeCompare");
-  Handle<String> str2;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, str2, Object::ToString(isolate, args.atOrUndefined(isolate, 1)));
-  RETURN_RESULT_OR_FAILURE(
-      isolate, Intl::StringLocaleCompare(isolate, str1, str2,
-                                         args.atOrUndefined(isolate, 2),
-                                         args.atOrUndefined(isolate, 3)));
-#else
   DCHECK_EQ(2, args.length());
 
   TO_THIS_STRING(str1, "String.prototype.localeCompare");
@@ -246,7 +235,6 @@ BUILTIN(StringPrototypeLocaleCompare) {
   }
 
   return Smi::FromInt(str1_length - str2_length);
-#endif  // !V8_INTL_SUPPORT
 }
 
 #ifndef V8_INTL_SUPPORT

src/contexts.h

@@ -95,8 +95,6 @@ enum ContextLookupFlags {
   V(PROMISE_FUNCTION_INDEX, JSFunction, promise_function)                      \
   V(RANGE_ERROR_FUNCTION_INDEX, JSFunction, range_error_function)              \
   V(REFERENCE_ERROR_FUNCTION_INDEX, JSFunction, reference_error_function)      \
-  V(CACHED_OR_NEW_SERVICE_LOCALE_FUNCTION_INDEX, JSFunction,                   \
-    cached_or_new_service)                                                     \
   V(RESOLVE_LOCALE_FUNCTION_INDEX, JSFunction, resolve_locale)                 \
   V(SET_ADD_INDEX, JSFunction, set_add)                                        \
   V(SET_DELETE_INDEX, JSFunction, set_delete)                                  \

src/js/intl.js

@@ -2089,11 +2089,23 @@ function cachedOrNewService(service, locales, options, defaults) {
   return new savedObjects[service](locales, useOptions);
 }
 
-// TODO(ftang) remove the %InstallToContext once
-// cachedOrNewService is available in C++
-%InstallToContext([
-  "cached_or_new_service", cachedOrNewService
-]);
+/**
+ * Compares this and that, and returns less than 0, 0 or greater than 0 value.
+ * Overrides the built-in method.
+ */
+DEFINE_METHOD(
+  GlobalString.prototype,
+  localeCompare(that) {
+    if (IS_NULL_OR_UNDEFINED(this)) {
+      throw %make_type_error(kMethodInvokedOnNullOrUndefined);
+    }
+
+    var locales = arguments[1];
+    var options = arguments[2];
+    var collator = cachedOrNewService('collator', locales, options);
+    return compare(collator, this, that);
+  }
+);
 
 /**
  * Formats a Number object (this) using locale and options values.

src/objects/intl-objects.cc

@@ -1790,12 +1790,6 @@ namespace {
 
 // Remove the following after we port InitializeLocaleList from src/js/intl.js
 // to c++ https://bugs.chromium.org/p/v8/issues/detail?id=7987
-// The following are temporary function calling back into js code in
-// src/js/intl.js to call pre-existing functions until they are all moved to C++
-// under src/objects/*.
-// TODO(ftang): remove these temp function after bstell move them from js into
-// C++
-
 MaybeHandle<JSObject> InitializeLocaleList(Isolate* isolate,
                                            Handle<Object> list) {
   Handle<Object> result;
@@ -1814,22 +1808,6 @@ bool IsAToZ(char ch) {
   return (('A' <= ch) && (ch <= 'Z')) || (('a' <= ch) && (ch <= 'z'));
 }
 
-MaybeHandle<JSObject> CachedOrNewService(Isolate* isolate,
-                                         Handle<String> service,
-                                         Handle<Object> locales,
-                                         Handle<Object> options) {
-  Handle<Object> result;
-  Handle<Object> undefined_value(ReadOnlyRoots(isolate).undefined_value(),
-                                 isolate);
-  Handle<Object> args[] = {service, locales, options};
-  ASSIGN_RETURN_ON_EXCEPTION(
-      isolate, result,
-      Execution::Call(isolate, isolate->cached_or_new_service(),
-                      undefined_value, arraysize(args), args),
-      JSArray);
-  return Handle<JSObject>::cast(result);
-}
-
 }  // namespace
 
 // Verifies that the input is a well-formed ISO 4217 currency code.
@@ -1939,52 +1917,5 @@ MaybeHandle<String> Intl::StringLocaleConvertCase(Isolate* isolate,
   return Object::ToString(isolate, obj);
 }
 
-MaybeHandle<Object> Intl::StringLocaleCompare(Isolate* isolate,
-                                              Handle<String> string1,
-                                              Handle<String> string2,
-                                              Handle<Object> locales,
-                                              Handle<Object> options) {
-  Factory* factory = isolate->factory();
-  Handle<JSObject> collator_holder;
-  ASSIGN_RETURN_ON_EXCEPTION(
-      isolate, collator_holder,
-      CachedOrNewService(isolate, factory->NewStringFromStaticChars("collator"),
-                         locales, options),
-      Object);
-  DCHECK(Intl::IsObjectOfType(isolate, collator_holder, Intl::kCollator));
-  return Intl::InternalCompare(isolate, collator_holder, string1, string2);
-}
-
-Handle<Object> Intl::InternalCompare(Isolate* isolate,
-                                     Handle<JSObject> collator_holder,
-                                     Handle<String> string1,
-                                     Handle<String> string2) {
-  Factory* factory = isolate->factory();
-  icu::Collator* collator = Collator::UnpackCollator(collator_holder);
-  CHECK_NOT_NULL(collator);
-
-  string1 = String::Flatten(isolate, string1);
-  string2 = String::Flatten(isolate, string2);
-
-  UCollationResult result;
-  UErrorCode status = U_ZERO_ERROR;
-  {
-    DisallowHeapAllocation no_gc;
-    int32_t length1 = string1->length();
-    int32_t length2 = string2->length();
-    String::FlatContent flat1 = string1->GetFlatContent();
-    String::FlatContent flat2 = string2->GetFlatContent();
-    std::unique_ptr<uc16[]> sap1;
-    std::unique_ptr<uc16[]> sap2;
-    icu::UnicodeString string_val1(
-        FALSE, GetUCharBufferFromFlat(flat1, &sap1, length1), length1);
-    icu::UnicodeString string_val2(
-        FALSE, GetUCharBufferFromFlat(flat2, &sap2, length2), length2);
-    result = collator->compare(string_val1, string_val2, status);
-  }
-  DCHECK(U_SUCCESS(status));
-
-  return factory->NewNumberFromInt(result);
-}
 }  // namespace internal
 }  // namespace v8

src/objects/intl-objects.h

@@ -319,14 +319,6 @@ class Intl {
   V8_WARN_UNUSED_RESULT static MaybeHandle<String> StringLocaleConvertCase(
       Isolate* isolate, Handle<String> s, bool is_upper,
       Handle<Object> locales);
-
-  V8_WARN_UNUSED_RESULT static MaybeHandle<Object> StringLocaleCompare(
-      Isolate* isolate, Handle<String> s1, Handle<String> s2,
-      Handle<Object> locales, Handle<Object> options);
-
-  V8_WARN_UNUSED_RESULT static Handle<Object> InternalCompare(
-      Isolate* isolate, Handle<JSObject> collator, Handle<String> s1,
-      Handle<String> s2);
 };
 
 }  // namespace internal

src/runtime/runtime-intl.cc

@@ -288,7 +288,32 @@ RUNTIME_FUNCTION(Runtime_InternalCompare) {
   CONVERT_ARG_HANDLE_CHECKED(JSObject, collator_holder, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, string1, 1);
   CONVERT_ARG_HANDLE_CHECKED(String, string2, 2);
-  return *Intl::InternalCompare(isolate, collator_holder, string1, string2);
+
+  icu::Collator* collator = Collator::UnpackCollator(collator_holder);
+  CHECK_NOT_NULL(collator);
+
+  string1 = String::Flatten(isolate, string1);
+  string2 = String::Flatten(isolate, string2);
+
+  UCollationResult result;
+  UErrorCode status = U_ZERO_ERROR;
+  {
+    DisallowHeapAllocation no_gc;
+    int32_t length1 = string1->length();
+    int32_t length2 = string2->length();
+    String::FlatContent flat1 = string1->GetFlatContent();
+    String::FlatContent flat2 = string2->GetFlatContent();
+    std::unique_ptr<uc16[]> sap1;
+    std::unique_ptr<uc16[]> sap2;
+    icu::UnicodeString string_val1(
+        FALSE, GetUCharBufferFromFlat(flat1, &sap1, length1), length1);
+    icu::UnicodeString string_val2(
+        FALSE, GetUCharBufferFromFlat(flat2, &sap2, length2), length2);
+    result = collator->compare(string_val1, string_val2, status);
+  }
+  if (U_FAILURE(status)) return isolate->ThrowIllegalOperation();
+
+  return *isolate->factory()->NewNumberFromInt(result);
 }
 
 RUNTIME_FUNCTION(Runtime_CreatePluralRules) {