Introduce DefineOwnPropertyIgnoreAttributes and make it call SetPropertyWithInterceptor.

Otherwise using Object.defineProperty with window.localStorage will not actually store the value into the database but on the object itself.

BUG=v8:4137
LOG=n

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

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

src/api.cc

@@ -3638,7 +3638,8 @@ static i::MaybeHandle<i::Object> DefineObjectProperty(
     name = i::Handle<i::String>::cast(converted);
   }
 
-  return i::JSObject::DefinePropertyOrElement(js_object, name, value, attrs);
+  return i::JSObject::DefinePropertyOrElementIgnoreAttributes(js_object, name,
+                                                              value, attrs);
 }
 
 

src/json-parser.h

@@ -434,7 +434,8 @@ Handle<Object> JsonParser<seq_one_byte>::ParseJsonObject() {
       // Commit the intermediate state to the object and stop transitioning.
       CommitStateToJsonObject(json_object, map, &properties);
 
-      JSObject::DefinePropertyOrElement(json_object, key, value).Check();
+      JSObject::DefinePropertyOrElementIgnoreAttributes(json_object, key, value)
+          .Check();
     } while (transitioning && MatchSkipWhiteSpace(','));
 
     // If we transitioned until the very end, transition the map now.
@@ -470,7 +471,8 @@ Handle<Object> JsonParser<seq_one_byte>::ParseJsonObject() {
         value = ParseJsonValue();
         if (value.is_null()) return ReportUnexpectedCharacter();
 
-        JSObject::DefinePropertyOrElement(json_object, key, value).Check();
+        JSObject::DefinePropertyOrElementIgnoreAttributes(json_object, key,
+                                                          value).Check();
       }
     }
 

src/objects-inl.h

@@ -1197,22 +1197,6 @@ MaybeHandle<Object> Object::GetProperty(Isolate* isolate,
 }
 
 
-MaybeHandle<Object> JSObject::DefinePropertyOrElement(
-    Handle<JSObject> object, Handle<Name> name, Handle<Object> value,
-    PropertyAttributes attributes, ExecutableAccessorInfoHandling handling) {
-  uint32_t index;
-  if (name->AsArrayIndex(&index)) {
-    return SetOwnElementIgnoreAttributes(object, index, value, attributes,
-                                         handling);
-  }
-
-  // TODO(verwaest): Is this necessary?
-  if (name->IsString()) name = String::Flatten(Handle<String>::cast(name));
-  return SetOwnPropertyIgnoreAttributes(object, name, value, attributes,
-                                        handling);
-}
-
-
 #define FIELD_ADDR(p, offset) \
   (reinterpret_cast<byte*>(p) + offset - kHeapObjectTag)
 

src/objects.cc

