Reviewers: Toon Verwaest, rossberg,

Description:
Properly handle-ify several calls to GetLocalElementAccessorPair

These are likely causing some of the flaky crashes in Object.observe code.

Also move down an unnecessarily early call to Uint32ToString when sending an
element deletion notification.


Please review this at https://codereview.chromium.org/11316202/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/objects.cc


Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 8d9f328f431fa71a2c9fd443210656c5983b964c..8a432fdd7f6eaa0cd360a35b947f85c0face6d28 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -4157,14 +4157,12 @@ MaybeObject* JSObject::DeleteElement(uint32_t index, DeleteMode mode) {
   HandleScope scope(isolate);
   Handle<JSObject> self(this);

-  Handle<String> name;
   Handle<Object> old_value;
   bool preexists = false;
   if (FLAG_harmony_observation && map()->is_observed()) {
-    name = isolate->factory()->Uint32ToString(index);
     preexists = self->HasLocalElement(index);
     if (preexists) {
-      old_value = GetLocalElementAccessorPair(index) != NULL
+      old_value = self->GetLocalElementAccessorPair(index) != NULL
           ? Handle<Object>::cast(isolate->factory()->the_hole_value())
           : Object::GetElement(self, index);
     }
@@ -4182,8 +4180,11 @@ MaybeObject* JSObject::DeleteElement(uint32_t index, DeleteMode mode) {
   if (!result->ToHandle(&hresult, isolate)) return result;

   if (FLAG_harmony_observation && map()->is_observed()) {
-    if (preexists && !self->HasLocalElement(index))
-      EnqueueChangeRecord(self, "deleted", name, old_value);
+    if (preexists && !self->HasLocalElement(index)) {
+      EnqueueChangeRecord(self, "deleted",
+                          isolate->factory()->Uint32ToString(index),
+                          old_value);
+    }
   }

   return *hresult;
@@ -4928,7 +4929,7 @@ MaybeObject* JSObject::DefineAccessor(String* name_raw,
   if (FLAG_harmony_observation && map()->is_observed()) {
     if (is_element) {
       preexists = HasLocalElement(index);
-      if (preexists && GetLocalElementAccessorPair(index) == NULL) {
+      if (preexists && self->GetLocalElementAccessorPair(index) == NULL) {
         old_value = Object::GetElement(self, index);
       }
     } else {
@@ -10344,7 +10345,7 @@ MaybeObject* JSObject::SetElement(uint32_t index,
   Handle<Object> old_length;

   if (old_attributes != ABSENT) {
-    if (GetLocalElementAccessorPair(index) == NULL)
+    if (self->GetLocalElementAccessorPair(index) == NULL)
       old_value = Object::GetElement(self, index);
   } else if (self->IsJSArray()) {
     // Store old array length in case adding an element grows the array.


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

Reply via email to