Initial JS stub implementation of Object.observe. Adds support for .object/.unobserve/.notify/.deli…

src/object-observe.js

Index: src/object-observe.js
diff --git a/src/object-observe.js b/src/object-observe.js
new file mode 100644
index 0000000000000000000000000000000000000000..d888b4e8ebe8c3018275ac36c451839a7182246d
--- /dev/null
+++ b/src/object-observe.js
@@ -0,0 +1,140 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"use strict";
+
+var observerInfoMap = new $WeakMap;
+var objectInfoMap = new $WeakMap;
+
+$Object.observe = function(object, callback) {
+  if (!IS_SPEC_OBJECT(object))
+    throw MakeTypeError("TODO", ["TODO"])
+  if (!IS_SPEC_FUNCTION(callback))
+    throw MakeTypeError("TODO", ["TODO"])
+  if ($Object.isFrozen(callback))
+    throw MakeTypeError("TODO", ["TODO"])
+
+  var objectInfo = objectInfoMap.get(object);
+  var observerInfo = observerInfoMap.get(callback);
+
+  if (IS_UNDEFINED(objectInfo)) {
+    objectInfo = new $Object();
+    objectInfo.changeObservers = new $Array(1);
+    objectInfo.changeObservers[0] = callback;

[adamk, 2012/10/23 15:35:57]

Even if we need $Array to avoid array literals here, this can be one line:

objectInfo.changeObservers = $Array(callback);

[rafaelw, 2012/10/23 16:18:20]

Done.

+    // TODO: setup objectInfo.notifier
+    objectInfoMap.set(object, objectInfo);
+
+    observerInfo = new $Object();

[adamk, 2012/10/23 15:35:57]

This doesn't seem right, here you want to see if observerInfo IS_UNDEFINED
otherwise you could overwrite an existing observerInfo. And actually, there's no
reason for this whole thing to be inside this if statement, it should happen
before any objectInfo initialization (to let the rest of this block return
early).

[rafaelw, 2012/10/23 16:18:20]

Good catch. For some reason I was thinking these always get initialized together
-- which of course is wrong. Jet lag ;-(.

On 2012/10/23 15:35:57, adamk wrote:

+    observerInfo.pendingChangeRecords = null;
+    // TODO: setup observerInfo.priority.
+    observerInfoMap.set(callback, observerInfo);
+    return;
+  }
+
+  var changeObservers = objectInfo.changeObservers;
+  for (var i = 0; i < changeObservers.length; i++) {

[adamk, 2012/10/23 15:35:57]

Can you use changeObservers.indexOf() instead of this for loop?

[rafaelw, 2012/10/23 16:18:20]

Done.

+    if (changeObservers[i] === callback)
+      return;
+  }
+
+  changeObservers.push(callback);
+}
+
+$Object.unobserve = function(object, callback) {
+  if (!IS_SPEC_OBJECT(object))
+    throw MakeTypeError("TODO", ["TODO"])
+
+  var objectInfo = objectInfoMap.get(object);
+  var observerInfo = observerInfoMap.get(callback);

[adamk, 2012/10/23 15:35:57]

Can move this hash lookup after the undefined check below to avoid unnecessary
lookups.

[rafaelw, 2012/10/23 16:18:20]

Done.

+  if (IS_UNDEFINED(objectInfo))
+    return;
+
+  var changeObservers = objectInfo.changeObservers;
+  for (var i = 0; i < changeObservers.length; i++) {

[adamk, 2012/10/23 15:35:57]

Same question re: indexOf

[rafaelw, 2012/10/23 16:18:20]

Done.

+    if (changeObservers[i] === callback) {
+      changeObservers.splice(i, 1);
+      return;
+    }
+  }
+}
+
+function EnqueueChangeRecord(changeRecord, observers) {
+  for (var i = 0; i < observers.length; i++) {
+    var observer = observers[i];
+    var observerInfo = observerInfoMap.get(observer);
+
+    // TODO: "activate" the observer
+
+    var pendingChangeRecords = observerInfo.pendingChangeRecords;
+    if (IS_NULL(pendingChangeRecords)) {
+      pendingChangeRecords = observerInfo.pendingChangeRecords = new $Array(1);

[adamk, 2012/10/23 15:35:57]

Again, $Array(changeRecord) should suffice if literals are out.

[rafaelw, 2012/10/23 16:18:20]

Done.

+      pendingChangeRecords[0] = changeRecord
+    } else {
+      pendingChangeRecords.push(changeRecord);
+    }
+  }
+}
+
+$Object.notify = function(object, changeRecord) {
+  // TODO: notifier needs to be [[THIS]]
+  var type = changeRecord['type'];

[adamk, 2012/10/23 15:35:57]

Nit: no need to copy this into a local, methinkgs:

if (!IS_STRING(changeRecord['type']))

seems clear enough to me.

[rafaelw, 2012/10/23 16:18:20]

Done.

+  if (!IS_STRING(type))
+    MakeTypeError("TODO", ["TODO"]); // TODO
+
+  var newRecord = new $Object();
+  newRecord['object'] = object; // TODO: Needs to be 'object' retreived from notifier
+  for (var prop in changeRecord) {
+    if (prop === 'object')
+      continue;
+    newRecord[prop] = changeRecord[prop];
+  }
+
+  $Object.freeze(newRecord);
+  var objectInfo = objectInfoMap.get(object);
+  if (IS_UNDEFINED(objectInfo))

[adamk, 2012/10/23 15:35:57]

Probably want to move this before the creation of the record? That's both more
efficient and matches the spec.

[rafaelw, 2012/10/23 16:18:20]

Done. Also updated spec to clarify this behavior

On 2012/10/23 15:35:57, adamk wrote:

+    return;
+
+  EnqueueChangeRecord(newRecord, objectInfo.changeObservers);
+}
+
+$Object.deliverChangeRecords = function(callback) {
+  if (!IS_SPEC_FUNCTION(callback))
+    throw MakeTypeError("TODO", ["TODO"])
+
+  var observerInfo = observerInfoMap.get(callback);
+  if (!observerInfo)

[adamk, 2012/10/23 15:35:57]

Should be if (IS_UNDEFINED()), I think?

[rafaelw, 2012/10/23 16:18:20]

Done.

+    return false;

[adamk, 2012/10/23 15:35:57]

Should be just return, per spec:

Note: The user facing function Object.deliverChangeRecords returns undefined to
prevent detection if anything was delivered or not.

[rafaelw, 2012/10/23 16:18:20]

Done.

+
+  var pendingChangeRecords = observerInfo.pendingChangeRecords;
+  if (IS_NULL(pendingChangeRecords))
+    return false;

[adamk, 2012/10/23 15:35:57]

Ditto, should return undefined.

[rafaelw, 2012/10/23 16:18:20]

Done.

+
+  observerInfo.pendingChangeRecords = null;
+
+  callback(pendingChangeRecords);
+  return true;

[adamk, 2012/10/23 15:35:57]

ditto, should return undefined.

[rafaelw, 2012/10/23 16:18:20]

Done.

+}