Revision: 21596
Author:   [email protected]
Date:     Mon Jun  2 11:02:06 2014 UTC
Log: Remove PROHIBITS_OVERWRITING as it is subsumed by non-configurable properties.

v8::DontDelete is set for Unforgeable properties, so just not setting PROHIBITS_OVERWRITING should be enough.

The secondary "feature" of not allowing accessors to be installed in extending objects is incorrect and confusing, given that it only applies to accessors but not to regular properties:
Object.defineProperty({__proto__:window}, "location", { value: 10 })
works where
Object.defineProperty({__proto__:window}, "location", { get: function() {} })
doesn't work.

LOG=y
[email protected]

Review URL: https://codereview.chromium.org/306203002
http://code.google.com/p/v8/source/detail?r=21596

Modified:
 /branches/bleeding_edge/include/v8.h
 /branches/bleeding_edge/src/accessors.cc
 /branches/bleeding_edge/src/api.cc
 /branches/bleeding_edge/src/objects-inl.h
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/objects.h
 /branches/bleeding_edge/test/cctest/test-accessors.cc

=======================================
--- /branches/bleeding_edge/include/v8.h        Wed May 28 18:40:04 2014 UTC
+++ /branches/bleeding_edge/include/v8.h        Mon Jun  2 11:02:06 2014 UTC
@@ -2072,11 +2072,7 @@
  * accessors have an explicit access control parameter which specifies
  * the kind of cross-context access that should be allowed.
  *
