Revision: 12923
Author: [email protected]
Date: Fri Nov 9 08:14:42 2012
Log: Implement Object.getNotifier() and remove Object.notify()
Updated all tests to use getNotifier or actual object mutation instead of
notify, and added tests for new behavior of getNotifier.
Review URL: https://codereview.chromium.org/11369154
Patch from Adam Klein <[email protected]>.
http://code.google.com/p/v8/source/detail?r=12923
Modified:
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/src/object-observe.js
/branches/bleeding_edge/test/cctest/test-object-observe.cc
/branches/bleeding_edge/test/mjsunit/harmony/object-observe.js
=======================================
--- /branches/bleeding_edge/src/messages.js Fri Nov 9 00:22:02 2012
+++ /branches/bleeding_edge/src/messages.js Fri Nov 9 08:14:42 2012
@@ -96,7 +96,8 @@
observe_non_object: ["Object.", "%0", " cannot ", "%0", "
non-object"],
observe_non_function: ["Object.", "%0", " cannot deliver to
non-function"],
observe_callback_frozen: ["Object.observe cannot deliver to a
frozen function object"],
- observe_type_non_string: ["Object.notify provided changeRecord
with non-string 'type' property"],
+ observe_type_non_string: ["Invalid changeRecord with
non-string 'type' property"],
+ observe_notify_non_notifier: ["notify called on non-notifier object"],
// RangeError
invalid_array_length: ["Invalid array length"],
stack_overflow: ["Maximum call stack size exceeded"],
=======================================
--- /branches/bleeding_edge/src/object-observe.js Fri Nov 9 02:51:35 2012
+++ /branches/bleeding_edge/src/object-observe.js Fri Nov 9 08:14:42 2012
@@ -34,6 +34,7 @@
if (IS_UNDEFINED(observationState.observerInfoMap)) {
observationState.observerInfoMap = %CreateObjectHashTable();
observationState.objectInfoMap = %CreateObjectHashTable();
+ observationState.notifierTargetMap = %CreateObjectHashTable();
observationState.activeObservers = new InternalArray;
observationState.observerPriority = 0;
}
@@ -57,6 +58,16 @@
var observerInfoMap = new InternalObjectHashTable('observerInfoMap');
var objectInfoMap = new InternalObjectHashTable('objectInfoMap');
+var notifierTargetMap = new InternalObjectHashTable('notifierTargetMap');
+
+function CreateObjectInfo(object) {
+ var info = {
+ changeObservers: new InternalArray,
+ notifier: null,
+ };
+ objectInfoMap.set(object, info);
+ return info;
+}
function ObjectObserve(object, callback) {
if (!IS_SPEC_OBJECT(object))
@@ -75,13 +86,8 @@
var objectInfo = objectInfoMap.get(object);
if (IS_UNDEFINED(objectInfo)) {
- // TODO: setup objectInfo.notifier
- objectInfo = {
- changeObservers: new InternalArray(callback)
- };
- objectInfoMap.set(object, objectInfo);
+ objectInfo = CreateObjectInfo(object);
%SetIsObserved(object, true);
- return;
}
var changeObservers = objectInfo.changeObservers;
@@ -130,17 +136,28 @@
EnqueueChangeRecord(changeRecord, objectInfo.changeObservers);
}
-function ObjectNotify(object, changeRecord) {
- // TODO: notifier needs to be [[THIS]]
+var notifierPrototype = {};
+
+function ObjectNotifierNotify(changeRecord) {
+ if (!IS_SPEC_OBJECT(this))
+ throw MakeTypeError("called_on_non_object", ["notify"]);
+
+ var target = notifierTargetMap.get(this);
+ if (IS_UNDEFINED(target))
+ throw MakeTypeError("observe_notify_non_notifier");
+
if (!IS_STRING(changeRecord.type))
throw MakeTypeError("observe_type_non_string");
- var objectInfo = objectInfoMap.get(object);
+ var objectInfo = objectInfoMap.get(target);
if (IS_UNDEFINED(objectInfo))
return;
+ if (!objectInfo.changeObservers.length)
+ return;
+
var newRecord = {
- object: object // TODO: Needs to be 'object' retrieved from notifier
+ object: target
};
for (var prop in changeRecord) {
if (prop === 'object')
@@ -151,6 +168,27 @@
EnqueueChangeRecord(newRecord, objectInfo.changeObservers);
}
+
+function ObjectGetNotifier(object) {
+ if (!IS_SPEC_OBJECT(object))
+ throw MakeTypeError("observe_non_object", ["getNotifier"]);
+
+ if (InternalObjectIsFrozen(object))
+ return null;
+
+ var objectInfo = objectInfoMap.get(object);
+ if (IS_UNDEFINED(objectInfo))
+ objectInfo = CreateObjectInfo(object);
+
+ if (IS_NULL(objectInfo.notifier)) {
+ objectInfo.notifier = {
+ __proto__: notifierPrototype
+ };
+ notifierTargetMap.set(objectInfo.notifier, object);
+ }
+
+ return objectInfo.notifier;
+}
function DeliverChangeRecordsForObserver(observer) {
var observerInfo = observerInfoMap.get(observer);
@@ -190,10 +228,13 @@
%CheckIsBootstrapping();
InstallFunctions($Object, DONT_ENUM, $Array(
"deliverChangeRecords", ObjectDeliverChangeRecords,
- "notify", ObjectNotify, // TODO: Remove when getNotifier is
implemented.
+ "getNotifier", ObjectGetNotifier,
"observe", ObjectObserve,
"unobserve", ObjectUnobserve
));
+ InstallFunctions(notifierPrototype, DONT_ENUM, $Array(
+ "notify", ObjectNotifierNotify
+ ));
}
SetupObjectObserve();
=======================================
--- /branches/bleeding_edge/test/cctest/test-object-observe.cc Fri Nov 9
02:51:35 2012
+++ /branches/bleeding_edge/test/cctest/test-object-observe.cc Fri Nov 9
08:14:42 2012
@@ -64,20 +64,20 @@
Handle<Value> observer = CompileRun("observer");
Handle<Value> obj = CompileRun("obj");
Handle<Value> notify_fun1 = CompileRun(
- "(function() { Object.notify(obj, {type: 'a'}); })");
+ "(function() { obj.foo = 'bar'; })");
Handle<Value> notify_fun2;
{
LocalContext context2;
context2->Global()->Set(String::New("obj"), obj);
notify_fun2 = CompileRun(
- "(function() { Object.notify(obj, {type: 'b'}); })");
+ "(function() { obj.foo = 'baz'; })");
}
Handle<Value> notify_fun3;
{
LocalContext context3;
context3->Global()->Set(String::New("obj"), obj);
notify_fun3 = CompileRun(
- "(function() { Object.notify(obj, {type: 'c'}); })");
+ "(function() { obj.foo = 'bat'; })");
}
{
LocalContext context4;
@@ -100,7 +100,7 @@
"var count = 0;"
"var observer = function(records) { count = records.length };"
"Object.observe(obj, observer);"
- "Object.notify(obj, {type: 'a'});");
+ "obj.foo = 'bar';");
CHECK_EQ(1, CompileRun("count")->Int32Value());
}
@@ -118,7 +118,7 @@
"Object.observe(obj1, observer1);"
"Object.observe(obj1, observer2);"
"Object.observe(obj1, observer3);"
- "Object.notify(obj1, {type: 'a'});");
+ "obj1.foo = 'bar';");
CHECK_EQ(3, CompileRun("ordering.length")->Int32Value());
CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
@@ -128,7 +128,7 @@
"Object.observe(obj2, observer3);"
"Object.observe(obj2, observer2);"
"Object.observe(obj2, observer1);"
- "Object.notify(obj2, {type: 'b'});");
+ "obj2.foo = 'baz'");
CHECK_EQ(3, CompileRun("ordering.length")->Int32Value());
CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
@@ -146,7 +146,7 @@
"function observer1() { ordering.push(1); };"
"function observer2() {"
" if (!reentered) {"
- " Object.notify(obj, {type: 'b'});"
+ " obj.foo = 'baz';"
" reentered = true;"
" }"
" ordering.push(2);"
@@ -155,7 +155,7 @@
"Object.observe(obj, observer1);"
"Object.observe(obj, observer2);"
"Object.observe(obj, observer3);"
- "Object.notify(obj, {type: 'a'});");
+ "obj.foo = 'bar';");
CHECK_EQ(5, CompileRun("ordering.length")->Int32Value());
CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Fri Nov
9 04:28:22 2012
+++ /branches/bleeding_edge/test/mjsunit/harmony/object-observe.js Fri Nov
9 08:14:42 2012
@@ -101,27 +101,42 @@
// Object.unobserve
assertThrows(function() { Object.unobserve(4, observer.callback); },
TypeError);
-// Object.notify
-assertThrows(function() { Object.notify(obj, {}); }, TypeError);
-assertThrows(function() { Object.notify(obj, { type: 4 }); }, TypeError);
+// Object.getNotifier
+var notifier = Object.getNotifier(obj);
+assertSame(notifier, Object.getNotifier(obj));
+assertEquals(null, Object.getNotifier(Object.freeze({})));
+assertFalse(notifier.hasOwnProperty('notify'));
+assertEquals([], Object.keys(notifier));
+var notifyDesc =
Object.getOwnPropertyDescriptor(notifier.__proto__, 'notify');
+assertTrue(notifyDesc.configurable);
+assertTrue(notifyDesc.writable);
+assertFalse(notifyDesc.enumerable);
+assertThrows(function() { notifier.notify({}); }, TypeError);
+assertThrows(function() { notifier.notify({ type: 4 }); }, TypeError);
+var notify = notifier.notify;
+assertThrows(function() { notify.call(undefined, { type: 'a' }); },
TypeError);
+assertThrows(function() { notify.call(null, { type: 'a' }); }, TypeError);
+assertThrows(function() { notify.call(5, { type: 'a' }); }, TypeError);
+assertThrows(function() { notify.call('hello', { type: 'a' }); },
TypeError);
+assertThrows(function() { notify.call(false, { type: 'a' }); }, TypeError);
+assertThrows(function() { notify.call({}, { type: 'a' }); }, TypeError);
assertFalse(recordCreated);
-Object.notify(obj, changeRecordWithAccessor);
-assertFalse(recordCreated);
+notifier.notify(changeRecordWithAccessor);
+assertFalse(recordCreated); // not observed yet
// Object.deliverChangeRecords
assertThrows(function() { Object.deliverChangeRecords(nonFunction); },
TypeError);
// Multiple records are delivered.
Object.observe(obj, observer.callback);
-Object.notify(obj, {
- object: obj,
+notifier.notify({
type: 'updated',
name: 'foo',
expando: 1
});
-Object.notify(obj, {
- object: obj,
+notifier.notify({
+ object: notifier, // object property is ignored
type: 'deleted',
name: 'bar',
expando2: 'str'
@@ -141,7 +156,7 @@
reset();
Object.observe(obj, observer.callback);
Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
});
Object.deliverChangeRecords(observer.callback);
@@ -150,7 +165,7 @@
// Observation can be stopped.
reset();
Object.unobserve(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
});
Object.deliverChangeRecords(observer.callback);
@@ -160,7 +175,7 @@
reset();
Object.unobserve(obj, observer.callback);
Object.unobserve(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
});
Object.deliverChangeRecords(observer.callback);
@@ -168,11 +183,11 @@
// Re-observation works and only includes changeRecords after of call.
reset();
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
});
Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
});
records = undefined;
@@ -182,31 +197,31 @@
// Observing a continuous stream of changes, while itermittantly
unobserving.
reset();
Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
val: 1
});
Object.unobserve(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
val: 2
});
Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
val: 3
});
Object.unobserve(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
val: 4
});
Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo',
val: 5
});
@@ -226,13 +241,13 @@
Object.observe(obj, observer.callback);
Object.observe(obj3, observer.callback);
Object.observe(obj2, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
type: 'foo1',
});
-Object.notify(obj2, {
+Object.getNotifier(obj2).notify({
type: 'foo2',
});
-Object.notify(obj3, {
+Object.getNotifier(obj3).notify({
type: 'foo3',
});
Object.observe(obj3, observer.callback);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
