Remove special ExecutableAccessorInfo handling based on flag

This additionally removes special "prototype" handling for O.o, since it's broken; and added test.

BUG=
R=mvstanton@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22171 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/objects.cc b/src/objects.cc
index 7db464e8c901207c569be2ef6daf8929f924371b..498479d8114a935846192e30d27c06a8c73fa307 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -4219,8 +4219,7 @@ MaybeHandle<Object> JSObject::SetOwnPropertyIgnoreAttributes(
     ValueType value_type,
     StoreMode mode,
     ExtensibilityCheck extensibility_check,
-    StoreFromKeyed store_from_keyed,
-    ExecutableAccessorInfoHandling handling) {
+    StoreFromKeyed store_from_keyed) {
   Isolate* isolate = object->GetIsolate();
 
   // Make sure that the top context does not change when doing callbacks or
@@ -4275,8 +4274,6 @@ MaybeHandle<Object> JSObject::SetOwnPropertyIgnoreAttributes(
     old_attributes = lookup.GetAttributes();
   }
 
-  bool executed_set_prototype = false;
-
   // Check of IsReadOnly removed from here in clone.
   if (lookup.IsTransition()) {
     Handle<Object> result;
@@ -4300,11 +4297,9 @@ MaybeHandle<Object> JSObject::SetOwnPropertyIgnoreAttributes(
           SetPropertyToFieldWithAttributes(&lookup, name, value, attributes);
         }
         break;
-      case CALLBACKS:
-      {
+      case CALLBACKS: {
         Handle<Object> callback(lookup.GetCallbackObject(), isolate);
-        if (callback->IsExecutableAccessorInfo() &&
-            handling == DONT_FORCE_FIELD) {
+        if (callback->IsExecutableAccessorInfo()) {
           Handle<Object> result;
           ASSIGN_RETURN_ON_EXCEPTION(
               isolate, result,
@@ -4316,31 +4311,29 @@ MaybeHandle<Object> JSObject::SetOwnPropertyIgnoreAttributes(
                                                 STRICT),
               Object);
 
-          if (attributes != lookup.GetAttributes()) {
-            Handle<ExecutableAccessorInfo> new_data =
-                Accessors::CloneAccessor(
-                    isolate, Handle<ExecutableAccessorInfo>::cast(callback));
-            new_data->set_property_attributes(attributes);
-            if (attributes & READ_ONLY) {
-              // This way we don't have to introduce a lookup to the setter,
-              // simply make it unavailable to reflect the attributes.
-              new_data->clear_setter();
-            }
+          if (attributes == lookup.GetAttributes()) return result;
+
+          Handle<ExecutableAccessorInfo> new_data =
+              Accessors::CloneAccessor(
+                  isolate, Handle<ExecutableAccessorInfo>::cast(callback));
+          new_data->set_property_attributes(attributes);
+          // This way we don't have to introduce a lookup to the setter, simply
+          // make it unavailable to reflect the attributes.
+          if (attributes & READ_ONLY) new_data->clear_setter();
+          SetPropertyCallback(object, name, new_data, attributes);
 
-            SetPropertyCallback(object, name, new_data, attributes);
-          }
           if (is_observed) {
-            // If we are setting the prototype of a function and are observed,
-            // don't send change records because the prototype handles that
-            // itself.
-            executed_set_prototype = object->IsJSFunction() &&
-                String::Equals(isolate->factory()->prototype_string(),
-                               Handle<String>::cast(name)) &&
-                Handle<JSFunction>::cast(object)->should_have_prototype();
+            Handle<Object> new_value =
+                Object::GetPropertyOrElement(object, name).ToHandleChecked();
+            if (old_value->SameValue(*new_value)) {
+              old_value = isolate->factory()->the_hole_value();
+            }
+            EnqueueChangeRecord(object, "reconfigure", name, old_value);
           }
-        } else {
-          ConvertAndSetOwnProperty(&lookup, name, value, attributes);
+
+          return result;
         }
+        ConvertAndSetOwnProperty(&lookup, name, value, attributes);
         break;
       }
       case NONEXISTENT:
@@ -4350,7 +4343,7 @@ MaybeHandle<Object> JSObject::SetOwnPropertyIgnoreAttributes(
     }
   }
 
-  if (is_observed && !executed_set_prototype) {
+  if (is_observed) {
     if (lookup.IsTransition()) {
       EnqueueChangeRecord(object, "add", name, old_value);
     } else if (old_value->IsTheHole()) {

diff --git a/src/objects.h b/src/objects.h
index 3e83eda46f82b11c190041a7e29b0dd6d12272e9..26b008a9468610b00ab63357aa93a2af3d647b41 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -2148,13 +2148,6 @@ class JSObject: public JSReceiver {
       StrictMode strict_mode,
       StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED);
 
-  // SetLocalPropertyIgnoreAttributes converts callbacks to fields. We need to
-  // grant an exemption to ExecutableAccessor callbacks in some cases.
-  enum ExecutableAccessorInfoHandling {
-    DEFAULT_HANDLING,
-    DONT_FORCE_FIELD
-  };
-
   MUST_USE_RESULT static MaybeHandle<Object> SetOwnPropertyIgnoreAttributes(
       Handle<JSObject> object,
       Handle<Name> key,
@@ -2163,8 +2156,7 @@ class JSObject: public JSReceiver {
       ValueType value_type = OPTIMAL_REPRESENTATION,
       StoreMode mode = ALLOW_AS_CONSTANT,
       ExtensibilityCheck extensibility_check = PERFORM_EXTENSIBILITY_CHECK,
-      StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED,
-      ExecutableAccessorInfoHandling handling = DEFAULT_HANDLING);
+      StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED);
 
   static void AddProperty(Handle<JSObject> object,
                           Handle<Name> key,

diff --git a/src/runtime.cc b/src/runtime.cc
index 949c6176893c07009a34ae13448f3fa68f798453..4516bb6a87debe3d6510b64136f8994365f09aed 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5092,12 +5092,7 @@ RUNTIME_FUNCTION(Runtime_DefineDataPropertyUnchecked) {
     ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
         isolate, result,
         JSObject::SetOwnPropertyIgnoreAttributes(
-            js_object, name, obj_value, attr,
-            Object::OPTIMAL_REPRESENTATION,
-            ALLOW_AS_CONSTANT,
-            JSReceiver::PERFORM_EXTENSIBILITY_CHECK,
-            JSReceiver::MAY_BE_STORE_FROM_KEYED,
-            JSObject::DONT_FORCE_FIELD));
+            js_object, name, obj_value, attr));
     return *result;
   }
 

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 04085961492ded446bc2326a65f72e3d1696ac06..0b50181bcf8c70379b14c815bd05a15d03ba04c6 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -15338,12 +15338,11 @@ TEST(ForceSet) {
   CHECK_EQ(3, global->Get(access_property)->Int32Value());
   CHECK_EQ(1, force_set_set_count);
   CHECK_EQ(2, force_set_get_count);
-  // Forcing the property to be set should override the accessor without
-  // calling it
+  // Forcing the property to be set should call the accessor
   global->ForceSet(access_property, v8::Int32::New(isolate, 8));
-  CHECK_EQ(8, global->Get(access_property)->Int32Value());
-  CHECK_EQ(1, force_set_set_count);
-  CHECK_EQ(2, force_set_get_count);
+  CHECK_EQ(3, global->Get(access_property)->Int32Value());
+  CHECK_EQ(2, force_set_set_count);
+  CHECK_EQ(3, force_set_get_count);
 }
 
 

diff --git a/test/mjsunit/es7/object-observe.js b/test/mjsunit/es7/object-observe.js
index 7bb579f0c1462b6b33ff1bc6c3229067317f2845..920b1fac5f06e182d9049c48d7e94c9c199e1b53 100644 (file)
--- a/test/mjsunit/es7/object-observe.js
+++ b/test/mjsunit/es7/object-observe.js
@@ -1136,7 +1136,8 @@ var objects = [
 //  createProxy(Proxy.create, null),
 //  createProxy(Proxy.createFunction, function(){}),
 ];
-var properties = ["a", "1", 1, "length", "setPrototype", "name", "caller"];
+var properties = ["a", "1", 1, "length", "setPrototype",
+                  "name", "caller", "prototype"];
 
 // Cases that yield non-standard results.
 function blacklisted(obj, prop) {