Reviewers: rossberg,
Description:
Move notification of JSArray length changes to JSArray::SetElementsLength
The previous implementation in Accessors::ArraySetLength failed when array
length was set through StoreIC_ArrayLength. But that stub and the accessor
both
delegate to JSArray::SetElementsLength, so moving the code there allows
notifications to be sent in both cases.
Please review this at https://codereview.chromium.org/11275292/
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/accessors.cc
M src/objects.cc
Index: src/accessors.cc
diff --git a/src/accessors.cc b/src/accessors.cc
index
c2f245cd9a516411770aba93da70afe159541830..855ddb22b6731e7c59caadbcf90157678968ae40
100644
--- a/src/accessors.cc
+++ b/src/accessors.cc
@@ -95,47 +95,6 @@ Object* Accessors::FlattenNumber(Object* value) {
}
-static MaybeObject* ArraySetLengthObserved(Isolate* isolate,
- Handle<JSArray> array,
- Handle<Object>
new_length_handle) {
- List<Handle<String> > indices;
- List<Handle<Object> > old_values;
- Handle<Object> old_length_handle(array->length(), isolate);
- uint32_t old_length = 0;
- CHECK(old_length_handle->ToArrayIndex(&old_length));
- uint32_t new_length = 0;
- CHECK(new_length_handle->ToArrayIndex(&new_length));
- // TODO(adamk): This loop can be very slow for arrays in dictionary mode.
- // Find another way to iterate over arrays with dictionary elements.
- for (uint32_t i = old_length - 1; i + 1 > new_length; --i) {
- PropertyAttributes attributes = array->GetLocalElementAttribute(i);
- if (attributes == ABSENT) continue;
- // A non-configurable property will cause the truncation operation to
- // stop at this index.
- if (attributes == DONT_DELETE) break;
- // TODO(adamk): Don't fetch the old value if it's an accessor.
- old_values.Add(Object::GetElement(array, i));
- indices.Add(isolate->factory()->Uint32ToString(i));
- }
-
- MaybeObject* result = array->SetElementsLength(*new_length_handle);
- Handle<Object> hresult;
- if (!result->ToHandle(&hresult)) return result;
-
- CHECK(array->length()->ToArrayIndex(&new_length));
- if (old_length != new_length) {
- for (int i = 0; i < indices.length(); ++i) {
- JSObject::EnqueueChangeRecord(
- array, "deleted", indices[i], old_values[i]);
- }
- JSObject::EnqueueChangeRecord(
- array, "updated", isolate->factory()->length_symbol(),
- old_length_handle);
- }
- return *hresult;
-}
-
-
MaybeObject* Accessors::ArraySetLength(JSObject* object, Object* value,
void*) {
Isolate* isolate = object->GetIsolate();
@@ -163,11 +122,7 @@ MaybeObject* Accessors::ArraySetLength(JSObject*
object, Object* value, void*) {
if (has_exception) return Failure::Exception();
if (uint32_v->Number() == number_v->Number()) {
- if (FLAG_harmony_observation && array_handle->map()->is_observed()) {
- return ArraySetLengthObserved(isolate, array_handle, uint32_v);
- } else {
- return array_handle->SetElementsLength(*uint32_v);
- }
+ return array_handle->SetElementsLength(*uint32_v);
}
return isolate->Throw(
*isolate->factory()->NewRangeError("invalid_array_length",
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
a03054653aa16cf014d8ffd83783a7ccd4d7c223..23ff2e274ae5e9abd9e3c721e1065d45006632c0
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -9378,7 +9378,51 @@ void JSArray::Expand(int required_size) {
MaybeObject* JSArray::SetElementsLength(Object* len) {
// We should never end in here with a pixel or external array.
ASSERT(AllowsSetElementsLength());
- return GetElementsAccessor()->SetLength(this, len);
+ if (!(FLAG_harmony_observation && map()->is_observed()))
+ return GetElementsAccessor()->SetLength(this, len);
+
+ Isolate* isolate = GetIsolate();
+ HandleScope scope(isolate);
+ Handle<JSArray> self(this);
+ List<Handle<String> > indices;
+ List<Handle<Object> > old_values;
+ Handle<Object> old_length_handle(self->length());
+ Handle<Object> new_length_handle(len);
+ uint32_t old_length = 0;
+ CHECK(old_length_handle->ToArrayIndex(&old_length));
+ uint32_t new_length = 0;
+ if (!new_length_handle->ToArrayIndex(&new_length))
+ return Failure::InternalError();
+
+ // TODO(adamk): This loop can be very slow for arrays in dictionary mode.
+ // Find another way to iterate over arrays with dictionary elements.
+ for (uint32_t i = old_length - 1; i + 1 > new_length; --i) {
+ PropertyAttributes attributes = self->GetLocalElementAttribute(i);
+ if (attributes == ABSENT) continue;
+ // A non-configurable property will cause the truncation operation to
+ // stop at this index.
+ if (attributes == DONT_DELETE) break;
+ // TODO(adamk): Don't fetch the old value if it's an accessor.
+ old_values.Add(Object::GetElement(self, i));
+ indices.Add(isolate->factory()->Uint32ToString(i));
+ }
+
+ MaybeObject* result =
+ self->GetElementsAccessor()->SetLength(*self, *new_length_handle);
+ Handle<Object> hresult;
+ if (!result->ToHandle(&hresult)) return result;
+
+ CHECK(self->length()->ToArrayIndex(&new_length));
+ if (old_length != new_length) {
+ for (int i = 0; i < indices.length(); ++i) {
+ JSObject::EnqueueChangeRecord(
+ self, "deleted", indices[i], old_values[i]);
+ }
+ JSObject::EnqueueChangeRecord(
+ self, "updated", isolate->factory()->length_symbol(),
+ old_length_handle);
+ }
+ return *hresult;
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
