Make it possible to set a getter and a setter at the same time

BUG=chromium:588893, chromium:325923
LOG=n
R=verwaest@chromium.org

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

src/lookup.cc

@@ -370,11 +370,10 @@ void LookupIterator::Delete() {
   state_ = NOT_FOUND;
 }
 
-
 void LookupIterator::TransitionToAccessorProperty(
-    AccessorComponent component, Handle<Object> accessor,
+    Handle<Object> getter, Handle<Object> setter,
     PropertyAttributes attributes) {
-  DCHECK(!accessor->IsNull());
+  DCHECK(!getter->IsNull() || !setter->IsNull());
   // Can only be called when the receiver is a JSObject. JSProxy has to be
   // handled via a trap. Adding properties to primitive values is not
   // observable.
@@ -393,7 +392,7 @@ void LookupIterator::TransitionToAccessorProperty(
         IsFound() ? static_cast<int>(number_) : DescriptorArray::kNotFound;
 
     Handle<Map> new_map = Map::TransitionToAccessorProperty(
-        old_map, name_, descriptor, component, accessor, attributes);
+        isolate_, old_map, name_, descriptor, getter, setter, attributes);
     bool simple_transition = new_map->GetBackPointer() == receiver->map();
     JSObject::MigrateToMap(receiver, new_map);
 
@@ -413,18 +412,18 @@ void LookupIterator::TransitionToAccessorProperty(
   if (state() == ACCESSOR && GetAccessors()->IsAccessorPair()) {
     pair = Handle<AccessorPair>::cast(GetAccessors());
     // If the component and attributes are identical, nothing has to be done.
-    if (pair->get(component) == *accessor) {
+    if (pair->Equals(*getter, *setter)) {
       if (property_details().attributes() == attributes) {
         if (!IsElement()) JSObject::ReoptimizeIfPrototype(receiver);
         return;
       }
     } else {
       pair = AccessorPair::Copy(pair);
-      pair->set(component, *accessor);
+      pair->SetComponents(*getter, *setter);
     }
   } else {
     pair = factory()->NewAccessorPair();
-    pair->set(component, *accessor);
+    pair->SetComponents(*getter, *setter);
   }
 
   TransitionToAccessorPair(pair, attributes);

src/lookup.h

@@ -229,8 +229,8 @@ class LookupIterator final BASE_EMBEDDED {
   void ReconfigureDataProperty(Handle<Object> value,
                                PropertyAttributes attributes);
   void Delete();
-  void TransitionToAccessorProperty(AccessorComponent component,
-                                    Handle<Object> accessor,
+  void TransitionToAccessorProperty(Handle<Object> getter,
+                                    Handle<Object> setter,
                                     PropertyAttributes attributes);
   void TransitionToAccessorPair(Handle<Object> pair,
                                 PropertyAttributes attributes);

src/objects.cc

@@ -8283,14 +8283,7 @@ MaybeHandle<Object> JSObject::DefineAccessor(LookupIterator* it,
          getter->IsFunctionTemplateInfo());
   DCHECK(setter->IsCallable() || setter->IsUndefined() || setter->IsNull() ||
          getter->IsFunctionTemplateInfo());
-  // At least one of the accessors needs to be a new value.
-  DCHECK(!getter->IsNull() || !setter->IsNull());
-  if (!getter->IsNull()) {
-    it->TransitionToAccessorProperty(ACCESSOR_GETTER, getter, attributes);
-  }
-  if (!setter->IsNull()) {
-    it->TransitionToAccessorProperty(ACCESSOR_SETTER, setter, attributes);
-  }
+  it->TransitionToAccessorProperty(getter, setter, attributes);
 
   return isolate->factory()->undefined_value();
 }
@@ -9088,13 +9081,14 @@ Handle<Map> Map::ReconfigureExistingProperty(Handle<Map> map, int descriptor,
   return new_map;
 }
 
-Handle<Map> Map::TransitionToAccessorProperty(Handle<Map> map,
+Handle<Map> Map::TransitionToAccessorProperty(Isolate* isolate, Handle<Map> map,
                                               Handle<Name> name, int descriptor,
-                                              AccessorComponent component,
-                                              Handle<Object> accessor,
+                                              Handle<Object> getter,
+                                              Handle<Object> setter,
                                               PropertyAttributes attributes) {
+  // At least one of the accessors needs to be a new value.
+  DCHECK(!getter->IsNull() || !setter->IsNull());
   DCHECK(name->IsUniqueName());
-  Isolate* isolate = name->GetIsolate();
 
   // Dictionary maps can always have additional data properties.
   if (map->is_dictionary_map()) return map;
@@ -9123,7 +9117,7 @@ Handle<Map> Map::TransitionToAccessorProperty(Handle<Map> map,
     }
 
     Handle<AccessorPair> pair = Handle<AccessorPair>::cast(maybe_pair);
-    if (pair->get(component) != *accessor) {
+    if (!pair->Equals(*getter, *setter)) {
       return Map::Normalize(map, mode, "TransitionToDifferentAccessor");
     }
 
@@ -9150,10 +9144,19 @@ Handle<Map> Map::TransitionToAccessorProperty(Handle<Map> map,
       return Map::Normalize(map, mode, "AccessorsOverwritingNonPair");
     }
 
-    Object* current = Handle<AccessorPair>::cast(maybe_pair)->get(component);
-    if (current == *accessor) return map;
+    Handle<AccessorPair> current_pair = Handle<AccessorPair>::cast(maybe_pair);
+    if (current_pair->Equals(*getter, *setter)) return map;
 
-    if (!current->IsNull()) {
+    bool overwriting_accessor = false;
+    if (!getter->IsNull() && !current_pair->get(ACCESSOR_GETTER)->IsNull() &&
+        current_pair->get(ACCESSOR_GETTER) != *getter) {
+      overwriting_accessor = true;
+    }
+    if (!setter->IsNull() && !current_pair->get(ACCESSOR_SETTER)->IsNull() &&
+        current_pair->get(ACCESSOR_SETTER) != *setter) {
+      overwriting_accessor = true;
+    }
+    if (overwriting_accessor) {
       return Map::Normalize(map, mode, "AccessorsOverwritingAccessors");
     }
 
@@ -9165,7 +9168,8 @@ Handle<Map> Map::TransitionToAccessorProperty(Handle<Map> map,
     pair = isolate->factory()->NewAccessorPair();
   }
 
-  pair->set(component, *accessor);
+  pair->SetComponents(*getter, *setter);
+
   TransitionFlag flag = INSERT_TRANSITION;
   AccessorConstantDescriptor new_desc(name, pair, attributes);
   return Map::CopyInsertDescriptor(map, &new_desc, flag);

src/objects.h

@@ -5914,8 +5914,8 @@ class Map: public HeapObject {
                                               PropertyAttributes attributes,
                                               StoreFromKeyed store_mode);
   static Handle<Map> TransitionToAccessorProperty(
-      Handle<Map> map, Handle<Name> name, int descriptor,
-      AccessorComponent component, Handle<Object> accessor,
+      Isolate* isolate, Handle<Map> map, Handle<Name> name, int descriptor,
+      Handle<Object> getter, Handle<Object> setter,
       PropertyAttributes attributes);
   static Handle<Map> ReconfigureExistingProperty(Handle<Map> map,
                                                  int descriptor,

test/cctest/test-field-type-tracking.cc

@@ -401,15 +401,8 @@ class Expectations {
 
     int descriptor =
         map->instance_descriptors()->SearchWithCache(isolate, *name, *map);
-    map = Map::TransitionToAccessorProperty(
-        map, name, descriptor, ACCESSOR_GETTER, getter, attributes);
-    CHECK(!map->is_deprecated());
-    CHECK(!map->is_dictionary_map());
-
-    descriptor =
-        map->instance_descriptors()->SearchWithCache(isolate, *name, *map);
-    map = Map::TransitionToAccessorProperty(
-        map, name, descriptor, ACCESSOR_SETTER, setter, attributes);
+    map = Map::TransitionToAccessorProperty(isolate, map, name, descriptor,
+                                            getter, setter, attributes);
     CHECK(!map->is_deprecated());
     CHECK(!map->is_dictionary_map());
     return map;