From 91daa127c92e12cf2ef403871ab401dec88c9cba Mon Sep 17 00:00:00 2001 From: "adamk@chromium.org" Date: Wed, 15 May 2013 18:47:48 +0000 Subject: [PATCH] Revert "Implement Object.getNotifier(obj).performChange()" (r14696) Reverts r14696 because it caused debug assertion failures when running test/mjsunit/harmony/object-observe.js TBR=rossberg Review URL: https://codereview.chromium.org/15203002 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14697 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/messages.js | 3 - src/object-observe.js | 194 ++---------------- test/mjsunit/harmony/object-observe.js | 348 ++------------------------------- 3 files changed, 31 insertions(+), 514 deletions(-) diff --git a/src/messages.js b/src/messages.js index 296965d..b9bce1e 100644 --- a/src/messages.js +++ b/src/messages.js @@ -98,10 +98,7 @@ var kMessages = { 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_invalid_accept: ["Object.observe accept must be an array of strings."], observe_type_non_string: ["Invalid changeRecord with non-string 'type' property"], - observe_perform_non_string: ["Invalid non-string changeType"], - observe_perform_non_function: ["Cannot perform non-function"], observe_notify_non_notifier: ["notify called on non-notifier object"], proto_poison_pill: ["Generic use of __proto__ accessor not allowed"], parameterless_typed_array_constr: diff --git a/src/object-observe.js b/src/object-observe.js index c4c6c14..77409b9 100644 --- a/src/object-observe.js +++ b/src/object-observe.js @@ -66,147 +66,18 @@ function CreateObjectInfo(object) { var info = { changeObservers: new InternalArray, notifier: null, - inactiveObservers: new InternalArray, - performing: { __proto__: null }, - performingCount: 0, }; objectInfoMap.set(object, info); return info; } -var defaultAcceptTypes = { - __proto__: null, - 'new': true, - 'updated': true, - 'deleted': true, - 'prototype': true, - 'reconfigured': true -}; - -function CreateObserver(callback, accept) { - var observer = { - __proto__: null, - callback: callback, - accept: defaultAcceptTypes - }; - - if (IS_UNDEFINED(accept)) - return observer; - - var acceptMap = { __proto__: null }; - for (var i = 0; i < accept.length; i++) - acceptMap[accept[i]] = true; - - observer.accept = acceptMap; - return observer; -} - -function ObserverIsActive(observer, objectInfo) { - if (objectInfo.performingCount === 0) - return true; - - var performing = objectInfo.performing; - for (var type in performing) { - if (performing[type] > 0 && observer.accept[type]) - return false; - } - - return true; -} - -function ObserverIsInactive(observer, objectInfo) { - return !ObserverIsActive(observer, objectInfo); -} - -function RemoveNullElements(from) { - var i = 0; - var j = 0; - for (; i < from.length; i++) { - if (from[i] === null) - continue; - if (j < i) - from[j] = from[i]; - j++; - } - - if (i !== j) - from.length = from.length - (i - j); -} - -function RepartitionObservers(conditionFn, from, to, objectInfo) { - var anyRemoved = false; - for (var i = 0; i < from.length; i++) { - var observer = from[i]; - if (conditionFn(observer, objectInfo)) { - anyRemoved = true; - from[i] = null; - to.push(observer); - } - } - - if (anyRemoved) - RemoveNullElements(from); -} - -function BeginPerformChange(objectInfo, type) { - objectInfo.performing[type] = (objectInfo.performing[type] || 0) + 1; - objectInfo.performingCount++; - RepartitionObservers(ObserverIsInactive, - objectInfo.changeObservers, - objectInfo.inactiveObservers, - objectInfo); -} - -function EndPerformChange(objectInfo, type) { - objectInfo.performing[type]--; - objectInfo.performingCount--; - RepartitionObservers(ObserverIsActive, - objectInfo.inactiveObservers, - objectInfo.changeObservers, - objectInfo); -} - -function ensureObserverRemoved(objectInfo, callback) { - function remove(observerList) { - for (var i = 0; i < observerList.length; i++) { - if (observerList[i].callback === callback) { - observerList.splice(i, 1); - return true; - } - } - return false; - } - - if (!remove(objectInfo.changeObservers)) - remove(objectInfo.inactiveObservers); -} - -function AcceptArgIsValid(arg) { - if (IS_UNDEFINED(arg)) - return true; - - if (!IS_SPEC_OBJECT(arg) || - !IS_NUMBER(arg.length) || - arg.length < 0) - return false; - - var length = arg.length; - for (var i = 0; i < length; i++) { - if (!IS_STRING(arg[i])) - return false; - } - return true; -} - -function ObjectObserve(object, callback, accept) { +function ObjectObserve(object, callback) { if (!IS_SPEC_OBJECT(object)) throw MakeTypeError("observe_non_object", ["observe"]); if (!IS_SPEC_FUNCTION(callback)) throw MakeTypeError("observe_non_function", ["observe"]); if (ObjectIsFrozen(callback)) throw MakeTypeError("observe_callback_frozen"); - if (!AcceptArgIsValid(accept)) - throw MakeTypeError("observe_accept_invalid"); if (!observerInfoMap.has(callback)) { observerInfoMap.set(callback, { @@ -219,13 +90,8 @@ function ObjectObserve(object, callback, accept) { if (IS_UNDEFINED(objectInfo)) objectInfo = CreateObjectInfo(object); %SetIsObserved(object, true); - ensureObserverRemoved(objectInfo, callback); - - var observer = CreateObserver(callback, accept); - if (ObserverIsActive(observer, objectInfo)) - objectInfo.changeObservers.push(observer); - else - objectInfo.inactiveObservers.push(observer); + var changeObservers = objectInfo.changeObservers; + if (changeObservers.indexOf(callback) < 0) changeObservers.push(callback); return object; } @@ -240,11 +106,11 @@ function ObjectUnobserve(object, callback) { if (IS_UNDEFINED(objectInfo)) return object; - ensureObserverRemoved(objectInfo, callback); - - if (objectInfo.changeObservers.length === 0 && - objectInfo.inactiveObservers.length === 0) { - %SetIsObserved(object, false); + var changeObservers = objectInfo.changeObservers; + var index = changeObservers.indexOf(callback); + if (index >= 0) { + changeObservers.splice(index, 1); + if (changeObservers.length === 0) %SetIsObserved(object, false); } return object; @@ -256,12 +122,8 @@ function EnqueueChangeRecord(changeRecord, observers) { for (var i = 0; i < observers.length; i++) { var observer = observers[i]; - if (IS_UNDEFINED(observer.accept[changeRecord.type])) - continue; - - var callback = observer.callback; - var observerInfo = observerInfoMap.get(callback); - observationState.pendingObservers[observerInfo.priority] = callback; + var observerInfo = observerInfoMap.get(observer); + observationState.pendingObservers[observerInfo.priority] = observer; %SetObserverDeliveryPending(); if (IS_NULL(observerInfo.pendingChangeRecords)) { observerInfo.pendingChangeRecords = new InternalArray(changeRecord); @@ -273,9 +135,6 @@ function EnqueueChangeRecord(changeRecord, observers) { function NotifyChange(type, object, name, oldValue) { var objectInfo = objectInfoMap.get(object); - if (objectInfo.changeObservers.length === 0) - return; - var changeRecord = (arguments.length < 4) ? { type: type, object: object, name: name } : { type: type, object: object, name: name, oldValue: oldValue }; @@ -314,36 +173,6 @@ function ObjectNotifierNotify(changeRecord) { EnqueueChangeRecord(newRecord, objectInfo.changeObservers); } -function ObjectNotifierPerformChange(changeType, changeFn, receiver) { - if (!IS_SPEC_OBJECT(this)) - throw MakeTypeError("called_on_non_object", ["performChange"]); - - var target = notifierTargetMap.get(this); - if (IS_UNDEFINED(target)) - throw MakeTypeError("observe_notify_non_notifier"); - if (!IS_STRING(changeType)) - throw MakeTypeError("observe_perform_non_string"); - if (!IS_SPEC_FUNCTION(changeFn)) - throw MakeTypeError("observe_perform_non_function"); - - if (IS_NULL_OR_UNDEFINED(receiver)) { - receiver = %GetDefaultReceiver(changeFn) || receiver; - } else if (!IS_SPEC_OBJECT(receiver) && %IsClassicModeFunction(changeFn)) { - receiver = ToObject(receiver); - } - - var objectInfo = objectInfoMap.get(target); - if (IS_UNDEFINED(objectInfo)) - return; - - BeginPerformChange(objectInfo, changeType); - try { - %_CallFunction(receiver, changeFn); - } finally { - EndPerformChange(objectInfo, changeType); - } -} - function ObjectGetNotifier(object) { if (!IS_SPEC_OBJECT(object)) throw MakeTypeError("observe_non_object", ["getNotifier"]); @@ -406,8 +235,7 @@ function SetupObjectObserve() { "unobserve", ObjectUnobserve )); InstallFunctions(notifierPrototype, DONT_ENUM, $Array( - "notify", ObjectNotifierNotify, - "performChange", ObjectNotifierPerformChange + "notify", ObjectNotifierNotify )); } diff --git a/test/mjsunit/harmony/object-observe.js b/test/mjsunit/harmony/object-observe.js index 8200e90..263154a 100644 --- a/test/mjsunit/harmony/object-observe.js +++ b/test/mjsunit/harmony/object-observe.js @@ -88,11 +88,7 @@ function createObserver() { } var observer = createObserver(); -var observer2 = createObserver(); - assertEquals("function", typeof observer.callback); -assertEquals("function", typeof observer2.callback); - var obj = {}; function frozenFunction() {} @@ -113,15 +109,9 @@ Object.defineProperty(changeRecordWithAccessor, 'name', { assertThrows(function() { Object.observe("non-object", observer.callback); }, TypeError); assertThrows(function() { Object.observe(obj, nonFunction); }, TypeError); assertThrows(function() { Object.observe(obj, frozenFunction); }, TypeError); -assertThrows(function() { Object.observe(obj, function() {}, 1); }, TypeError); -assertThrows(function() { Object.observe(obj, function() {}, [undefined]); }, TypeError); -assertThrows(function() { Object.observe(obj, function() {}, [1]); }, TypeError); -assertThrows(function() { Object.observe(obj, function() {}, ['foo', null]); }, TypeError); -assertEquals(obj, Object.observe(obj, observer.callback, ['foo', 'bar', 'baz'])); -assertEquals(obj, Object.observe(obj, observer.callback, [])); -assertEquals(obj, Object.observe(obj, observer.callback, undefined)); assertEquals(obj, Object.observe(obj, observer.callback)); + // Object.unobserve assertThrows(function() { Object.unobserve(4, observer.callback); }, TypeError); assertThrows(function() { Object.unobserve(obj, nonFunction); }, TypeError); @@ -140,20 +130,6 @@ assertTrue(notifyDesc.writable); assertFalse(notifyDesc.enumerable); assertThrows(function() { notifier.notify({}); }, TypeError); assertThrows(function() { notifier.notify({ type: 4 }); }, TypeError); - -assertThrows(function() { notifier.performChange(1, function(){}); }, TypeError); -assertThrows(function() { notifier.performChange(undefined, function(){}); }, TypeError); -assertThrows(function() { notifier.performChange('foo', undefined); }, TypeError); -assertThrows(function() { notifier.performChange('foo', 'bar'); }, TypeError); -var testSelf = {}; -notifier.performChange('foo', function() { - assertTrue(testSelf === this); -}, testSelf); -var self = this; -notifier.performChange('foo', function() { - assertTrue(self === this); -}); - var notify = notifier.notify; assertThrows(function() { notify.call(undefined, { type: 'a' }); }, TypeError); assertThrows(function() { notify.call(null, { type: 'a' }); }, TypeError); @@ -219,7 +195,7 @@ reset(); Object.observe(obj, observer.callback); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', }); Object.deliverChangeRecords(observer.callback); observer.assertCalled(); @@ -229,7 +205,7 @@ observer.assertCalled(); reset(); Object.unobserve(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', }); Object.deliverChangeRecords(observer.callback); observer.assertNotCalled(); @@ -240,7 +216,7 @@ reset(); Object.unobserve(obj, observer.callback); Object.unobserve(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', }); Object.deliverChangeRecords(observer.callback); observer.assertNotCalled(); @@ -249,11 +225,11 @@ observer.assertNotCalled(); // Re-observation works and only includes changeRecords after of call. reset(); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', }); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', }); records = undefined; Object.deliverChangeRecords(observer.callback); @@ -264,326 +240,42 @@ observer.assertRecordCount(1); reset(); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', val: 1 }); Object.unobserve(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', val: 2 }); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', val: 3 }); Object.unobserve(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', val: 4 }); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'updated', + type: 'foo', val: 5 }); Object.unobserve(obj, observer.callback); Object.deliverChangeRecords(observer.callback); observer.assertCallbackRecords([ - { object: obj, type: 'updated', val: 1 }, - { object: obj, type: 'updated', val: 3 }, - { object: obj, type: 'updated', val: 5 } + { object: obj, type: 'foo', val: 1 }, + { object: obj, type: 'foo', val: 3 }, + { object: obj, type: 'foo', val: 5 } ]); -// Accept -reset(); -Object.observe(obj, observer.callback, []); -Object.getNotifier(obj).notify({ - type: 'new' -}); -Object.getNotifier(obj).notify({ - type: 'updated' -}); -Object.getNotifier(obj).notify({ - type: 'deleted' -}); -Object.getNotifier(obj).notify({ - type: 'reconfigured' -}); -Object.getNotifier(obj).notify({ - type: 'prototype' -}); -Object.deliverChangeRecords(observer.callback); -observer.assertNotCalled(); - -reset(); -Object.observe(obj, observer.callback, ['new', 'deleted', 'prototype']); -Object.getNotifier(obj).notify({ - type: 'new' -}); -Object.getNotifier(obj).notify({ - type: 'updated' -}); -Object.getNotifier(obj).notify({ - type: 'deleted' -}); -Object.getNotifier(obj).notify({ - type: 'deleted' -}); -Object.getNotifier(obj).notify({ - type: 'reconfigured' -}); -Object.getNotifier(obj).notify({ - type: 'prototype' -}); -Object.deliverChangeRecords(observer.callback); -observer.assertCallbackRecords([ - { object: obj, type: 'new' }, - { object: obj, type: 'deleted' }, - { object: obj, type: 'deleted' }, - { object: obj, type: 'prototype' } -]); - -reset(); -Object.observe(obj, observer.callback, ['updated', 'foo']); -Object.getNotifier(obj).notify({ - type: 'new' -}); -Object.getNotifier(obj).notify({ - type: 'updated' -}); -Object.getNotifier(obj).notify({ - type: 'deleted' -}); -Object.getNotifier(obj).notify({ - type: 'foo' -}); -Object.getNotifier(obj).notify({ - type: 'bar' -}); -Object.getNotifier(obj).notify({ - type: 'foo' -}); -Object.deliverChangeRecords(observer.callback); -observer.assertCallbackRecords([ - { object: obj, type: 'updated' }, - { object: obj, type: 'foo' }, - { object: obj, type: 'foo' } -]); - -reset(); -function Thingy(a, b, c) { - this.a = a; - this.b = b; -} - -Thingy.MULTIPLY = 'multiply'; -Thingy.INCREMENT = 'increment'; -Thingy.INCREMENT_AND_MULTIPLY = 'incrementAndMultiply'; - -Thingy.prototype = { - increment: function(amount) { - var notifier = Object.getNotifier(this); - - notifier.performChange(Thingy.INCREMENT, function() { - this.a += amount; - this.b += amount; - }, this); - - notifier.notify({ - object: this, - type: Thingy.INCREMENT, - incremented: amount - }); - }, - - multiply: function(amount) { - var notifier = Object.getNotifier(this); - - notifier.performChange(Thingy.MULTIPLY, function() { - this.a *= amount; - this.b *= amount; - }, this); - - notifier.notify({ - object: this, - type: Thingy.MULTIPLY, - multiplied: amount - }); - }, - - incrementAndMultiply: function(incAmount, multAmount) { - var notifier = Object.getNotifier(this); - - notifier.performChange(Thingy.INCREMENT_AND_MULTIPLY, function() { - this.increment(incAmount); - this.multiply(multAmount); - }, this); - - notifier.notify({ - object: this, - type: Thingy.INCREMENT_AND_MULTIPLY, - incremented: incAmount, - multiplied: multAmount - }); - } -} - -Thingy.observe = function(thingy, callback) { - Object.observe(thingy, callback, [Thingy.INCREMENT, - Thingy.MULTIPLY, - Thingy.INCREMENT_AND_MULTIPLY, - 'updated']); -} - -Thingy.unobserve = function(thingy, callback) { - Object.unobserve(thingy); -} - -var thingy = new Thingy(2, 4); - -Object.observe(thingy, observer.callback); -Thingy.observe(thingy, observer2.callback); -thingy.increment(3); // { a: 5, b: 7 } -thingy.b++; // { a: 5, b: 8 } -thingy.multiply(2); // { a: 10, b: 16 } -thingy.a++; // { a: 11, b: 16 } -thingy.incrementAndMultiply(2, 2); // { a: 26, b: 36 } - -Object.deliverChangeRecords(observer.callback); -Object.deliverChangeRecords(observer2.callback); -observer.assertCallbackRecords([ - { object: thingy, type: 'updated', name: 'a', oldValue: 2 }, - { object: thingy, type: 'updated', name: 'b', oldValue: 4 }, - { object: thingy, type: 'updated', name: 'b', oldValue: 7 }, - { object: thingy, type: 'updated', name: 'a', oldValue: 5 }, - { object: thingy, type: 'updated', name: 'b', oldValue: 8 }, - { object: thingy, type: 'updated', name: 'a', oldValue: 10 }, - { object: thingy, type: 'updated', name: 'a', oldValue: 11 }, - { object: thingy, type: 'updated', name: 'b', oldValue: 16 }, - { object: thingy, type: 'updated', name: 'a', oldValue: 13 }, - { object: thingy, type: 'updated', name: 'b', oldValue: 18 }, -]); -observer2.assertCallbackRecords([ - { object: thingy, type: Thingy.INCREMENT, incremented: 3 }, - { object: thingy, type: 'updated', name: 'b', oldValue: 7 }, - { object: thingy, type: Thingy.MULTIPLY, multiplied: 2 }, - { object: thingy, type: 'updated', name: 'a', oldValue: 10 }, - { - object: thingy, - type: Thingy.INCREMENT_AND_MULTIPLY, - incremented: 2, - multiplied: 2 - } -]); - - -reset(); -function RecursiveThingy() {} - -RecursiveThingy.MULTIPLY_FIRST_N = 'multiplyFirstN'; - -RecursiveThingy.prototype = { - __proto__: Array.prototype, - - multiplyFirstN: function(amount, n) { - if (!n) - return; - var notifier = Object.getNotifier(this); - notifier.performChange(RecursiveThingy.MULTIPLY_FIRST_N, function() { - this[n-1] = this[n-1]*amount; - this.multiplyFirstN(amount, n-1); - }, this); - - notifier.notify({ - object: this, - type: RecursiveThingy.MULTIPLY_FIRST_N, - multiplied: amount, - n: n - }); - }, -} - -RecursiveThingy.observe = function(thingy, callback) { - Object.observe(thingy, callback, [RecursiveThingy.MULTIPLY_FIRST_N]); -} - -RecursiveThingy.unobserve = function(thingy, callback) { - Object.unobserve(thingy); -} - -var thingy = new RecursiveThingy; -thingy.push(1, 2, 3, 4); - -Object.observe(thingy, observer.callback); -RecursiveThingy.observe(thingy, observer2.callback); -thingy.multiplyFirstN(2, 3); // [2, 4, 6, 4] - -Object.deliverChangeRecords(observer.callback); -Object.deliverChangeRecords(observer2.callback); -observer.assertCallbackRecords([ - { object: thingy, type: 'updated', name: '2', oldValue: 3 }, - { object: thingy, type: 'updated', name: '1', oldValue: 2 }, - { object: thingy, type: 'updated', name: '0', oldValue: 1 } -]); -observer2.assertCallbackRecords([ - { object: thingy, type: RecursiveThingy.MULTIPLY_FIRST_N, multiplied: 2, n: 3 } -]); - -reset(); -function DeckSuit() { - this.push('1', '2', '3', '4', '5', '6', '7', '8', '9', '10', 'A', 'Q', 'K'); -} - -DeckSuit.SHUFFLE = 'shuffle'; - -DeckSuit.prototype = { - __proto__: Array.prototype, - - shuffle: function() { - var notifier = Object.getNotifier(this); - notifier.performChange(DeckSuit.SHUFFLE, function() { - this.reverse(); - this.sort(function() { return Math.random()* 2 - 1; }); - var cut = this.splice(0, 6); - Array.prototype.push.apply(this, cut); - this.reverse(); - this.sort(function() { return Math.random()* 2 - 1; }); - var cut = this.splice(0, 6); - Array.prototype.push.apply(this, cut); - this.reverse(); - this.sort(function() { return Math.random()* 2 - 1; }); - }, this); - - notifier.notify({ - object: this, - type: DeckSuit.SHUFFLE - }); - }, -} - -DeckSuit.observe = function(thingy, callback) { - Object.observe(thingy, callback, [DeckSuit.SHUFFLE]); -} - -DeckSuit.unobserve = function(thingy, callback) { - Object.unobserve(thingy); -} - -var deck = new DeckSuit; - -DeckSuit.observe(deck, observer2.callback); -deck.shuffle(); - -Object.deliverChangeRecords(observer2.callback); -observer2.assertCallbackRecords([ - { object: deck, type: DeckSuit.SHUFFLE } -]); // Observing multiple objects; records appear in order. reset(); @@ -593,20 +285,20 @@ Object.observe(obj, observer.callback); Object.observe(obj3, observer.callback); Object.observe(obj2, observer.callback); Object.getNotifier(obj).notify({ - type: 'new', + type: 'foo1', }); Object.getNotifier(obj2).notify({ - type: 'updated', + type: 'foo2', }); Object.getNotifier(obj3).notify({ - type: 'deleted', + type: 'foo3', }); Object.observe(obj3, observer.callback); Object.deliverChangeRecords(observer.callback); observer.assertCallbackRecords([ - { object: obj, type: 'new' }, - { object: obj2, type: 'updated' }, - { object: obj3, type: 'deleted' } + { object: obj, type: 'foo1' }, + { object: obj2, type: 'foo2' }, + { object: obj3, type: 'foo3' } ]); -- 2.7.4