[intl] Fix Intl.NumberFormat constructor

camillobruni · Commit Bot · commit 09d14728ca25 · 2020-03-03T11:33:53.000Z
Call the @@hasInstance trap only when required by the spec. Bug: chromium:1052647 Change-Id: I7a0a3133c7b6280c6a3215e379bf02e9c22ffe55 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2082560 Commit-Queue: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org> Cr-Commit-Position: refs/heads/master@{#66558}
diff --git a/src/builtins/builtins-intl.cc b/src/builtins/builtins-intl.cc
@@ -265,13 +265,11 @@ Object LegacyFormatConstructor(BuiltinArguments args, Isolate* isolate,
 
   // [[Construct]]
   Handle<JSFunction> target = args.target();
-
   Handle<Object> locales = args.atOrUndefined(isolate, 1);
   Handle<Object> options = args.atOrUndefined(isolate, 2);
 
   // 2. Let format be ? OrdinaryCreateFromConstructor(newTarget,
   // "%<T>Prototype%", ...).
-
   Handle<Map> map;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, map, JSFunction::GetDerivedMap(isolate, target, new_target));
@@ -281,45 +279,42 @@ Object LegacyFormatConstructor(BuiltinArguments args, Isolate* isolate,
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, format, T::New(isolate, map, locales, options, method));
   // 4. Let this be the this value.
-  Handle<Object> receiver = args.receiver();
-
-  // 5. If NewTarget is undefined and ? InstanceofOperator(this, %<T>%)
-  // is true, then
-  //
-  // Look up the intrinsic value that has been stored on the context.
-  // Call the instanceof function
-  Handle<Object> is_instance_of_obj;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, is_instance_of_obj,
-      Object::InstanceOf(isolate, receiver, constructor));
-
-  // Get the boolean value of the result
-  bool is_instance_of = is_instance_of_obj->BooleanValue(isolate);
-
-  if (args.new_target()->IsUndefined(isolate) && is_instance_of) {
-    if (!receiver->IsJSReceiver()) {
-      THROW_NEW_ERROR_RETURN_FAILURE(
-          isolate,
-          NewTypeError(MessageTemplate::kIncompatibleMethodReceiver,
-                       isolate->factory()->NewStringFromAsciiChecked(method),
-                       receiver));
+  if (args.new_target()->IsUndefined(isolate)) {
+    Handle<Object> receiver = args.receiver();
+
+    // 5. If NewTarget is undefined and ? InstanceofOperator(this, %<T>%)
+    // is true, then Look up the intrinsic value that has been stored on
+    // the context.
+    Handle<Object> is_instance_of_obj;
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, is_instance_of_obj,
+        Object::InstanceOf(isolate, receiver, constructor));
+
+    if (is_instance_of_obj->BooleanValue(isolate)) {
+      if (!receiver->IsJSReceiver()) {
+        THROW_NEW_ERROR_RETURN_FAILURE(
+            isolate,
+            NewTypeError(MessageTemplate::kIncompatibleMethodReceiver,
+                         isolate->factory()->NewStringFromAsciiChecked(method),
+                         receiver));
+      }
+      Handle<JSReceiver> rec = Handle<JSReceiver>::cast(receiver);
+      // a. Perform ? DefinePropertyOrThrow(this,
+      // %Intl%.[[FallbackSymbol]], PropertyDescriptor{ [[Value]]: format,
+      // [[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: false }).
+      PropertyDescriptor desc;
+      desc.set_value(format);
+      desc.set_writable(false);
+      desc.set_enumerable(false);
+      desc.set_configurable(false);
+      Maybe<bool> success = JSReceiver::DefineOwnProperty(
+          isolate, rec, isolate->factory()->intl_fallback_symbol(), &desc,
+          Just(kThrowOnError));
+      MAYBE_RETURN(success, ReadOnlyRoots(isolate).exception());
+      CHECK(success.FromJust());
+      // b. b. Return this.
+      return *receiver;
     }
-    Handle<JSReceiver> rec = Handle<JSReceiver>::cast(receiver);
-    // a. Perform ? DefinePropertyOrThrow(this,
-    // %Intl%.[[FallbackSymbol]], PropertyDescriptor{ [[Value]]: format,
-    // [[Writable]]: false, [[Enumerable]]: false, [[Configurable]]: false }).
-    PropertyDescriptor desc;
-    desc.set_value(format);
-    desc.set_writable(false);
-    desc.set_enumerable(false);
-    desc.set_configurable(false);
-    Maybe<bool> success = JSReceiver::DefineOwnProperty(
-        isolate, rec, isolate->factory()->intl_fallback_symbol(), &desc,
-        Just(kThrowOnError));
-    MAYBE_RETURN(success, ReadOnlyRoots(isolate).exception());
-    CHECK(success.FromJust());
-    // b. b. Return this.
-    return *receiver;
   }
   // 6. Return format.
   return *format;
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
@@ -404,6 +404,9 @@
   'tzoffset-transition-moscow': [SKIP],
   'tzoffset-transition-new-york': [SKIP],
   'tzoffset-seoul': [SKIP],
+
+  # noi18n is required for Intl
+  'regress/regress-crbug-1052647': [SKIP],
 }],  # 'no_i18n'
 
 ##############################################################################
diff --git a/test/mjsunit/regress/regress-crbug-1052647.js b/test/mjsunit/regress/regress-crbug-1052647.js
@@ -0,0 +1,12 @@
+// 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.
+
+let useArgs = undefined;
+function f(arg) {
+    useArgs = 'result' + arguments[0] + arg;
+}
+
+Intl.NumberFormat.__proto__ = { [Symbol.hasInstance]: f };
+
+new Intl.NumberFormat();
