[intl] Remove redundant type checking system

Previously, the Intl implementation tracked types two ways:
 - In the intl_initialized_marker_symbol
 - In various named properties of the intl_impl_object_symbol value

As far as I can tell, these will never disagree with each other,
modulo bugs in Intl itself. This patch removes the second type
checking system.

This reland includes a fixed type check for
Intl.DateTimeFormat.prototype.formatToParts , which is the only Intl
method which is not bound. All future methods will follow this
pattern.

BUG=v8:5751,chromium:677055, v8:4962
CQ_INCLUDE_TRYBOTS=master.tryserver.v8:v8_linux_noi18n_rel_ng

Review-Url: https://codereview.chromium.org/2600913002
Cr-Commit-Position: refs/heads/master@{#42118}

src/i18n.cc

@@ -759,16 +759,7 @@ icu::SimpleDateFormat* DateFormat::InitializeDateTimeFormat(
 icu::SimpleDateFormat* DateFormat::UnpackDateFormat(
     Isolate* isolate,
     Handle<JSObject> obj) {
-  Handle<String> key =
-      isolate->factory()->NewStringFromStaticChars("dateFormat");
-  Maybe<bool> maybe = JSReceiver::HasOwnProperty(obj, key);
-  CHECK(maybe.IsJust());
-  if (maybe.FromJust()) {
-    return reinterpret_cast<icu::SimpleDateFormat*>(
-        obj->GetInternalField(0));
-  }
-
-  return NULL;
+  return reinterpret_cast<icu::SimpleDateFormat*>(obj->GetInternalField(0));
 }
 
 void DateFormat::DeleteDateFormat(const v8::WeakCallbackInfo<void>& data) {
@@ -823,15 +814,7 @@ icu::DecimalFormat* NumberFormat::InitializeNumberFormat(
 icu::DecimalFormat* NumberFormat::UnpackNumberFormat(
     Isolate* isolate,
     Handle<JSObject> obj) {
-  Handle<String> key =
-      isolate->factory()->NewStringFromStaticChars("numberFormat");
-  Maybe<bool> maybe = JSReceiver::HasOwnProperty(obj, key);
-  CHECK(maybe.IsJust());
-  if (maybe.FromJust()) {
-    return reinterpret_cast<icu::DecimalFormat*>(obj->GetInternalField(0));
-  }
-
-  return NULL;
+  return reinterpret_cast<icu::DecimalFormat*>(obj->GetInternalField(0));
 }
 
 void NumberFormat::DeleteNumberFormat(const v8::WeakCallbackInfo<void>& data) {
@@ -883,14 +866,7 @@ icu::Collator* Collator::InitializeCollator(
 
 icu::Collator* Collator::UnpackCollator(Isolate* isolate,
                                         Handle<JSObject> obj) {
-  Handle<String> key = isolate->factory()->NewStringFromStaticChars("collator");
-  Maybe<bool> maybe = JSReceiver::HasOwnProperty(obj, key);
-  CHECK(maybe.IsJust());
-  if (maybe.FromJust()) {
-    return reinterpret_cast<icu::Collator*>(obj->GetInternalField(0));
-  }
-
-  return NULL;
+  return reinterpret_cast<icu::Collator*>(obj->GetInternalField(0));
 }
 
 void Collator::DeleteCollator(const v8::WeakCallbackInfo<void>& data) {
@@ -945,15 +921,7 @@ icu::BreakIterator* BreakIterator::InitializeBreakIterator(
 
 icu::BreakIterator* BreakIterator::UnpackBreakIterator(Isolate* isolate,
                                                        Handle<JSObject> obj) {
-  Handle<String> key =
-      isolate->factory()->NewStringFromStaticChars("breakIterator");
-  Maybe<bool> maybe = JSReceiver::HasOwnProperty(obj, key);
-  CHECK(maybe.IsJust());
-  if (maybe.FromJust()) {
-    return reinterpret_cast<icu::BreakIterator*>(obj->GetInternalField(0));
-  }
-
-  return NULL;
+  return reinterpret_cast<icu::BreakIterator*>(obj->GetInternalField(0));
 }
 
 void BreakIterator::DeleteBreakIterator(

src/js/i18n.js

@@ -1747,6 +1747,11 @@ function FormatDateToParts(dateValue) {
   if (!IS_OBJECT(this)) {
     throw %make_type_error(kCalledOnNonObject, this);
   }
+  if (!%IsInitializedIntlObjectOfType(this, 'dateformat')) {
+    throw %make_type_error(kIncompatibleMethodReceiver,
+                          'Intl.DateTimeFormat.prototype.formatToParts',
+                          this);
+  }
   var dateMs;
   if (IS_UNDEFINED(dateValue)) {
     dateMs = %DateCurrentTime();

src/runtime/runtime-i18n.cc

@@ -359,11 +359,6 @@ RUNTIME_FUNCTION(Runtime_CreateDateTimeFormat) {
 
   local_object->SetInternalField(0, reinterpret_cast<Smi*>(date_format));
 
-  Factory* factory = isolate->factory();
-  Handle<String> key = factory->NewStringFromStaticChars("dateFormat");
-  Handle<String> value = factory->NewStringFromStaticChars("valid");
-  JSObject::AddProperty(local_object, key, value, NONE);
-
   // Make object handle weak so we can delete the data format once GC kicks in.
   Handle<Object> wrapper = isolate->global_handles()->Create(*local_object);
   GlobalHandles::MakeWeak(wrapper.location(), wrapper.location(),
@@ -386,7 +381,7 @@ RUNTIME_FUNCTION(Runtime_InternalDateFormat) {
 
   icu::SimpleDateFormat* date_format =
       DateFormat::UnpackDateFormat(isolate, date_format_holder);
-  if (!date_format) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(date_format);
 
   icu::UnicodeString result;
   date_format->format(value->Number(), result);
@@ -487,7 +482,7 @@ RUNTIME_FUNCTION(Runtime_InternalDateFormatToParts) {
 
   icu::SimpleDateFormat* date_format =
       DateFormat::UnpackDateFormat(isolate, date_format_holder);
-  if (!date_format) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(date_format);
 
   icu::UnicodeString formatted;
   icu::FieldPositionIterator fp_iter;
@@ -556,11 +551,6 @@ RUNTIME_FUNCTION(Runtime_CreateNumberFormat) {
 
   local_object->SetInternalField(0, reinterpret_cast<Smi*>(number_format));
 
-  Factory* factory = isolate->factory();
-  Handle<String> key = factory->NewStringFromStaticChars("numberFormat");
-  Handle<String> value = factory->NewStringFromStaticChars("valid");
-  JSObject::AddProperty(local_object, key, value, NONE);
-
   Handle<Object> wrapper = isolate->global_handles()->Create(*local_object);
   GlobalHandles::MakeWeak(wrapper.location(), wrapper.location(),
                           NumberFormat::DeleteNumberFormat,
@@ -582,7 +572,7 @@ RUNTIME_FUNCTION(Runtime_InternalNumberFormat) {
 
   icu::DecimalFormat* number_format =
       NumberFormat::UnpackNumberFormat(isolate, number_format_holder);
-  if (!number_format) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(number_format);
 
   icu::UnicodeString result;
   number_format->format(value->Number(), result);
@@ -618,11 +608,6 @@ RUNTIME_FUNCTION(Runtime_CreateCollator) {
 
   local_object->SetInternalField(0, reinterpret_cast<Smi*>(collator));
 
-  Factory* factory = isolate->factory();
-  Handle<String> key = factory->NewStringFromStaticChars("collator");
-  Handle<String> value = factory->NewStringFromStaticChars("valid");
-  JSObject::AddProperty(local_object, key, value, NONE);
-
   Handle<Object> wrapper = isolate->global_handles()->Create(*local_object);
   GlobalHandles::MakeWeak(wrapper.location(), wrapper.location(),
                           Collator::DeleteCollator,
@@ -641,7 +626,7 @@ RUNTIME_FUNCTION(Runtime_InternalCompare) {
   CONVERT_ARG_HANDLE_CHECKED(String, string2, 2);
 
   icu::Collator* collator = Collator::UnpackCollator(isolate, collator_holder);
-  if (!collator) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(collator);
 
   string1 = String::Flatten(string1);
   string2 = String::Flatten(string2);
@@ -753,11 +738,6 @@ RUNTIME_FUNCTION(Runtime_CreateBreakIterator) {
   // Make sure that the pointer to adopted text is NULL.
   local_object->SetInternalField(1, static_cast<Smi*>(nullptr));
 
-  Factory* factory = isolate->factory();
-  Handle<String> key = factory->NewStringFromStaticChars("breakIterator");
-  Handle<String> value = factory->NewStringFromStaticChars("valid");
-  JSObject::AddProperty(local_object, key, value, NONE);
-
   // Make object handle weak so we can delete the break iterator once GC kicks
   // in.
   Handle<Object> wrapper = isolate->global_handles()->Create(*local_object);
@@ -778,7 +758,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorAdoptText) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  if (!break_iterator) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(break_iterator);
 
   icu::UnicodeString* u_text = reinterpret_cast<icu::UnicodeString*>(
       break_iterator_holder->GetInternalField(1));
@@ -808,7 +788,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorFirst) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  if (!break_iterator) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(break_iterator);
 
   return *isolate->factory()->NewNumberFromInt(break_iterator->first());
 }
@@ -823,7 +803,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorNext) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  if (!break_iterator) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(break_iterator);
 
   return *isolate->factory()->NewNumberFromInt(break_iterator->next());
 }
@@ -838,7 +818,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorCurrent) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  if (!break_iterator) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(break_iterator);
 
   return *isolate->factory()->NewNumberFromInt(break_iterator->current());
 }
@@ -853,7 +833,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorBreakType) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  if (!break_iterator) return isolate->ThrowIllegalOperation();
+  CHECK_NOT_NULL(break_iterator);
 
   // TODO(cira): Remove cast once ICU fixes base BreakIterator class.
   icu::RuleBasedBreakIterator* rule_based_iterator =

test/intl/bad-target.js

+// Copyright 2016 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.
+
+// Calling Intl methods with a bad receiver throws a TypeError
+
+// An uninitialized object of the same type
+assertThrows(() => Object.create(Intl.DateTimeFormat.prototype).format(),
+             TypeError);
+assertThrows(() => Object.create(Intl.NumberFormat.prototype).format(),
+             TypeError);
+assertThrows(() => Object.create(Intl.Collator.prototype).compare(),
+             TypeError);
+assertThrows(() => Object.create(Intl.v8BreakIterator.prototype).adoptText(),
+             TypeError);
+assertThrows(() => Object.create(Intl.v8BreakIterator.prototype).first(),
+             TypeError);
+assertThrows(() => Object.create(Intl.v8BreakIterator.prototype).next(),
+             TypeError);
+assertThrows(() => Object.create(Intl.v8BreakIterator.prototype).current(),
+             TypeError);
+assertThrows(() => Object.create(Intl.v8BreakIterator.prototype).breakType(),
+             TypeError);
+
+// Or similarly, just accessing the method getter on the prototype
+assertThrows(() => Intl.DateTimeFormat.prototype.format, TypeError);
+assertThrows(() => Intl.NumberFormat.prototype.format, TypeError);
+assertThrows(() => Intl.Collator.prototype.compare, TypeError);
+assertThrows(() => Intl.v8BreakIterator.prototype.adoptText, TypeError);
+assertThrows(() => Intl.v8BreakIterator.prototype.first, TypeError);
+assertThrows(() => Intl.v8BreakIterator.prototype.next, TypeError);
+assertThrows(() => Intl.v8BreakIterator.prototype.current, TypeError);
+assertThrows(() => Intl.v8BreakIterator.prototype.breakType, TypeError);
+
+// The method .call'd on a different instance will have that
+// other instance benignly ignored, since it's a bound function
+let nf = Intl.NumberFormat();
+let df = Intl.DateTimeFormat();
+assertEquals("0", nf.format.call(df, 0));

test/mjsunit/regress/regress-4962.js

+// Copyright 2016 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.
+
+assertInstanceof(Intl.NumberFormat.call(new Proxy({},{})), Intl.NumberFormat);
+assertThrows(() =>
+             Intl.DateTimeFormat.prototype.formatToParts.call(
+                 new Proxy({}, {})),
+             TypeError);

test/mjsunit/regress/regress-677055.js

+// Copyright 2016 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.
+
+if (this.Intl) {
+  v5 = new Intl.NumberFormat();
+  v9 = new Intl.DateTimeFormat();
+  v52 = v9["formatToParts"];
+  var v55 = {};
+  assertThrows(() => Reflect.apply(v52, v5, v55), TypeError);
+}