Revision: 6562
Author: [email protected]
Date: Tue Feb  1 12:08:01 2011
Log: Revert "Fix bugs 992 and 1083"

This reverts commit 6561 as the new assert caused failures in sputnik.
http://code.google.com/p/v8/source/detail?r=6562

Deleted:
 /branches/bleeding_edge/test/mjsunit/regress/regress-1083.js
 /branches/bleeding_edge/test/mjsunit/regress/regress-992.js
Modified:
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/runtime.cc
 /branches/bleeding_edge/src/v8natives.js
 /branches/bleeding_edge/test/mjsunit/object-define-property.js

=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-1083.js Tue Feb 1 09:08:14 2011
+++ /dev/null
@@ -1,38 +0,0 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//     * Redistributions of source code must retain the above copyright
-//       notice, this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above
-//       copyright notice, this list of conditions and the following
-//       disclaimer in the documentation and/or other materials provided
-//       with the distribution.
-//     * Neither the name of Google Inc. nor the names of its
-//       contributors may be used to endorse or promote products derived
-//       from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// Test that changing the generic descriptor flags on a property
-// on the global object doesn't break invariants.
-Object.defineProperty(this, 'Object', {enumerable:true});
-
-var desc = Object.getOwnPropertyDescriptor(this, 'Object');
-assertTrue(desc.enumerable);
-assertTrue(desc.configurable);
-assertFalse(desc.hasOwnProperty('get'));
-assertFalse(desc.hasOwnProperty('set'));
-assertTrue(desc.hasOwnProperty('value'));
-assertTrue(desc.writable);
=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-992.js Tue Feb 1 09:08:14 2011
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are
-// met:
-//
-//     * Redistributions of source code must retain the above copyright
-//       notice, this list of conditions and the following disclaimer.
-//     * Redistributions in binary form must reproduce the above
-//       copyright notice, this list of conditions and the following
-//       disclaimer in the documentation and/or other materials provided
-//       with the distribution.
-//     * Neither the name of Google Inc. nor the names of its
-//       contributors may be used to endorse or promote products derived
-//       from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// Object.defineProperty with generic desc on existing property
-// should just update enumerable/configurable flags.
-
-var obj =  { get p() { return 42; }  };
-var desc = Object.getOwnPropertyDescriptor(obj, 'p');
-var getter = desc.get;
-
-Object.defineProperty(obj, 'p', {enumerable: false });
-assertEquals(obj.p, 42);
-desc = Object.getOwnPropertyDescriptor(obj, 'p');
-assertFalse(desc.enumerable);
-assertTrue(desc.configurable);
-assertEquals(desc.get, getter);
-assertEquals(desc.set, undefined);
-assertFalse(desc.hasOwnProperty('value'));
-assertFalse(desc.hasOwnProperty('writable'));
=======================================
--- /branches/bleeding_edge/src/objects.cc      Tue Feb  1 09:08:14 2011
+++ /branches/bleeding_edge/src/objects.cc      Tue Feb  1 12:08:01 2011
@@ -2284,9 +2284,6 @@
   // The global object is always normalized.
   ASSERT(!IsGlobalObject());

-  // JSGlobalProxy must never be normalized
-  ASSERT(!IsJSGlobalProxy());
-
   // Allocate new content.
   int property_count = map()->NumberOfDescribedProperties();
   if (expected_additional_properties > 0) {
=======================================
--- /branches/bleeding_edge/src/runtime.cc      Tue Feb  1 09:08:14 2011
+++ /branches/bleeding_edge/src/runtime.cc      Tue Feb  1 12:08:01 2011
@@ -3496,12 +3496,7 @@
                                     args.at<Object>(1));
 }

-// Implements part of 8.12.9 DefineOwnProperty.
-// There are 3 cases that lead here:
-// Step 4b - define a new accessor property.
-// Steps 9c & 12 - replace an existing data property with an accessor property. -// Step 12 - update an existing accessor property with an accessor or generic
-//           descriptor.
+
static MaybeObject* Runtime_DefineOrRedefineAccessorProperty(Arguments args) {
   ASSERT(args.length() == 5);
   HandleScope scope;
@@ -3533,12 +3528,6 @@
   return obj->DefineAccessor(name, flag_setter->value() == 0, fun, attr);
 }

