Fix another corner-case behavior of Object::SetSuperProperty.

If the property is a data property on the holder (or does not exist) and is a readonly data property in the receiver, then we must fail.

R=rossberg, verwaest@chromium.org
BUG=

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

Cr-Commit-Position: refs/heads/master@{#31751}

src/objects.cc

@@ -3684,7 +3684,7 @@ Maybe<bool> Object::SetPropertyInternal(LookupIterator* it,
           return WriteToReadOnlyProperty(it, value, should_throw);
         }
         if (it->HolderIsReceiverOrHiddenPrototype()) {
-          return SetDataProperty(it, value, should_throw);
+          return SetDataProperty(it, value);
         }
         done = true;
         break;
@@ -3763,11 +3763,10 @@ Maybe<bool> Object::SetSuperProperty(LookupIterator* it, Handle<Object> value,
 
       case LookupIterator::DATA: {
         PropertyDetails details = own_lookup.property_details();
-        if (details.IsConfigurable() || !details.IsReadOnly()) {
-          return JSObject::DefineOwnPropertyIgnoreAttributes(
-              &own_lookup, value, details.attributes(), should_throw);
+        if (details.IsReadOnly()) {
+          return WriteToReadOnlyProperty(&own_lookup, value, should_throw);
         }
-        return WriteToReadOnlyProperty(&own_lookup, value, should_throw);
+        return SetDataProperty(&own_lookup, value);
       }
 
       case LookupIterator::ACCESSOR: {
@@ -3860,8 +3859,7 @@ Maybe<bool> Object::RedefineIncompatibleProperty(Isolate* isolate,
 }
 
 
-Maybe<bool> Object::SetDataProperty(LookupIterator* it, Handle<Object> value,
-                                    ShouldThrow should_throw) {
+Maybe<bool> Object::SetDataProperty(LookupIterator* it, Handle<Object> value) {
   // Proxies are handled on the WithHandler path. Other non-JSObjects cannot
   // have own properties.
   Handle<JSObject> receiver = Handle<JSObject>::cast(it->GetReceiver());
@@ -4852,7 +4850,7 @@ Maybe<bool> JSObject::DefineOwnPropertyIgnoreAttributes(
         Handle<Object> old_value = it->factory()->the_hole_value();
         // Regular property update if the attributes match.
         if (details.attributes() == attributes) {
-          return SetDataProperty(it, value, should_throw);
+          return SetDataProperty(it, value);
         }
 
         // Special case: properties of typed arrays cannot be reconfigured to

src/objects.h

@@ -1268,8 +1268,7 @@ class Object {
       Isolate* isolate, Handle<Object> name, Handle<Object> value,
       ShouldThrow should_throw);
   MUST_USE_RESULT static Maybe<bool> SetDataProperty(LookupIterator* it,
-                                                     Handle<Object> value,
-                                                     ShouldThrow should_throw);
+                                                     Handle<Object> value);
   MUST_USE_RESULT static Maybe<bool> AddDataProperty(
       LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
       ShouldThrow should_throw, StoreFromKeyed store_mode);

test/mjsunit/es6/super.js

@@ -939,12 +939,8 @@
     mSloppy() {
       assertEquals(42, this.ownReadOnly);
       super.ownReadOnly = 55;
-      assertEquals(55, this.ownReadOnly);
-      var descr = Object.getOwnPropertyDescriptor(this, 'ownReadOnly');
-      assertEquals(55, descr.value);
-      assertTrue(descr.configurable);
-      assertFalse(descr.enumerable);
-      assertFalse(descr.writable);
+      assertSame(undefined, super.ownReadOnly);
+      assertEquals(42, this.ownReadOnly);
       assertFalse(Base.prototype.hasOwnProperty('ownReadOnly'));
 
       assertEquals(15, this.ownReadonlyAccessor);
@@ -962,13 +958,9 @@
     mStrict() {
       'use strict';
       assertEquals(42, this.ownReadOnly);
-      super.ownReadOnly = 55;
-      assertEquals(55, this.ownReadOnly);
-      var descr = Object.getOwnPropertyDescriptor(this, 'ownReadOnly');
-      assertEquals(55, descr.value);
-      assertTrue(descr.configurable);
-      assertFalse(descr.enumerable);
-      assertFalse(descr.writable);
+      assertThrows(() => {super.ownReadOnly = 55}, TypeError);
+      assertSame(undefined, super.ownReadOnly);
+      assertEquals(42, this.ownReadOnly);
       assertFalse(Base.prototype.hasOwnProperty('ownReadOnly'));
 
       assertEquals(15, this.ownReadonlyAccessor);
@@ -1167,12 +1159,8 @@ function TestKeyedSetterCreatingOwnPropertiesReconfigurable(ownReadOnly,
     mSloppy() {
       assertEquals(42, this[ownReadOnly]);
       super[ownReadOnly] = 55;
-      assertEquals(55, this[ownReadOnly]);
-      var descr = Object.getOwnPropertyDescriptor(this, ownReadOnly);
-      assertEquals(55, descr.value);
-      assertTrue(descr.configurable);
-      assertFalse(descr.enumerable);
-      assertFalse(descr.writable);
+      assertSame(undefined, super[ownReadOnly]);
+      assertEquals(42, this[ownReadOnly]);
       assertFalse(Base.prototype.hasOwnProperty(ownReadOnly));
 
       assertEquals(15, this[ownReadonlyAccessor]);
@@ -1190,13 +1178,9 @@ function TestKeyedSetterCreatingOwnPropertiesReconfigurable(ownReadOnly,
     mStrict() {
       'use strict';
       assertEquals(42, this[ownReadOnly]);
-      super[ownReadOnly] = 55;
-      assertEquals(55, this[ownReadOnly]);
-      var descr = Object.getOwnPropertyDescriptor(this, ownReadOnly);
-      assertEquals(55, descr.value);
-      assertTrue(descr.configurable);
-      assertFalse(descr.enumerable);
-      assertFalse(descr.writable);
+      assertThrows(() => {super[ownReadOnly] = 55}, TypeError);
+      assertSame(undefined, super[ownReadOnly]);
+      assertEquals(42, this[ownReadOnly]);
       assertFalse(Base.prototype.hasOwnProperty(ownReadOnly));
 
       assertEquals(15, this[ownReadonlyAccessor]);

test/mjsunit/harmony/reflect.js

@@ -244,8 +244,7 @@ function prepare(target) {
     assertFalse("nowrite" in receiver);
 
     // Data vs Non-Writable
-    // TODO(neis): This must return false but currently doesn't.
-    // assertFalse(Reflect.set({}, "nowrite", value, target));
+    assertFalse(Reflect.set({}, "nowrite", value, target));
 
     // Data vs Accessor
     assertFalse(Reflect.set({}, "unknown", 0, {set unknown(x) {}}));
@@ -264,6 +263,14 @@ function prepare(target) {
     // Accessor vs Non-Object
     assertTrue(Reflect.set(target, "setter2", value, null));
     assertFalse(Reflect.set(target, "getter", value, null));
+
+    let receiver2 = {};
+    Object.defineProperty(receiver2, "bla",
+        {configurable: false, writable: true, value: true});
+    Object.defineProperty(receiver2, "not_in_target",
+        {configurable: false, writable: true, value: true});
+    assertTrue(Reflect.set(target, "bla", value, receiver2));
+    assertTrue(Reflect.set(target, "not_in_target", value, receiver2));
   }
 })();