Revision: 13070
Author: [email protected]
Date: Wed Nov 28 00:35:46 2012
Log: 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
http://code.google.com/p/v8/source/detail?r=13070
Modified:
/branches/bleeding_edge/src/objects.cc
=======================================
--- /branches/bleeding_edge/src/objects.cc Tue Nov 27 04:01:14 2012
+++ /branches/bleeding_edge/src/objects.cc Wed Nov 28 00:35:46 2012
@@ -4157,14 +4157,12 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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
