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

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);
 }
 
 

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,

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);
 }

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,

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)\

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);
+}