Don't leak contexts in Object.observe

The Object.observe API may construct internal structures as a result of API calls. These structures can persist as long as an object that was once observed persists. This patch ensures that these structures are created in the correct context so as to avoid leaking contexts

R=verwaest@chromium.org, dcarney
BUG=

Review URL: https://codereview.chromium.org/263833007

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@21126 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index 0d700de..b158715 100644 (file)
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -1556,6 +1556,12 @@ void Genesis::InstallNativeFunctions() {
                  observers_begin_perform_splice);
   INSTALL_NATIVE(JSFunction, "EndPerformSplice",
                  observers_end_perform_splice);
+  INSTALL_NATIVE(JSFunction, "NativeObjectObserve",
+                 native_object_observe);
+  INSTALL_NATIVE(JSFunction, "NativeObjectGetNotifier",
+                 native_object_get_notifier);
+  INSTALL_NATIVE(JSFunction, "NativeObjectNotifierPerformChange",
+                 native_object_notifier_perform_change);
 }
 
 

diff --git a/src/contexts.h b/src/contexts.h
index 43b2329..f1aa380 100644 (file)
--- a/src/contexts.h
+++ b/src/contexts.h
@@ -174,6 +174,12 @@ enum BindingFlags {
     observers_begin_perform_splice) \
   V(OBSERVERS_END_SPLICE_INDEX, JSFunction, \
     observers_end_perform_splice) \