@@ -536,10 +536,11 @@ static bool FindAllCanWriteHolder(LookupIterator* it) {
 
 
 MaybeHandle<Object> JSObject::SetPropertyWithFailedAccessCheck(
-    LookupIterator* it, Handle<Object> value, LanguageMode language_mode) {
+    LookupIterator* it, Handle<Object> value) {
   Handle<JSObject> checked = it->GetHolder<JSObject>();
   if (FindAllCanWriteHolder(it)) {
-    return SetPropertyWithAccessor(it, value, language_mode);
+    // The supplied language-mode is ignored by SetPropertyWithAccessor.
+    return SetPropertyWithAccessor(it, value, SLOPPY);
   }
 
   it->isolate()->ReportFailedAccessCheck(checked);
@@ -3044,8 +3045,7 @@ MaybeHandle<Object> Object::SetPropertyInternal(LookupIterator* it,
         if (it->HasAccess()) break;
         // Check whether it makes sense to reuse the lookup iterator. Here it
         // might still call into setters up the prototype chain.
-        return JSObject::SetPropertyWithFailedAccessCheck(it, value,
-                                                          language_mode);
+        return JSObject::SetPropertyWithFailedAccessCheck(it, value);
 
       case LookupIterator::JSPROXY:
         if (it->HolderIsReceiverOrHiddenPrototype()) {
@@ -3165,9 +3165,8 @@ MaybeHandle<Object> Object::SetSuperProperty(LookupIterator* it,
   for (; own_lookup.IsFound(); own_lookup.Next()) {
     switch (own_lookup.state()) {
       case LookupIterator::ACCESS_CHECK:
-        if (!it->isolate()->MayAccess(own_lookup.GetHolder<JSObject>())) {
-          return JSObject::SetPropertyWithFailedAccessCheck(&own_lookup, value,
-                                                            language_mode);
+        if (!own_lookup.HasAccess()) {
+          return JSObject::SetPropertyWithFailedAccessCheck(&own_lookup, value);
         }
         break;
 
@@ -3177,8 +3176,8 @@ MaybeHandle<Object> Object::SetSuperProperty(LookupIterator* it,
       case LookupIterator::DATA: {
         PropertyDetails details = own_lookup.property_details();
         if (details.IsConfigurable() || !details.IsReadOnly()) {
-          return JSObject::ReconfigureAsDataProperty(&own_lookup, value,
-                                                     details.attributes());
+          return JSObject::DefineOwnPropertyIgnoreAttributes(
+              &own_lookup, value, details.attributes());
         }
         return WriteToReadOnlyProperty(&own_lookup, value, language_mode);
       }
@@ -3186,8 +3185,8 @@ MaybeHandle<Object> Object::SetSuperProperty(LookupIterator* it,
       case LookupIterator::ACCESSOR: {
         PropertyDetails details = own_lookup.property_details();
         if (details.IsConfigurable()) {
-          return JSObject::ReconfigureAsDataProperty(&own_lookup, value,
-                                                     details.attributes());
+          return JSObject::DefineOwnPropertyIgnoreAttributes(
+              &own_lookup, value, details.attributes());
         }
 
         return RedefineNonconfigurableProperty(it->isolate(), it->GetName(),
@@ -4131,7 +4130,11 @@ void ExecutableAccessorInfo::ClearSetter(Handle<ExecutableAccessorInfo> info) {
 }
 
 
-MaybeHandle<Object> JSObject::ReconfigureAsDataProperty(
+// Reconfigures a property to a data property with attributes, even if it is not
+// reconfigurable.
+// Requires a LookupIterator that does not look at the prototype chain beyond
+// hidden prototypes.
+MaybeHandle<Object> JSObject::DefineOwnPropertyIgnoreAttributes(
     LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
     ExecutableAccessorInfoHandling handling) {
   Handle<JSObject> object = Handle<JSObject>::cast(it->GetReceiver());
@@ -4139,28 +4142,50 @@ MaybeHandle<Object> JSObject::ReconfigureAsDataProperty(
                      (it->IsElement() ||
                       !it->isolate()->IsInternallyUsedPropertyName(it->name()));
 
+  for (; it->IsFound(); it->Next()) {
     switch (it->state()) {
-    case LookupIterator::INTERCEPTOR:
       case LookupIterator::JSPROXY:
       case LookupIterator::NOT_FOUND:
       case LookupIterator::TRANSITION:
-    case LookupIterator::ACCESS_CHECK:
         UNREACHABLE();
 
+      case LookupIterator::ACCESS_CHECK:
+        if (!it->HasAccess()) {
+          return SetPropertyWithFailedAccessCheck(it, value);
+        }
+        break;
+
+      // If there's an interceptor, try to store the property with the
+      // interceptor.
+      // In case of success, the attributes will have been reset to the default
+      // attributes of the interceptor, rather than the incoming attributes.
+      //
+      // TODO(verwaest): JSProxy afterwards verify the attributes that the
+      // JSProxy claims it has, and verifies that they are compatible. If not,
+      // they throw. Here we should do the same.
+      case LookupIterator::INTERCEPTOR:
+        if (handling == DONT_FORCE_FIELD) {
+          MaybeHandle<Object> maybe_result =
+              JSObject::SetPropertyWithInterceptor(it, value);
+          if (!maybe_result.is_null()) return maybe_result;
+          if (it->isolate()->has_pending_exception()) return maybe_result;
+        }
+        break;
+
       case LookupIterator::INTEGER_INDEXED_EXOTIC:
         return value;
 
       case LookupIterator::ACCESSOR: {
-      PropertyDetails details = it->property_details();
-      // Ensure the context isn't changed after calling into accessors.
-      AssertNoContextChange ncc(it->isolate());
-
         Handle<Object> accessors = it->GetAccessors();
 
         // Special handling for ExecutableAccessorInfo, which behaves like a
         // data property.
         if (accessors->IsExecutableAccessorInfo() &&
             handling == DONT_FORCE_FIELD) {
+          PropertyDetails details = it->property_details();
+          // Ensure the context isn't changed after calling into accessors.
+          AssertNoContextChange ncc(it->isolate());
+
           Handle<Object> result;
           ASSIGN_RETURN_ON_EXCEPTION(
               it->isolate(), result,
@@ -4187,18 +4212,10 @@ MaybeHandle<Object> JSObject::ReconfigureAsDataProperty(
             SetPropertyCallback(it->GetHolder<JSObject>(), it->name(), new_data,
                                 attributes);
           }
-        if (is_observed) {
-          RETURN_ON_EXCEPTION(
-              it->isolate(),
-              EnqueueChangeRecord(object, "reconfigure", it->GetName(),
-                                  it->factory()->the_hole_value()),
-              Object);
-        }
-        return value;
-      }
-
+        } else {
           it->ReconfigureDataProperty(value, attributes);
           it->WriteDataValue(value);
+        }
 
         if (is_observed) {
           RETURN_ON_EXCEPTION(
@@ -4210,7 +4227,6 @@ MaybeHandle<Object> JSObject::ReconfigureAsDataProperty(
 
         return value;
       }
-
       case LookupIterator::DATA: {
         PropertyDetails details = it->property_details();
         Handle<Object> old_value = it->factory()->the_hole_value();
@@ -4242,33 +4258,22 @@ MaybeHandle<Object> JSObject::ReconfigureAsDataProperty(
                                                   it->GetName(), old_value),
                               Object);
         }
+        return value;
+      }
     }
   }
 
-  return value;
+  return AddDataProperty(it, value, attributes, STRICT,
+                         CERTAINLY_NOT_STORE_FROM_KEYED);
 }
 
 
-// Reconfigures a property to a data property with attributes, even if it is not
-// reconfigurable.
 MaybeHandle<Object> JSObject::SetOwnPropertyIgnoreAttributes(
     Handle<JSObject> object, Handle<Name> name, Handle<Object> value,
     PropertyAttributes attributes, ExecutableAccessorInfoHandling handling) {
   DCHECK(!value->IsTheHole());
-  LookupIterator it(object, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
-  if (it.state() == LookupIterator::ACCESS_CHECK) {
-    if (!it.HasAccess()) {
-      return SetPropertyWithFailedAccessCheck(&it, value, SLOPPY);
-    }
-    it.Next();
-  }
-
-  if (it.IsFound()) {
-    return ReconfigureAsDataProperty(&it, value, attributes, handling);
-  }
-
-  return AddDataProperty(&it, value, attributes, STRICT,
-                         CERTAINLY_NOT_STORE_FROM_KEYED);
+  LookupIterator it(object, name, LookupIterator::OWN);
+  return DefineOwnPropertyIgnoreAttributes(&it, value, attributes, handling);
 }
 
 
@@ -4276,21 +4281,21 @@ MaybeHandle<Object> JSObject::SetOwnElementIgnoreAttributes(
     Handle<JSObject> object, uint32_t index, Handle<Object> value,
     PropertyAttributes attributes, ExecutableAccessorInfoHandling handling) {
   Isolate* isolate = object->GetIsolate();
-  LookupIterator it(isolate, object, index,
-                    LookupIterator::OWN_SKIP_INTERCEPTOR);
-  if (it.state() == LookupIterator::ACCESS_CHECK) {
-    if (!it.HasAccess()) {
-      return SetPropertyWithFailedAccessCheck(&it, value, STRICT);
-    }
-    it.Next();
-  }
+  LookupIterator it(isolate, object, index, LookupIterator::OWN);
+  return DefineOwnPropertyIgnoreAttributes(&it, value, attributes, handling);
+}
 
-  if (it.IsFound()) {
-    return ReconfigureAsDataProperty(&it, value, attributes, handling);
-  }
 
-  return AddDataProperty(&it, value, attributes, STRICT,
-                         MAY_BE_STORE_FROM_KEYED);
+MaybeHandle<Object> JSObject::DefinePropertyOrElementIgnoreAttributes(
+    Handle<JSObject> object, Handle<Name> name, Handle<Object> value,
+    PropertyAttributes attributes, ExecutableAccessorInfoHandling handling) {
+  Isolate* isolate = object->GetIsolate();
+  uint32_t index;
+  LookupIterator it =
+      name->AsArrayIndex(&index)
+          ? LookupIterator(isolate, object, index, LookupIterator::OWN)
+          : LookupIterator(object, name, LookupIterator::OWN);
+  return DefineOwnPropertyIgnoreAttributes(&it, value, attributes, handling);
 }
 
 

src/objects.h

@@ -1842,11 +1842,8 @@ class JSObject: public JSReceiver {
   // grant an exemption to ExecutableAccessor callbacks in some cases.
   enum ExecutableAccessorInfoHandling { DEFAULT_HANDLING, DONT_FORCE_FIELD };
 
-  // Calls SetOwn[Property|Element]IgnoreAttributes depending on whether name is
-  // convertible to an index.
-  MUST_USE_RESULT static inline MaybeHandle<Object> DefinePropertyOrElement(
-      Handle<JSObject> object, Handle<Name> name, Handle<Object> value,
-      PropertyAttributes attributes = NONE,
+  MUST_USE_RESULT static MaybeHandle<Object> DefineOwnPropertyIgnoreAttributes(
+      LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
       ExecutableAccessorInfoHandling handling = DEFAULT_HANDLING);
 
   MUST_USE_RESULT static MaybeHandle<Object> SetOwnPropertyIgnoreAttributes(
@@ -1859,8 +1856,12 @@ class JSObject: public JSReceiver {
       PropertyAttributes attributes,
       ExecutableAccessorInfoHandling handling = DEFAULT_HANDLING);
 
-  MUST_USE_RESULT static MaybeHandle<Object> ReconfigureAsDataProperty(
-      LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
+  // Equivalent to one of the above depending on whether |name| can be converted
+  // to an array index.
+  MUST_USE_RESULT static MaybeHandle<Object>
+  DefinePropertyOrElementIgnoreAttributes(
+      Handle<JSObject> object, Handle<Name> name, Handle<Object> value,
+      PropertyAttributes attributes = NONE,
       ExecutableAccessorInfoHandling handling = DEFAULT_HANDLING);
 
   static void AddProperty(Handle<JSObject> object, Handle<Name> name,
@@ -2315,7 +2316,7 @@ class JSObject: public JSReceiver {
       LookupIterator* it);
 
   MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithFailedAccessCheck(
-      LookupIterator* it, Handle<Object> value, LanguageMode language_mode);
+      LookupIterator* it, Handle<Object> value);
 
   // Add a property to a slow-case object.
   static void AddSlowProperty(Handle<JSObject> object,

src/runtime/runtime-classes.cc

@@ -200,7 +200,8 @@ RUNTIME_FUNCTION(Runtime_DefineClassMethod) {
   CONVERT_ARG_HANDLE_CHECKED(Name, name, 1);
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 2);
 
-  RETURN_FAILURE_ON_EXCEPTION(isolate, JSObject::DefinePropertyOrElement(
+  RETURN_FAILURE_ON_EXCEPTION(isolate,
+                              JSObject::DefinePropertyOrElementIgnoreAttributes(
                                   object, name, function, DONT_ENUM));
   return isolate->heap()->undefined_value();
 }