[v8-dev] Object.observe: avoid accessing acceptList properties more than once (issue 270763003)

adamk Thu, 08 May 2014 17:56:34 -0700

Reviewers: rossberg, arv,

Description:
Object.observe: avoid accessing acceptList properties more than once


BUG=v8:3315

Please review this at https://codereview.chromium.org/270763003/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+51, -22 lines):
  M src/object-observe.js
  M src/runtime.cc
  A test/mjsunit/regress/regress-3315.js


Index: src/object-observe.js
diff --git a/src/object-observe.js b/src/object-observe.js

index532b0d25254c28dfee157e61da3e55c175861002..f4b1f539634a58f583265695ed59e92c72f26f51100644

--- a/src/object-observe.js
+++ b/src/object-observe.js
@@ -127,9 +127,9 @@ function TypeMapRemoveType(typeMap, type) {
   typeMap[type]--;
 }

-function TypeMapCreateFromList(typeList) {
+function TypeMapCreateFromList(typeList, length) {
   var typeMap = TypeMapCreate();
-  for (var i = 0; i < typeList.length; i++) {
+  for (var i = 0; i < length; i++) {
     TypeMapAddType(typeMap, typeList[i], true);
   }
   return typeMap;
@@ -151,14 +151,17 @@ function TypeMapIsDisjointFrom(typeMap1, typeMap2) {
   return true;
 }

-var defaultAcceptTypes = TypeMapCreateFromList([
-  'add',
-  'update',
-  'delete',
-  'setPrototype',
-  'reconfigure',
-  'preventExtensions'
-]);
+var defaultAcceptTypes = (function() {
+  var defaultTypes = [
+    'add',
+    'update',
+    'delete',
+    'setPrototype',
+    'reconfigure',
+    'preventExtensions'
+  ];
+  return TypeMapCreateFromList(defaultTypes, defaultTypes.length);
+})();

 // An Observer is a registration to observe an object by a callback with
 // a given set of accept types. If the set of accept types is the default
@@ -170,7 +173,7 @@ function ObserverCreate(callback, acceptList) {
     return callback;
   var observer = nullProtoObject();
   observer.callback = callback;
-  observer.accept = TypeMapCreateFromList(acceptList);
+  observer.accept = acceptList;
   return observer;
 }

@@ -306,16 +309,18 @@ function ObjectInfoGetPerformingTypes(objectInfo) {
   return objectInfo.performingCount > 0 ? objectInfo.performing : null;
 }

-function AcceptArgIsValid(arg) {
+function ConvertAcceptListToTypeMap(arg) {
+  // We use undefined as a sentinel for the default accept list.
   if (IS_UNDEFINED(arg))
-    return true;
+    return arg;

-  if (!IS_SPEC_OBJECT(arg) ||
-      !IS_NUMBER(arg.length) ||
-      arg.length < 0)
-    return false;
+  if (!IS_SPEC_OBJECT(arg))
+    throw MakeTypeError("observe_accept_invalid");

-  return true;
+  var len = ToInteger(arg.length);
+  if (len < 0) len = 0;
+
+  return TypeMapCreateFromList(arg, len);
 }

// CallbackInfo's optimized state is just a number which represents itsglobal

@@ -362,15 +367,14 @@ function ObjectObserve(object, callback, acceptList) {
     throw MakeTypeError("observe_non_function", ["observe"]);
   if (ObjectIsFrozen(callback))
     throw MakeTypeError("observe_callback_frozen");
-  if (!AcceptArgIsValid(acceptList))
-    throw MakeTypeError("observe_accept_invalid");

   return %ObjectObserveInObjectContext(object, callback, acceptList);
 }

 function NativeObjectObserve(object, callback, acceptList) {
   var objectInfo = ObjectInfoGetOrCreate(object);
-  ObjectInfoAddObserver(objectInfo, callback, acceptList);
+  var typeList = ConvertAcceptListToTypeMap(acceptList);
+  ObjectInfoAddObserver(objectInfo, callback, typeList);
   return object;
 }

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc

index743b34b534639a31917e1097ee27558605696b25..4bf63476228feb89c7bead7f9c7bc68fc1c886de100644

--- a/src/runtime.cc
+++ b/src/runtime.cc

@@ -14999,7 +14999,6 @@RUNTIME_FUNCTION(Runtime_ObjectObserveInObjectContext) {

   CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, callback, 1);
   CONVERT_ARG_HANDLE_CHECKED(Object, accept, 2);
-  RUNTIME_ASSERT(accept->IsUndefined() || accept->IsJSObject());

   Handle<Context> context(object->GetCreationContext(), isolate);
   Handle<JSFunction> function(context->native_object_observe(), isolate);
Index: test/mjsunit/regress/regress-3315.js

diff --git a/test/mjsunit/regress/regress-3315.jsb/test/mjsunit/regress/regress-3315.js

new file mode 100644

index0000000000000000000000000000000000000000..a1105e28488971dca2a930db92f22b8cde26866d

--- /dev/null
+++ b/test/mjsunit/regress/regress-3315.js
@@ -0,0 +1,26 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+var indexZeroCallCount = 0;
+var indexOneCallCount = 0;
+var lengthCallCount = 0;
+var acceptList = {
+  get 0() {
+    indexZeroCallCount++;
+    return 'foo';
+  },
+  get 1() {
+    indexOneCallCount++;
+    return 'bar';
+  },
+  get length() {
+    lengthCallCount++;
+    return 1;
+  }
+};
+
+Object.observe({}, function(){}, acceptList);
+assertEquals(1, lengthCallCount);
+assertEquals(1, indexZeroCallCount);
+assertEquals(0, indexOneCallCount);


--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev

---You received this message because you are subscribed to the Google Groups "v8-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[v8-dev] Object.observe: avoid accessing acceptList properties more than once (issue 270763003)

Reply via email to