+  V(NATIVE_OBJECT_OBSERVE_INDEX, JSFunction, \
+    native_object_observe) \
+  V(NATIVE_OBJECT_GET_NOTIFIER_INDEX, JSFunction, \
+    native_object_get_notifier) \
+  V(NATIVE_OBJECT_NOTIFIER_PERFORM_CHANGE, JSFunction, \
+    native_object_notifier_perform_change) \
   V(SLOPPY_GENERATOR_FUNCTION_MAP_INDEX, Map, sloppy_generator_function_map) \
   V(STRICT_GENERATOR_FUNCTION_MAP_INDEX, Map, strict_generator_function_map) \
   V(GENERATOR_OBJECT_PROTOTYPE_MAP_INDEX, Map, \
@@ -342,6 +348,9 @@ class Context: public FixedArray {
     OBSERVERS_ENQUEUE_SPLICE_INDEX,
     OBSERVERS_BEGIN_SPLICE_INDEX,
     OBSERVERS_END_SPLICE_INDEX,
+    NATIVE_OBJECT_OBSERVE_INDEX,
+    NATIVE_OBJECT_GET_NOTIFIER_INDEX,
+    NATIVE_OBJECT_NOTIFIER_PERFORM_CHANGE,
     SLOPPY_GENERATOR_FUNCTION_MAP_INDEX,
     STRICT_GENERATOR_FUNCTION_MAP_INDEX,
     GENERATOR_OBJECT_PROTOTYPE_MAP_INDEX,

diff --git a/src/object-observe.js b/src/object-observe.js
index 9d66254..0528a6e 100644 (file)
--- a/src/object-observe.js
+++ b/src/object-observe.js
@@ -56,6 +56,7 @@ function GetWeakMapWrapper() {
   };
 
   MapWrapper.prototype = {
+    __proto__: null,
     get: function(key) {
       return %WeakCollectionGet(this.map_, key);
     },
@@ -364,6 +365,10 @@ function ObjectObserve(object, callback, acceptList) {
   if (!AcceptArgIsValid(acceptList))
     throw MakeTypeError("observe_accept_invalid");
 
+  return %NativeObjectObserve(object, callback, acceptList);
+}
+
+function NativeObjectObserve(object, callback, acceptList) {
   var objectInfo = ObjectInfoGetOrCreate(object);
   ObjectInfoAddObserver(objectInfo, callback, acceptList);
   return object;
@@ -527,7 +532,6 @@ function ObjectNotifierPerformChange(changeType, changeFn) {
     throw MakeTypeError("called_on_non_object", ["performChange"]);
 
   var objectInfo = ObjectInfoGetFromNotifier(this);
-
   if (IS_UNDEFINED(objectInfo))
     throw MakeTypeError("observe_notify_non_notifier");
   if (!IS_STRING(changeType))
@@ -535,6 +539,10 @@ function ObjectNotifierPerformChange(changeType, changeFn) {
   if (!IS_SPEC_FUNCTION(changeFn))
     throw MakeTypeError("observe_perform_non_function");
 
+  return %NativeObjectNotifierPerformChange(objectInfo, changeType, changeFn)
+}
+
+function NativeObjectNotifierPerformChange(objectInfo, changeType, changeFn) {
   ObjectInfoAddPerformingType(objectInfo, changeType);
 
   var changeRecord;
@@ -558,6 +566,10 @@ function ObjectGetNotifier(object) {
 
   if (!%ObjectWasCreatedInCurrentOrigin(object)) return null;
 
+  return %NativeObjectGetNotifier(object);
+}
+
+function NativeObjectGetNotifier(object) {
   var objectInfo = ObjectInfoGetOrCreate(object);
   return ObjectInfoGetNotifier(objectInfo);
 }

diff --git a/src/runtime.cc b/src/runtime.cc
index a62a0f9..7c5d9c7 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -14996,6 +14996,65 @@ RUNTIME_FUNCTION(Runtime_ObjectWasCreatedInCurrentOrigin) {
 }
 
 
+RUNTIME_FUNCTION(Runtime_NativeObjectObserve) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 3);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
+  CONVERT_ARG_HANDLE_CHECKED(Object, callback, 1);
+  CONVERT_ARG_HANDLE_CHECKED(Object, accept, 2);
+
+  Handle<Context> context(object->GetCreationContext(), isolate);
+  Handle<JSFunction> function(context->native_object_observe(), isolate);
+  Handle<Object> call_args[] = { object, callback, accept };
+  Handle<Object> result;
+
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result,
+      Execution::Call(isolate, function,
+          handle(context->object_function(), isolate), 3, call_args, true));
+  return *result;
+}
+
+
+RUNTIME_FUNCTION(Runtime_NativeObjectGetNotifier) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 1);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
+
+  Handle<Context> context(object->GetCreationContext(), isolate);
+  Handle<JSFunction> function(context->native_object_get_notifier(), isolate);
+  Handle<Object> call_args[] = { object };
+  Handle<Object> result;
+
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result,
+      Execution::Call(isolate, function,
+          handle(context->object_function(), isolate), 1, call_args, true));
+  return *result;
+}
+
+
+RUNTIME_FUNCTION(Runtime_NativeObjectNotifierPerformChange) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 3);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, object_info, 0);
+  CONVERT_ARG_HANDLE_CHECKED(Object, change_type, 1);
+  CONVERT_ARG_HANDLE_CHECKED(Object, change_fn, 2);
+
+  Handle<Context> context(object_info->GetCreationContext(), isolate);
+  Handle<JSFunction> function(context->native_object_notifier_perform_change(),
+      isolate);
+  Handle<Object> call_args[] = { change_type, change_fn };
+  Handle<Object> result;
+
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result,
+      Execution::Call(isolate, function, isolate->factory()->undefined_value(),
+                      2, call_args, true));
+  return *result;
+}
+
+
 static Object* ArrayConstructorCommon(Isolate* isolate,
                                            Handle<JSFunction> constructor,
                                            Handle<AllocationSite> site,

diff --git a/src/runtime.h b/src/runtime.h
index 1b1bd8d..2ccf801 100644 (file)
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -313,6 +313,8 @@ namespace internal {
   F(ObservationWeakMapCreate, 0, 1) \
   F(ObserverObjectAndRecordHaveSameOrigin, 3, 1) \
   F(ObjectWasCreatedInCurrentOrigin, 1, 1) \
+  F(NativeObjectObserve, 3, 1) \
+  F(NativeObjectGetNotifier, 1, 1) \
   \
   /* Harmony typed arrays */ \
   F(ArrayBufferInitialize, 2, 1)\

diff --git a/test/cctest/test-object-observe.cc b/test/cctest/test-object-observe.cc
index a096828..3e9dcd7 100644 (file)
--- a/test/cctest/test-object-observe.cc
+++ b/test/cctest/test-object-observe.cc
@@ -613,3 +613,95 @@ TEST(GetNotifierFromSameOrigin) {
     CHECK(CompileRun("Object.getNotifier(obj)")->IsObject());
   }
 }
+
+
+static int GetGlobalObjectsCount() {
+  CcTest::heap()->EnsureHeapIsIterable();
+  int count = 0;
+  i::HeapIterator it(CcTest::heap());
+  for (i::HeapObject* object = it.next(); object != NULL; object = it.next())
+    if (object->IsJSGlobalObject()) count++;
+  return count;
+}
+
+
+static void CheckSurvivingGlobalObjectsCount(int expected) {
+  // We need to collect all garbage twice to be sure that everything
+  // has been collected.  This is because inline caches are cleared in
+  // the first garbage collection but some of the maps have already
+  // been marked at that point.  Therefore some of the maps are not
+  // collected until the second garbage collection.
+  CcTest::heap()->CollectAllGarbage(i::Heap::kNoGCFlags);
+  CcTest::heap()->CollectAllGarbage(i::Heap::kMakeHeapIterableMask);
+  int count = GetGlobalObjectsCount();
+#ifdef DEBUG
+  if (count != expected) CcTest::heap()->TracePathToGlobal();
+#endif
+  CHECK_EQ(expected, count);
+}
+
+
+TEST(DontLeakContextOnObserve) {
+  HandleScope scope(CcTest::isolate());
+  Handle<Value> foo = String::NewFromUtf8(CcTest::isolate(), "foo");
+  LocalContext context(CcTest::isolate());
+  context->SetSecurityToken(foo);
+  CompileRun("var obj = {};");
+  Handle<Value> object = CompileRun("obj");
+  {
+    HandleScope scope(CcTest::isolate());
+    LocalContext context2(CcTest::isolate());
+    context2->SetSecurityToken(foo);
+    context2->Global()->Set(String::NewFromUtf8(CcTest::isolate(), "obj"),
+                            object);
+    CompileRun("function observer() {};"
+               "Object.observe(obj, observer, ['foo', 'bar', 'baz']);"
+               "Object.unobserve(obj, observer);");
+  }
+
+  v8::V8::ContextDisposedNotification();
+  CheckSurvivingGlobalObjectsCount(1);
+}
+
+
+TEST(DontLeakContextOnGetNotifier) {
+  HandleScope scope(CcTest::isolate());
+  Handle<Value> foo = String::NewFromUtf8(CcTest::isolate(), "foo");
+  LocalContext context(CcTest::isolate());
+  context->SetSecurityToken(foo);
+  CompileRun("var obj = {};");
+  Handle<Value> object = CompileRun("obj");
+  {
+    HandleScope scope(CcTest::isolate());
+    LocalContext context2(CcTest::isolate());
+    context2->SetSecurityToken(foo);
+    context2->Global()->Set(String::NewFromUtf8(CcTest::isolate(), "obj"),
+                            object);
+    CompileRun("Object.getNotifier(obj);");
+  }
+
+  v8::V8::ContextDisposedNotification();
+  CheckSurvivingGlobalObjectsCount(1);
+}
+
+
+TEST(DontLeakContextOnNotifierPerformChange) {
+  HandleScope scope(CcTest::isolate());
+  Handle<Value> foo = String::NewFromUtf8(CcTest::isolate(), "foo");
+  LocalContext context(CcTest::isolate());
+  context->SetSecurityToken(foo);
+  CompileRun("var obj = {};");
+  Handle<Value> object = CompileRun("obj");
+  {
+    HandleScope scope(CcTest::isolate());
+    LocalContext context2(CcTest::isolate());
+    context2->SetSecurityToken(foo);
+    context2->Global()->Set(String::NewFromUtf8(CcTest::isolate(), "obj"),
+                            object);
+    CompileRun("var n = Object.getNotifier(obj);"
+               "n.performChange('foo', function() {});");
+  }
+
+  v8::V8::ContextDisposedNotification();
+  CheckSurvivingGlobalObjectsCount(1);
+}