Revert of Mark internal AccessorInfo properties as "special data properties"...

Revert of Mark internal AccessorInfo properties as "special data properties" (patchset #2 id:20001 of https://codereview.chromium.org/1123163005/)

Reason for revert:
[Sheriff] Blocks current roll:
https://codereview.chromium.org/1124403007/

Bisection (https://codereview.chromium.org/1142753002/) points to this CL.

Original issue's description:
> Mark internal AccessorInfo properties as "special data properties" to ensure correct strict-mode handling.
>
> BUG=
>
> Committed: https://crrev.com/188297160d2b82a4e2a206ebbddfc21dd99a9d8d
> Cr-Commit-Position: refs/heads/master@{#28369}

TBR=rossberg@chromium.org,verwaest@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/1142763002

Cr-Commit-Position: refs/heads/master@{#28437}

src/accessors.cc

@@ -32,7 +32,6 @@ Handle<AccessorInfo> Accessors::MakeAccessor(
   info->set_property_attributes(attributes);
   info->set_all_can_read(false);
   info->set_all_can_write(false);
-  info->set_is_special_data_property(true);
   info->set_name(*name);
   Handle<Object> get = v8::FromCData(isolate, getter);
   Handle<Object> set = v8::FromCData(isolate, setter);
@@ -127,6 +126,31 @@ bool Accessors::IsJSArrayBufferViewFieldAccessor(Handle<Map> map,
 }
 
 
+bool SetPropertyOnInstanceIfInherited(
+    Isolate* isolate, const v8::PropertyCallbackInfo<void>& info,
+    v8::Local<v8::Name> name, Handle<Object> value) {
+  Handle<Object> holder = Utils::OpenHandle(*info.Holder());
+  Handle<Object> receiver = Utils::OpenHandle(*info.This());
+  if (*holder == *receiver) return false;
+  if (receiver->IsJSObject()) {
+    Handle<JSObject> object = Handle<JSObject>::cast(receiver);
+    // This behaves sloppy since we lost the actual strict-mode.
+    // TODO(verwaest): Fix by making ExecutableAccessorInfo behave like data
+    // properties.
+    if (object->IsJSGlobalProxy()) {
+      PrototypeIterator iter(isolate, object);
+      if (iter.IsAtEnd()) return true;
+      DCHECK(PrototypeIterator::GetCurrent(iter)->IsJSGlobalObject());
+      object = Handle<JSObject>::cast(PrototypeIterator::GetCurrent(iter));
+    }
+    if (!object->map()->is_extensible()) return true;
+    JSObject::SetOwnPropertyIgnoreAttributes(object, Utils::OpenHandle(*name),
+                                             value, NONE).Check();
+  }
+  return true;
+}
+
+
 //
 // Accessors::ArgumentsIterator
 //
@@ -150,6 +174,8 @@ void Accessors::ArgumentsIteratorSetter(
   Handle<JSObject> object = Utils::OpenHandle(*info.This());
   Handle<Object> value = Utils::OpenHandle(*val);
 
+  if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) return;
+
   LookupIterator it(object, Utils::OpenHandle(*name));
   CHECK_EQ(LookupIterator::ACCESSOR, it.state());
   DCHECK(it.HolderIsReceiverOrHiddenPrototype());
@@ -208,6 +234,9 @@ void Accessors::ArrayLengthSetter(
   HandleScope scope(isolate);
   Handle<JSObject> object = Utils::OpenHandle(*info.This());
   Handle<Object> value = Utils::OpenHandle(*val);
+  if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) {
+    return;
+  }
 
   value = FlattenNumber(isolate, value);
 
@@ -941,6 +970,9 @@ void Accessors::FunctionPrototypeSetter(
   i::Isolate* isolate = reinterpret_cast<i::Isolate*>(info.GetIsolate());
   HandleScope scope(isolate);
   Handle<Object> value = Utils::OpenHandle(*val);
+  if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) {
+    return;
+  }
   Handle<JSFunction> object =
       Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
   if (SetFunctionPrototype(isolate, object, value).is_null()) {
@@ -1029,6 +1061,8 @@ void Accessors::FunctionLengthSetter(
   HandleScope scope(isolate);
   Handle<Object> value = Utils::OpenHandle(*val);
 
+  if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) return;
+
   Handle<JSFunction> object =
       Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
   if (SetFunctionLength(isolate, object, value).is_null()) {
@@ -1086,6 +1120,8 @@ void Accessors::FunctionNameSetter(
   HandleScope scope(isolate);
   Handle<Object> value = Utils::OpenHandle(*val);
 
+  if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) return;
+
   Handle<JSFunction> object =
       Handle<JSFunction>::cast(Utils::OpenHandle(*info.Holder()));
   if (SetFunctionName(isolate, object, value).is_null()) {

src/lookup.cc

@@ -142,9 +142,7 @@ void LookupIterator::PrepareTransitionToDataProperty(
     Handle<Object> value, PropertyAttributes attributes,
     Object::StoreFromKeyed store_mode) {
   if (state_ == TRANSITION) return;
-  DCHECK(state_ != LookupIterator::ACCESSOR ||
-         (GetAccessors()->IsAccessorInfo() &&
-          AccessorInfo::cast(*GetAccessors())->is_special_data_property()));
+  DCHECK_NE(LookupIterator::ACCESSOR, state_);
   DCHECK_NE(LookupIterator::INTEGER_INDEXED_EXOTIC, state_);
   DCHECK(state_ == NOT_FOUND || !HolderIsReceiverOrHiddenPrototype());
   // Can only be called when the receiver is a JSObject. JSProxy has to be

src/objects-inl.h

@@ -7055,16 +7055,6 @@ void AccessorInfo::set_all_can_write(bool value) {
 }
 
 
-bool AccessorInfo::is_special_data_property() {
-  return BooleanBit::get(flag(), kSpecialDataProperty);
-}
-
-
-void AccessorInfo::set_is_special_data_property(bool value) {
-  set_flag(BooleanBit::set(flag(), kSpecialDataProperty, value));
-}
-
-
 PropertyAttributes AccessorInfo::property_attributes() {
   return AttributesField::decode(static_cast<uint32_t>(flag()->value()));
 }

src/objects.cc

@@ -3215,21 +3215,14 @@ MaybeHandle<Object> Object::SetPropertyInternal(LookupIterator* it,
         }
         break;
 
-      case LookupIterator::ACCESSOR: {
+      case LookupIterator::ACCESSOR:
         if (it->property_details().IsReadOnly()) {
           return WriteToReadOnlyProperty(it, value, language_mode);
         }
-        Handle<Object> accessors = it->GetAccessors();
-        if (accessors->IsAccessorInfo() &&
-            !it->HolderIsReceiverOrHiddenPrototype() &&
-            AccessorInfo::cast(*accessors)->is_special_data_property()) {
-          done = true;
-          break;
-        }
         return SetPropertyWithAccessor(it->GetReceiver(), it->name(), value,
-                                       it->GetHolder<JSObject>(), accessors,
-                                       language_mode);
-      }
+                                       it->GetHolder<JSObject>(),
+                                       it->GetAccessors(), language_mode);
+
       case LookupIterator::INTEGER_INDEXED_EXOTIC:
         done = true;
         break;

src/objects.h

@@ -10516,9 +10516,6 @@ class AccessorInfo: public Struct {
   inline bool all_can_write();
   inline void set_all_can_write(bool value);
 
-  inline bool is_special_data_property();
-  inline void set_is_special_data_property(bool value);
-
   inline PropertyAttributes property_attributes();
   inline void set_property_attributes(PropertyAttributes attributes);
 
@@ -10551,8 +10548,7 @@ class AccessorInfo: public Struct {
   // Bit positions in flag.
   static const int kAllCanReadBit = 0;
   static const int kAllCanWriteBit = 1;
-  static const int kSpecialDataProperty = 2;
-  class AttributesField : public BitField<PropertyAttributes, 3, 3> {};
+  class AttributesField: public BitField<PropertyAttributes, 2, 3> {};
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(AccessorInfo);
 };

test/mjsunit/array-length.js

@@ -119,9 +119,3 @@ for (var i = 0; i < 7; i++) {
   t = a.length = 7;
   assertEquals(7, t);
 }
-
-(function () {
-  "use strict";
-  var frozen_object = Object.freeze({__proto__:[]});
-  assertThrows(function () { frozen_object.length = 10 });
-})();

test/mjsunit/es6/arguments-iterator.js

@@ -219,7 +219,10 @@ function TestArgumentsAsProto() {
   assertSame([][Symbol.iterator], o[Symbol.iterator]);
   assertFalse(o.hasOwnProperty(Symbol.iterator));
   assertSame([][Symbol.iterator], o[Symbol.iterator]);
-  assertThrows(function () { o[Symbol.iterator] = 10 });
+  // This should throw, but currently it doesn't, because
+  // ExecutableAccessorInfo callbacks don't see the current strict mode.
+  // See note in accessors.cc:SetPropertyOnInstanceIfInherited.
+  o[Symbol.iterator] = 10;
   assertFalse(o.hasOwnProperty(Symbol.iterator));
   assertEquals([][Symbol.iterator], o[Symbol.iterator]);
   assertSame([][Symbol.iterator], arguments[Symbol.iterator]);