Properly handle-ify method calls to map() and GetLocalElementAccessorPair()

These are likely causing some of the flaky crashes in Object.observe code. I've reorganized some of the code to minimize the number of necessary calls to map() (by saving the result of map()->is_observed() in a local bool).

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

Review URL: https://chromiumcodereview.appspot.com/11316202

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13070 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/objects.cc b/src/objects.cc
index 8d9f328..8328bdc 100644 (file)
--- 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
+  bool should_enqueue_change_record = false;
+  if (FLAG_harmony_observation && self->map()->is_observed()) {
+    should_enqueue_change_record = self->HasLocalElement(index);
+    if (should_enqueue_change_record) {
+      old_value = self->GetLocalElementAccessorPair(index) != NULL
           ? Handle<Object>::cast(isolate->factory()->the_hole_value())
           : Object::GetElement(self, index);
     }
@@ -4181,9 +4179,9 @@ MaybeObject* JSObject::DeleteElement(uint32_t index, DeleteMode mode) {
   Handle<Object> hresult;
   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 (should_enqueue_change_record && !self->HasLocalElement(index)) {
+    Handle<String> name = isolate->factory()->Uint32ToString(index);
+    EnqueueChangeRecord(self, "deleted", name, old_value);
   }
 
   return *hresult;
@@ -4243,7 +4241,8 @@ MaybeObject* JSObject::DeleteProperty(String* name, DeleteMode mode) {
   Handle<String> hname(name);
 
   Handle<Object> old_value(isolate->heap()->the_hole_value());
-  if (FLAG_harmony_observation && map()->is_observed()) {
+  bool is_observed = FLAG_harmony_observation && self->map()->is_observed();
+  if (is_observed) {
     old_value = handle(lookup.GetLazyValue(), isolate);
   }
   MaybeObject* result;
@@ -4268,9 +4267,8 @@ MaybeObject* JSObject::DeleteProperty(String* name, DeleteMode mode) {
   Handle<Object> hresult;
   if (!result->ToHandle(&hresult, isolate)) return result;
 
-  if (FLAG_harmony_observation && map()->is_observed()) {
-    if (!self->HasLocalProperty(*hname))
-      EnqueueChangeRecord(self, "deleted", hname, old_value);
+  if (is_observed && !self->HasLocalProperty(*hname)) {
+    EnqueueChangeRecord(self, "deleted", hname, old_value);
   }
 
   return *hresult;
@@ -4924,11 +4922,12 @@ MaybeObject* JSObject::DefineAccessor(String* name_raw,
   bool is_element = name->AsArrayIndex(&index);
 
   Handle<Object> old_value = isolate->factory()->the_hole_value();
+  bool is_observed = FLAG_harmony_observation && self->map()->is_observed();
   bool preexists = false;
-  if (FLAG_harmony_observation && map()->is_observed()) {
+  if (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 {
@@ -4946,7 +4945,7 @@ MaybeObject* JSObject::DefineAccessor(String* name_raw,
   Handle<Object> hresult;
   if (!result->ToHandle(&hresult, isolate)) return result;
 
-  if (FLAG_harmony_observation && map()->is_observed()) {
+  if (is_observed) {
     const char* type = preexists ? "reconfigured" : "new";
     EnqueueChangeRecord(self, type, name, old_value);
   }
@@ -10344,7 +10343,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.