From 0ed681905c93d99a9cf26c92517ceafa2e64e3b4 Mon Sep 17 00:00:00 2001 From: "adamk@chromium.org" Date: Wed, 15 May 2013 22:09:40 +0000 Subject: [PATCH] Re-land Notifier.prototype.performChange + tests Fixes the debug check failure on sorting an object with an array __proto__. Original Issue: https://codereview.chromium.org/14779011/ TBR=adamk@chromium.org Review URL: https://codereview.chromium.org/14977015 Patch from Rafael Weinstein . git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14698 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/messages.js | 3 + src/object-observe.js | 194 ++++++++++++++++-- src/objects.cc | 2 +- test/mjsunit/harmony/object-observe.js | 348 +++++++++++++++++++++++++++++++-- 4 files changed, 515 insertions(+), 32 deletions(-) diff --git a/src/messages.js b/src/messages.js index b9bce1e..296965d 100644 --- a/src/messages.js +++ b/src/messages.js @@ -98,7 +98,10 @@ 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 77409b9..c4c6c14 100644 --- a/src/object-observe.js +++ b/src/object-observe.js @@ -66,18 +66,147 @@ function CreateObjectInfo(object) { var info = { changeObservers: new InternalArray, notifier: null, + inactiveObservers: new InternalArray, + performing: { __proto__: null }, + performingCount: 0, }; objectInfoMap.set(object, info); return info; } -function ObjectObserve(object, callback) { +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) { 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, { @@ -90,8 +219,13 @@ function ObjectObserve(object, callback) { if (IS_UNDEFINED(objectInfo)) objectInfo = CreateObjectInfo(object); %SetIsObserved(object, true); - var changeObservers = objectInfo.changeObservers; - if (changeObservers.indexOf(callback) < 0) changeObservers.push(callback); + ensureObserverRemoved(objectInfo, callback); + + var observer = CreateObserver(callback, accept); + if (ObserverIsActive(observer, objectInfo)) + objectInfo.changeObservers.push(observer); + else + objectInfo.inactiveObservers.push(observer); return object; } @@ -106,11 +240,11 @@ function ObjectUnobserve(object, callback) { if (IS_UNDEFINED(objectInfo)) return object; - var changeObservers = objectInfo.changeObservers; - var index = changeObservers.indexOf(callback); - if (index >= 0) { - changeObservers.splice(index, 1); - if (changeObservers.length === 0) %SetIsObserved(object, false); + ensureObserverRemoved(objectInfo, callback); + + if (objectInfo.changeObservers.length === 0 && + objectInfo.inactiveObservers.length === 0) { + %SetIsObserved(object, false); } return object; @@ -122,8 +256,12 @@ function EnqueueChangeRecord(changeRecord, observers) { for (var i = 0; i < observers.length; i++) { var observer = observers[i]; - var observerInfo = observerInfoMap.get(observer); - observationState.pendingObservers[observerInfo.priority] = observer; + if (IS_UNDEFINED(observer.accept[changeRecord.type])) + continue; + + var callback = observer.callback; + var observerInfo = observerInfoMap.get(callback); + observationState.pendingObservers[observerInfo.priority] = callback; %SetObserverDeliveryPending(); if (IS_NULL(observerInfo.pendingChangeRecords)) { observerInfo.pendingChangeRecords = new InternalArray(changeRecord); @@ -135,6 +273,9 @@ 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 }; @@ -173,6 +314,36 @@ 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"]); @@ -235,7 +406,8 @@ function SetupObjectObserve() { "unobserve", ObjectUnobserve )); InstallFunctions(notifierPrototype, DONT_ENUM, $Array( - "notify", ObjectNotifierNotify + "notify", ObjectNotifierNotify, + "performChange", ObjectNotifierPerformChange )); } diff --git a/src/objects.cc b/src/objects.cc index b2ad3d4..845fd45 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -13560,7 +13560,7 @@ MaybeObject* JSObject::PrepareElementsForSort(uint32_t limit) { // Ordering is irrelevant, since we are going to sort anyway. SeededNumberDictionary* dict = element_dictionary(); if (IsJSArray() || dict->requires_slow_elements() || - dict->max_number_key() >= limit) { + dict->max_number_key() >= limit || map()->is_observed()) { return PrepareSlowElementsForSort(limit); } // Convert to fast elements. diff --git a/test/mjsunit/harmony/object-observe.js b/test/mjsunit/harmony/object-observe.js index 263154a..8200e90 100644 --- a/test/mjsunit/harmony/object-observe.js +++ b/test/mjsunit/harmony/object-observe.js @@ -88,7 +88,11 @@ function createObserver() { } var observer = createObserver(); +var observer2 = createObserver(); + assertEquals("function", typeof observer.callback); +assertEquals("function", typeof observer2.callback); + var obj = {}; function frozenFunction() {} @@ -109,9 +113,15 @@ 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); @@ -130,6 +140,20 @@ 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); @@ -195,7 +219,7 @@ reset(); Object.observe(obj, observer.callback); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', }); Object.deliverChangeRecords(observer.callback); observer.assertCalled(); @@ -205,7 +229,7 @@ observer.assertCalled(); reset(); Object.unobserve(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', }); Object.deliverChangeRecords(observer.callback); observer.assertNotCalled(); @@ -216,7 +240,7 @@ reset(); Object.unobserve(obj, observer.callback); Object.unobserve(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', }); Object.deliverChangeRecords(observer.callback); observer.assertNotCalled(); @@ -225,11 +249,11 @@ observer.assertNotCalled(); // Re-observation works and only includes changeRecords after of call. reset(); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', }); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', }); records = undefined; Object.deliverChangeRecords(observer.callback); @@ -240,42 +264,326 @@ observer.assertRecordCount(1); reset(); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', val: 1 }); Object.unobserve(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', val: 2 }); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', val: 3 }); Object.unobserve(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', val: 4 }); Object.observe(obj, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo', + type: 'updated', val: 5 }); Object.unobserve(obj, observer.callback); Object.deliverChangeRecords(observer.callback); observer.assertCallbackRecords([ - { object: obj, type: 'foo', val: 1 }, - { object: obj, type: 'foo', val: 3 }, - { object: obj, type: 'foo', val: 5 } + { object: obj, type: 'updated', val: 1 }, + { object: obj, type: 'updated', val: 3 }, + { object: obj, type: 'updated', 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(); @@ -285,20 +593,20 @@ Object.observe(obj, observer.callback); Object.observe(obj3, observer.callback); Object.observe(obj2, observer.callback); Object.getNotifier(obj).notify({ - type: 'foo1', + type: 'new', }); Object.getNotifier(obj2).notify({ - type: 'foo2', + type: 'updated', }); Object.getNotifier(obj3).notify({ - type: 'foo3', + type: 'deleted', }); Object.observe(obj3, observer.callback); Object.deliverChangeRecords(observer.callback); observer.assertCallbackRecords([ - { object: obj, type: 'foo1' }, - { object: obj2, type: 'foo2' }, - { object: obj3, type: 'foo3' } + { object: obj, type: 'new' }, + { object: obj2, type: 'updated' }, + { object: obj3, type: 'deleted' } ]); -- 2.7.4