[v8-dev] Remove duplicate flattening. Defining accessors doesn't call out, so don't assert that the context … (issue 1233073003 by verwaest@chromium.org)

[verwaest]

Reviewers: Igor Sheludko,

Message:
ptal
Description:
Remove duplicate flattening. Defining accessors doesn't call out, so don't
assert that the context doesn't change.

BUG=v8:4137
LOG=n

Please review this at https://codereview.chromium.org/1233073003/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+9, -21 lines):
  M src/objects.cc


Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index  
934c8f39335e3dcdecfcf55ede0e0f3220ea540c..d60fb7c1aaa356611a6d8d8f4115a2cfeb16dbef  
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -6260,13 +6260,6 @@ MaybeHandle<Object>  
JSObject::DefineAccessor(Handle<JSObject> object,
                                              PropertyAttributes  
attributes) {
   Isolate* isolate = object->GetIsolate();

-  // Make sure that the top context does not change when doing callbacks or
-  // interceptor calls.
-  AssertNoContextChange ncc(isolate);
-
-  // Try to flatten before operating on the string.
-  if (name->IsString()) name = String::Flatten(Handle<String>::cast(name));
-
   LookupIterator it = LookupIterator::PropertyOrElement(
       isolate, object, name, LookupIterator::HIDDEN_SKIP_INTERCEPTOR);

@@ -6310,6 +6303,8 @@ MaybeHandle<Object>  
JSObject::DefineAccessor(Handle<JSObject> object,
   }

   if (is_observed) {
+    // Make sure the top context isn't changed.
+    AssertNoContextChange ncc(isolate);
     const char* type = preexists ? "reconfigure" : "add";
     RETURN_ON_EXCEPTION(
         isolate, EnqueueChangeRecord(object, type, name, old_value),  
Object);
@@ -6322,14 +6317,7 @@ MaybeHandle<Object>  
JSObject::DefineAccessor(Handle<JSObject> object,
 MaybeHandle<Object> JSObject::SetAccessor(Handle<JSObject> object,
                                           Handle<AccessorInfo> info) {
   Isolate* isolate = object->GetIsolate();
-
-  // Make sure that the top context does not change when doing callbacks or
-  // interceptor calls.
-  AssertNoContextChange ncc(isolate);
-
-  // Try to flatten before operating on the string.
-  Handle<Name> name(Name::cast(info->name()));
-  if (name->IsString()) name = String::Flatten(Handle<String>::cast(name));
+  Handle<Name> name(Name::cast(info->name()), isolate);

   LookupIterator it = LookupIterator::PropertyOrElement(
       isolate, object, name, LookupIterator::HIDDEN_SKIP_INTERCEPTOR);
@@ -6348,6 +6336,12 @@ MaybeHandle<Object>  
JSObject::SetAccessor(Handle<JSObject> object,
     it.Next();
   }

+  // Ignore accessors on typed arrays.
+  if (it.IsElement() && (object->HasFixedTypedArrayElements() ||
+                         object->HasExternalArrayElements())) {
+    return it.factory()->undefined_value();
+  }
+
   CHECK(GetPropertyAttributes(&it).IsJust());

   // ES5 forbids turning a property into an accessor if it's not
@@ -6356,12 +6350,6 @@ MaybeHandle<Object>  
JSObject::SetAccessor(Handle<JSObject> object,
     return it.factory()->undefined_value();
   }

-  // Ignore accessors on typed arrays.
-  if (it.IsElement() && (object->HasFixedTypedArrayElements() ||
-                         object->HasExternalArrayElements())) {
-    return it.factory()->undefined_value();
-  }
-
   it.TransitionToAccessorPair(info, info->property_attributes());

   return object;


--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- 
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.