-// Implements part of 8.12.9 DefineOwnProperty.
-// There are 3 cases that lead here:
-// Step 4a - define a new data property.
-// Steps 9b & 12 - replace an existing accessor property with a data property.
-// Step 12 - update an existing data property with a data or generic
-//           descriptor.
 static MaybeObject* Runtime_DefineOrRedefineDataProperty(Arguments args) {
   ASSERT(args.length() == 4);
   HandleScope scope;
@@ -3562,9 +3551,7 @@
   if (((unchecked & (DONT_DELETE | DONT_ENUM | READ_ONLY)) != 0) &&
       is_element) {
     // Normalize the elements to enable attributes on the property.
-    if (!js_object->IsJSGlobalProxy()) {
-      NormalizeElements(js_object);
-    }
+    NormalizeElements(js_object);
     Handle<NumberDictionary> dictionary(js_object->element_dictionary());
     // Make sure that we never go back to fast case.
     dictionary->set_requires_slow_elements();
@@ -3584,9 +3571,7 @@
   if (result.IsProperty() &&
       (attr != result.GetAttributes() || result.type() == CALLBACKS)) {
     // New attributes - normalize to avoid writing to instance descriptor
-    if (!js_object->IsJSGlobalProxy()) {
-      NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
-    }
+    NormalizeProperties(js_object, CLEAR_INOBJECT_PROPERTIES, 0);
     // Use IgnoreAttributes version since a readonly property may be
     // overridden and SetProperty does not allow this.
     return js_object->SetLocalPropertyIgnoreAttributes(*name,
=======================================
--- /branches/bleeding_edge/src/v8natives.js    Tue Feb  1 09:08:14 2011
+++ /branches/bleeding_edge/src/v8natives.js    Tue Feb  1 12:08:01 2011
@@ -545,12 +545,10 @@
   if (IS_UNDEFINED(current) && !extensible)
     throw MakeTypeError("define_disallowed", ["defineProperty"]);

-  if (!IS_UNDEFINED(current)) {
+  if (!IS_UNDEFINED(current) && !current.isConfigurable()) {
     // Step 5 and 6
-    if ((IsGenericDescriptor(desc) ||
-         IsDataDescriptor(desc) == IsDataDescriptor(current)) &&
-        (!desc.hasEnumerable() ||
-         SameValue(desc.isEnumerable(), current.isEnumerable())) &&
+    if ((!desc.hasEnumerable() ||
+         SameValue(desc.isEnumerable() && current.isEnumerable())) &&
         (!desc.hasConfigurable() ||
          SameValue(desc.isConfigurable(), current.isConfigurable())) &&
         (!desc.hasWritable() ||
@@ -563,36 +561,30 @@
          SameValue(desc.getSet(), current.getSet()))) {
       return true;
     }
-    if (!current.isConfigurable()) {
-      // Step 7
-      if (desc.isConfigurable() ||
-          (desc.hasEnumerable() &&
-           desc.isEnumerable() != current.isEnumerable()))
+
+    // Step 7
+ if (desc.isConfigurable() || desc.isEnumerable() != current.isEnumerable())
+      throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
+    // Step 9
+    if (IsDataDescriptor(current) != IsDataDescriptor(desc))
+      throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
+    // Step 10
+    if (IsDataDescriptor(current) && IsDataDescriptor(desc)) {
+      if (!current.isWritable() && desc.isWritable())
         throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
-      // Step 8
-      if (!IsGenericDescriptor(desc)) {
-        // Step 9a
-        if (IsDataDescriptor(current) != IsDataDescriptor(desc))
-          throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
-        // Step 10a
-        if (IsDataDescriptor(current) && IsDataDescriptor(desc)) {
-          if (!current.isWritable() && desc.isWritable())
-            throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
-          if (!current.isWritable() && desc.hasValue() &&
-              !SameValue(desc.getValue(), current.getValue())) {
-            throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
-          }
-        }
-        // Step 11
-        if (IsAccessorDescriptor(desc) && IsAccessorDescriptor(current)) {
- if (desc.hasSetter() && !SameValue(desc.getSet(), current.getSet())){
-            throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
-          }
- if (desc.hasGetter() && !SameValue(desc.getGet(),current.getGet()))
-            throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
-        }
+      if (!current.isWritable() && desc.hasValue() &&
+          !SameValue(desc.getValue(), current.getValue())) {
+        throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
       }
     }
+    // Step 11
+    if (IsAccessorDescriptor(desc) && IsAccessorDescriptor(current)) {
+      if (desc.hasSetter() && !SameValue(desc.getSet(), current.getSet())){
+        throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
+      }
+      if (desc.hasGetter() && !SameValue(desc.getGet(),current.getGet()))
+        throw MakeTypeError("redefine_disallowed", ["defineProperty"]);
+    }
   }

   // Send flags - enumerable and configurable are common - writable is
@@ -615,16 +607,7 @@
   } else
     flag |= DONT_DELETE;

-  if (IsDataDescriptor(desc) ||
-      (IsGenericDescriptor(desc) &&
-       (IS_UNDEFINED(current) || IsDataDescriptor(current)))) {
-    // There are 3 cases that lead here:
-    // Step 4a - defining a new data property.
-    // Steps 9b & 12 - replacing an existing accessor property with a data
-    //                 property.
-    // Step 12 - updating an existing data property with a data or generic
-    //           descriptor.
-
+  if (IsDataDescriptor(desc) || IsGenericDescriptor(desc)) {
     if (desc.hasWritable()) {
       flag |= desc.isWritable() ? 0 : READ_ONLY;
     } else if (!IS_UNDEFINED(current)) {
@@ -632,30 +615,20 @@
     } else {
       flag |= READ_ONLY;
     }
-
     var value = void 0;  // Default value is undefined.
     if (desc.hasValue()) {
       value = desc.getValue();
-    } else if (!IS_UNDEFINED(current) && IsDataDescriptor(current)) {
+    } else if (!IS_UNDEFINED(current)) {
       value = current.getValue();
     }
-
     %DefineOrRedefineDataProperty(obj, p, value, flag);
-  } else if (IsGenericDescriptor(desc)) {
-    // Step 12 - updating an existing accessor property with generic
-    //           descriptor. Changing flags only.
- %DefineOrRedefineAccessorProperty(obj, p, GETTER, current.getGet(), flag);
   } else {
-    // There are 3 cases that lead here:
-    // Step 4b - defining a new accessor property.
-    // Steps 9c & 12 - replacing an existing data property with an accessor
-    //                 property.
-    // Step 12 - updating an existing accessor property with an accessor
-    //           descriptor.
-    if (desc.hasGetter()) {
- %DefineOrRedefineAccessorProperty(obj, p, GETTER, desc.getGet(), flag);
-    }
-    if (desc.hasSetter()) {
+    if (desc.hasGetter() &&
+        (IS_FUNCTION(desc.getGet()) || IS_UNDEFINED(desc.getGet()))) {
+ %DefineOrRedefineAccessorProperty(obj, p, GETTER, desc.getGet(), flag);
+    }
+    if (desc.hasSetter() &&
+        (IS_FUNCTION(desc.getSet()) || IS_UNDEFINED(desc.getSet()))) {
%DefineOrRedefineAccessorProperty(obj, p, SETTER, desc.getSet(), flag);
     }
   }
=======================================
--- /branches/bleeding_edge/test/mjsunit/object-define-property.js Tue Feb 1 09:08:14 2011 +++ /branches/bleeding_edge/test/mjsunit/object-define-property.js Tue Feb 1 12:08:01 2011
@@ -749,32 +749,13 @@
 assertTrue(desc.enumerable);
 assertFalse(desc.configurable);

-// Can use defineProperty to change the value of a non
-// configurable property.
+// Ensure that we can't overwrite the non configurable element.
 try {
   Object.defineProperty(obj6, '2', descElement);
-  desc = Object.getOwnPropertyDescriptor(obj6, '2');
-  assertEquals(desc.value, 'foobar');
-} catch (e) {
   assertUnreachable();
-}
-
-// Ensure that we can't change the descriptor of a
-// non configurable property.
-try {
-  var descAccessor = { get: function() { return 0; } };
-  Object.defineProperty(obj6, '2', descAccessor);
-  assertUnreachable();
 } catch (e) {
   assertTrue(/Cannot redefine property/.test(e));
 }
-
-Object.defineProperty(obj6, '2', descElementNonWritable);
-desc = Object.getOwnPropertyDescriptor(obj6, '2');
-assertEquals(desc.value, 'foofoo');
-assertFalse(desc.writable);
-assertTrue(desc.enumerable);
-assertFalse(desc.configurable);

 Object.defineProperty(obj6, '3', descElementNonWritable);
 desc = Object.getOwnPropertyDescriptor(obj6, '3');
@@ -846,32 +827,13 @@
 assertTrue(desc.enumerable);
 assertFalse(desc.configurable);

-// Can use defineProperty to change the value of a non
-// configurable property of an array.
+// Ensure that we can't overwrite the non configurable element.
 try {
   Object.defineProperty(arr, '2', descElement);
-  desc = Object.getOwnPropertyDescriptor(arr, '2');
-  assertEquals(desc.value, 'foobar');
-} catch (e) {
   assertUnreachable();
-}
-
-// Ensure that we can't change the descriptor of a
-// non configurable property.
-try {
-  var descAccessor = { get: function() { return 0; } };
-  Object.defineProperty(arr, '2', descAccessor);
-  assertUnreachable();
 } catch (e) {
   assertTrue(/Cannot redefine property/.test(e));
 }
-
-Object.defineProperty(arr, '2', descElementNonWritable);
-desc = Object.getOwnPropertyDescriptor(arr, '2');
-assertEquals(desc.value, 'foofoo');
-assertFalse(desc.writable);
-assertTrue(desc.enumerable);
-assertFalse(desc.configurable);

 Object.defineProperty(arr, '3', descElementNonWritable);
 desc = Object.getOwnPropertyDescriptor(arr, '3');
@@ -936,98 +898,3 @@
 assertEquals(undefined, o.x);
 o.x = 37;
 assertEquals(undefined, o.x);
-
-function testDefineProperty(obj, propertyName, desc, resultDesc) {
-  Object.defineProperty(obj, propertyName, desc);
-  var actualDesc = Object.getOwnPropertyDescriptor(obj, propertyName);
-  assertEquals(resultDesc.enumerable, actualDesc.enumerable);
-  assertEquals(resultDesc.configurable, actualDesc.configurable);
-  if (resultDesc.hasOwnProperty('value')) {
-    assertEquals(resultDesc.value, actualDesc.value);
-    assertEquals(resultDesc.writable, actualDesc.writable);
-    assertFalse(resultDesc.hasOwnProperty('get'));
-    assertFalse(resultDesc.hasOwnProperty('set'));
-  } else {
-    assertEquals(resultDesc.get, actualDesc.get);
-    assertEquals(resultDesc.set, actualDesc.set);
-    assertFalse(resultDesc.hasOwnProperty('value'));
-    assertFalse(resultDesc.hasOwnProperty('writable'));
-  }
-}
-
-// tests redefining existing property with a generic descriptor
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { },
-  { value : 42, writable : true, enumerable : true, configurable : true });
-
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { enumerable : true },
-  { value : 42, writable : true, enumerable : true, configurable : true });
-
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { configurable : true },
-  { value : 42, writable : true, enumerable : true, configurable : true });
-
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { enumerable : false },
- { value : 42, writable : true, enumerable : false, configurable : true });
-
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { configurable : false },
- { value : 42, writable : true, enumerable : true, configurable : false });
-
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { enumerable : true, configurable : true },
-  { value : 42, writable : true, enumerable : true, configurable : true });
-
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { enumerable : false, configurable : true },
- { value : 42, writable : true, enumerable : false, configurable : true });
-
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { enumerable : true, configurable : false },
- { value : 42, writable : true, enumerable : true, configurable : false });
-
-o = { p : 42 };
-testDefineProperty(o, 'p',
-  { enumerable : false, configurable : false },
- { value : 42, writable : true, enumerable : false, configurable : false });
-
-// can make a writable, non-configurable field non-writable
-o = { p : 42 };
-Object.defineProperty(o, 'p', { configurable: false });
-testDefineProperty(o, 'p',
-  { writable: false },
- { value : 42, writable : false, enumerable : true, configurable : false });
-
-// redefine of get only property with generic descriptor
-o = {};
-Object.defineProperty(o, 'p',
-  { get : getter1, enumerable: true, configurable: true });
-testDefineProperty(o, 'p',
-  { enumerable : false, configurable : false },
- { get: getter1, set: undefined, enumerable : false, configurable : false });
-
-// redefine of get/set only property with generic descriptor
-o = {};
-Object.defineProperty(o, 'p',
-  { get: getter1, set: setter1, enumerable: true, configurable: true });
-testDefineProperty(o, 'p',
-  { enumerable : false, configurable : false },
- { get: getter1, set: setter1, enumerable : false, configurable : false });
-
-// redefine of set only property with generic descriptor
-o = {};
-Object.defineProperty(o, 'p',
-  { set : setter1, enumerable: true, configurable: true });
-testDefineProperty(o, 'p',
-  { enumerable : false, configurable : false },
- { get: undefined, set: setter1, enumerable : false, configurable : false });

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to