Revert of [intl] Remove redundant type checking system (patchset #3 id:40001...

Revert of [intl] Remove redundant type checking system (patchset #3 id:40001 of https://codereview.chromium.org/2591203002/ )

Reason for revert:
Issue https://bugs.chromium.org/p/chromium/issues/detail?id=677055 . I'll send out a follow-on reland, as it should still be possible to eliminate the redundant type system.

Original issue's description:
> [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.
>
> BUG=v8:5751
>
> Review-Url: https://codereview.chromium.org/2591203002
> Cr-Commit-Position: refs/heads/master@{#41941}
> Committed: https://chromium.googlesource.com/v8/v8/+/0d5561b64d34129e6546947255e543c219c61655

TBR=yangguo@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=v8:5751

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

src/i18n.cc

@@ -759,7 +759,16 @@ icu::SimpleDateFormat* DateFormat::InitializeDateTimeFormat(
 icu::SimpleDateFormat* DateFormat::UnpackDateFormat(
     Isolate* isolate,
     Handle<JSObject> obj) {
-  return reinterpret_cast<icu::SimpleDateFormat*>(obj->GetInternalField(0));
+  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;
 }
 
 void DateFormat::DeleteDateFormat(const v8::WeakCallbackInfo<void>& data) {
@@ -814,7 +823,15 @@ icu::DecimalFormat* NumberFormat::InitializeNumberFormat(
 icu::DecimalFormat* NumberFormat::UnpackNumberFormat(
     Isolate* isolate,
     Handle<JSObject> obj) {
-  return reinterpret_cast<icu::DecimalFormat*>(obj->GetInternalField(0));
+  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;
 }
 
 void NumberFormat::DeleteNumberFormat(const v8::WeakCallbackInfo<void>& data) {
@@ -866,7 +883,14 @@ icu::Collator* Collator::InitializeCollator(
 
 icu::Collator* Collator::UnpackCollator(Isolate* isolate,
                                         Handle<JSObject> obj) {
-  return reinterpret_cast<icu::Collator*>(obj->GetInternalField(0));
+  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;
 }
 
 void Collator::DeleteCollator(const v8::WeakCallbackInfo<void>& data) {
@@ -921,7 +945,15 @@ icu::BreakIterator* BreakIterator::InitializeBreakIterator(
 
 icu::BreakIterator* BreakIterator::UnpackBreakIterator(Isolate* isolate,
                                                        Handle<JSObject> obj) {
-  return reinterpret_cast<icu::BreakIterator*>(obj->GetInternalField(0));
+  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;
 }
 
 void BreakIterator::DeleteBreakIterator(

src/runtime/runtime-i18n.cc

@@ -359,6 +359,11 @@ 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(),
@@ -381,7 +386,7 @@ RUNTIME_FUNCTION(Runtime_InternalDateFormat) {
 
   icu::SimpleDateFormat* date_format =
       DateFormat::UnpackDateFormat(isolate, date_format_holder);
-  CHECK_NOT_NULL(date_format);
+  if (!date_format) return isolate->ThrowIllegalOperation();
 
   icu::UnicodeString result;
   date_format->format(value->Number(), result);
@@ -482,7 +487,7 @@ RUNTIME_FUNCTION(Runtime_InternalDateFormatToParts) {
 
   icu::SimpleDateFormat* date_format =
       DateFormat::UnpackDateFormat(isolate, date_format_holder);
-  CHECK_NOT_NULL(date_format);
+  if (!date_format) return isolate->ThrowIllegalOperation();
 
   icu::UnicodeString formatted;
   icu::FieldPositionIterator fp_iter;
@@ -551,6 +556,11 @@ 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,
@@ -572,7 +582,7 @@ RUNTIME_FUNCTION(Runtime_InternalNumberFormat) {
 
   icu::DecimalFormat* number_format =
       NumberFormat::UnpackNumberFormat(isolate, number_format_holder);
-  CHECK_NOT_NULL(number_format);
+  if (!number_format) return isolate->ThrowIllegalOperation();
 
   icu::UnicodeString result;
   number_format->format(value->Number(), result);
@@ -608,6 +618,11 @@ 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,
@@ -626,7 +641,7 @@ RUNTIME_FUNCTION(Runtime_InternalCompare) {
   CONVERT_ARG_HANDLE_CHECKED(String, string2, 2);
 
   icu::Collator* collator = Collator::UnpackCollator(isolate, collator_holder);
-  CHECK_NOT_NULL(collator);
+  if (!collator) return isolate->ThrowIllegalOperation();
 
   string1 = String::Flatten(string1);
   string2 = String::Flatten(string2);
@@ -738,6 +753,11 @@ 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);
@@ -758,7 +778,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorAdoptText) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  CHECK_NOT_NULL(break_iterator);
+  if (!break_iterator) return isolate->ThrowIllegalOperation();
 
   icu::UnicodeString* u_text = reinterpret_cast<icu::UnicodeString*>(
       break_iterator_holder->GetInternalField(1));
@@ -788,7 +808,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorFirst) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  CHECK_NOT_NULL(break_iterator);
+  if (!break_iterator) return isolate->ThrowIllegalOperation();
 
   return *isolate->factory()->NewNumberFromInt(break_iterator->first());
 }
@@ -803,7 +823,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorNext) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  CHECK_NOT_NULL(break_iterator);
+  if (!break_iterator) return isolate->ThrowIllegalOperation();
 
   return *isolate->factory()->NewNumberFromInt(break_iterator->next());
 }
@@ -818,7 +838,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorCurrent) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  CHECK_NOT_NULL(break_iterator);
+  if (!break_iterator) return isolate->ThrowIllegalOperation();
 
   return *isolate->factory()->NewNumberFromInt(break_iterator->current());
 }
@@ -833,7 +853,7 @@ RUNTIME_FUNCTION(Runtime_BreakIteratorBreakType) {
 
   icu::BreakIterator* break_iterator =
       BreakIterator::UnpackBreakIterator(isolate, break_iterator_holder);
-  CHECK_NOT_NULL(break_iterator);
+  if (!break_iterator) return isolate->ThrowIllegalOperation();
 
   // 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));