- * Additionally, for security, accessors can prohibit overwriting by
- * accessors defined in JavaScript.  For objects that have such
- * accessors either locally or in their prototype chain it is not
- * possible to overwrite the accessor by using __defineGetter__ or
- * __defineSetter__ from JavaScript code.
+ * TODO(dcarney): Remove PROHIBITS_OVERWRITING as it is now unused.
  */
 enum AccessControl {
   DEFAULT               = 0,
=======================================
--- /branches/bleeding_edge/src/accessors.cc    Wed May 28 09:58:27 2014 UTC
+++ /branches/bleeding_edge/src/accessors.cc    Mon Jun  2 11:02:06 2014 UTC
@@ -41,7 +41,6 @@
   info->set_property_attributes(attributes);
   info->set_all_can_read(false);
   info->set_all_can_write(false);
-  info->set_prohibits_overwriting(false);
   info->set_name(*name);
   Handle<Object> get = v8::FromCData(isolate, getter);
   Handle<Object> set = v8::FromCData(isolate, setter);
=======================================
--- /branches/bleeding_edge/src/api.cc  Tue May 27 13:20:58 2014 UTC
+++ /branches/bleeding_edge/src/api.cc  Mon Jun  2 11:02:06 2014 UTC
@@ -1144,7 +1144,6 @@
   obj->set_name(*Utils::OpenHandle(*name));
   if (settings & ALL_CAN_READ) obj->set_all_can_read(true);
   if (settings & ALL_CAN_WRITE) obj->set_all_can_write(true);
- if (settings & PROHIBITS_OVERWRITING) obj->set_prohibits_overwriting(true); obj->set_property_attributes(static_cast<PropertyAttributes>(attributes));
   if (!signature.IsEmpty()) {
     obj->set_expected_receiver_type(*Utils::OpenHandle(*signature));
=======================================
--- /branches/bleeding_edge/src/objects-inl.h   Wed May 28 09:58:27 2014 UTC
+++ /branches/bleeding_edge/src/objects-inl.h   Mon Jun  2 11:02:06 2014 UTC
@@ -6391,16 +6391,6 @@
 void AccessorInfo::set_all_can_write(bool value) {
   set_flag(BooleanBit::set(flag(), kAllCanWriteBit, value));
 }
-
-
-bool AccessorInfo::prohibits_overwriting() {
-  return BooleanBit::get(flag(), kProhibitsOverwritingBit);
-}
-
-
-void AccessorInfo::set_prohibits_overwriting(bool value) {
-  set_flag(BooleanBit::set(flag(), kProhibitsOverwritingBit, value));
-}


 PropertyAttributes AccessorInfo::property_attributes() {
@@ -6428,9 +6418,6 @@
 void AccessorPair::set_access_flags(v8::AccessControl access_control) {
   int current = access_flags()->value();
   current = BooleanBit::set(current,
-                            kProhibitsOverwritingBit,
-                            access_control & PROHIBITS_OVERWRITING);
-  current = BooleanBit::set(current,
                             kAllCanReadBit,
                             access_control & ALL_CAN_READ);
   current = BooleanBit::set(current,
@@ -6448,11 +6435,6 @@
 bool AccessorPair::all_can_write() {
   return BooleanBit::get(access_flags(), kAllCanWriteBit);
 }
-
-
-bool AccessorPair::prohibits_overwriting() {
-  return BooleanBit::get(access_flags(), kProhibitsOverwritingBit);
-}


 template<typename Derived, typename Shape, typename Key>
=======================================
--- /branches/bleeding_edge/src/objects.cc      Mon Jun  2 10:59:11 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc      Mon Jun  2 11:02:06 2014 UTC
@@ -6326,20 +6326,6 @@
   }
   result->NotFound();
 }
-
-
-// Search object and its prototype chain for callback properties.
-void JSObject::LookupCallbackProperty(Handle<Name> name, LookupResult* result) {
-  DisallowHeapAllocation no_gc;
-  Handle<Object> null_value = GetIsolate()->factory()->null_value();
-  for (Object* current = this;
-       current != *null_value && current->IsJSObject();
-       current = JSObject::cast(current)->GetPrototype()) {
-    JSObject::cast(current)->LookupOwnRealNamedProperty(name, result);
-    if (result->IsPropertyCallbacks()) return;
-  }
-  result->NotFound();
-}


 static bool ContainsOnlyValidKeys(Handle<FixedArray> array) {
@@ -6725,32 +6711,6 @@
   SetPropertyCallback(object, name, accessors, attributes);
 }

-
-bool JSObject::CanSetCallback(Handle<JSObject> object, Handle<Name> name) {
-  Isolate* isolate = object->GetIsolate();
-  ASSERT(!object->IsAccessCheckNeeded() ||
-         isolate->MayNamedAccess(object, name, v8::ACCESS_SET));
-
-  // Check if there is an API defined callback object which prohibits
-  // callback overwriting in this object or its prototype chain.
-  // This mechanism is needed for instance in a browser setting, where
-  // certain accessors such as window.location should not be allowed
-  // to be overwritten because allowing overwriting could potentially
-  // cause security problems.
-  LookupResult callback_result(isolate);
-  object->LookupCallbackProperty(name, &callback_result);
-  if (callback_result.IsFound()) {
-    Object* callback_obj = callback_result.GetCallbackObject();
-    if (callback_obj->IsAccessorInfo()) {
-      return !AccessorInfo::cast(callback_obj)->prohibits_overwriting();
-    }
-    if (callback_obj->IsAccessorPair()) {
-      return !AccessorPair::cast(callback_obj)->prohibits_overwriting();
-    }
-  }
-  return true;
-}
-

 bool Map::DictionaryElementsInPrototypeChainOnly() {
   Heap* heap = GetHeap();
@@ -6878,8 +6838,6 @@
   // Try to flatten before operating on the string.
   if (name->IsString()) name = String::Flatten(Handle<String>::cast(name));

-  if (!JSObject::CanSetCallback(object, name)) return;
-
   uint32_t index = 0;
   bool is_element = name->AsArrayIndex(&index);

@@ -7051,10 +7009,6 @@
   // Try to flatten before operating on the string.
   if (name->IsString()) name = String::Flatten(Handle<String>::cast(name));

-  if (!JSObject::CanSetCallback(object, name)) {
-    return factory->undefined_value();
-  }
-
   uint32_t index = 0;
   bool is_element = name->AsArrayIndex(&index);

=======================================
--- /branches/bleeding_edge/src/objects.h       Mon Jun  2 10:59:11 2014 UTC
+++ /branches/bleeding_edge/src/objects.h       Mon Jun  2 11:02:06 2014 UTC
@@ -2441,7 +2441,6 @@
   void LookupRealNamedProperty(Handle<Name> name, LookupResult* result);
   void LookupRealNamedPropertyInPrototypes(Handle<Name> name,
                                            LookupResult* result);
-  void LookupCallbackProperty(Handle<Name> name, LookupResult* result);

// Returns the number of properties on this object filtering out properties
   // with the specified attributes (ignoring interceptors).
@@ -10373,9 +10372,6 @@
   inline bool all_can_write();
   inline void set_all_can_write(bool value);

-  inline bool prohibits_overwriting();
-  inline void set_prohibits_overwriting(bool value);
-
   inline PropertyAttributes property_attributes();
   inline void set_property_attributes(PropertyAttributes attributes);

@@ -10402,8 +10398,7 @@
   // Bit positions in flag.
   static const int kAllCanReadBit = 0;
   static const int kAllCanWriteBit = 1;
-  static const int kProhibitsOverwritingBit = 2;
-  class AttributesField: public BitField<PropertyAttributes, 3, 3> {};
+  class AttributesField: public BitField<PropertyAttributes, 2, 3> {};

   DISALLOW_IMPLICIT_CONSTRUCTORS(AccessorInfo);
 };
@@ -10568,7 +10563,6 @@
   inline void set_access_flags(v8::AccessControl access_control);
   inline bool all_can_read();
   inline bool all_can_write();
-  inline bool prohibits_overwriting();

   static inline AccessorPair* cast(Object* obj);

@@ -10611,7 +10605,6 @@
  private:
   static const int kAllCanReadBit = 0;
   static const int kAllCanWriteBit = 1;
-  static const int kProhibitsOverwritingBit = 2;

// Strangely enough, in addition to functions and harmony proxies, the spec
   // requires us to consider undefined as a kind of accessor, too:
=======================================
--- /branches/bleeding_edge/test/cctest/test-accessors.cc Fri Feb 14 14:13:06 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-accessors.cc Mon Jun 2 11:02:06 2014 UTC
@@ -227,54 +227,6 @@
     CHECK_EQ(v8::Integer::New(isolate, i/2), entry);
   }
 }
-
-
-static void AccessorProhibitsOverwritingGetter(
-    Local<String> name,
-    const v8::PropertyCallbackInfo<v8::Value>& info) {
-  ApiTestFuzzer::Fuzz();
-  info.GetReturnValue().Set(true);
-}
-
-
-THREADED_TEST(AccessorProhibitsOverwriting) {
-  LocalContext context;
-  v8::Isolate* isolate = context->GetIsolate();
-  v8::HandleScope scope(isolate);
-  Local<ObjectTemplate> templ = ObjectTemplate::New(isolate);
-  templ->SetAccessor(v8_str("x"),
-                     AccessorProhibitsOverwritingGetter,
-                     0,
-                     v8::Handle<Value>(),
-                     v8::PROHIBITS_OVERWRITING,
-                     v8::ReadOnly);
-  Local<v8::Object> instance = templ->NewInstance();
-  context->Global()->Set(v8_str("obj"), instance);
-  Local<Value> value = CompileRun(
-      "obj.__defineGetter__('x', function() { return false; });"
-      "obj.x");
-  CHECK(value->BooleanValue());
-  value = CompileRun(
-      "var setter_called = false;"
-      "obj.__defineSetter__('x', function() { setter_called = true; });"
-      "obj.x = 42;"
-      "setter_called");
-  CHECK(!value->BooleanValue());
-  value = CompileRun(
-      "obj2 = {};"
-      "obj2.__proto__ = obj;"
-      "obj2.__defineGetter__('x', function() { return false; });"
-      "obj2.x");
-  CHECK(value->BooleanValue());
-  value = CompileRun(
-      "var setter_called = false;"
-      "obj2 = {};"
-      "obj2.__proto__ = obj;"
-      "obj2.__defineSetter__('x', function() { setter_called = true; });"
-      "obj2.x = 42;"
-      "setter_called");
-  CHECK(!value->BooleanValue());
-}


 template <int C>

--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to