Don't leak contexts in Object.observe

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index a62a0f978b50b99c69edf6becf889249b232ab21..7c5d9c71572bf6161047ca7060826af2e65ec573 100644
--- 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);

[Jakob Kummerow, 2014/05/02 19:31:37]

Please be as specific as you can regarding the object type here. I have a
feeling |callback| must be a JSFunction, so use
CONVERT_ARG_HANDLE_CHECKED(JSFunction, callback, 1);

[adamk, 2014/05/02 20:09:13]

Done.

[Toon Verwaest, 2014/05/03 05:28:26]

I think this should become JSReceiver to handle function proxy callbacks.

On 2014/05/02 19:31:37, Jakob wrote:

[adamk, 2014/05/05 19:55:05]

I've filed a but to track the fact that proxies don't work with Object.observe
at the moment: https://code.google.com/p/v8/issues/detail?id=3310

[rossberg, 2014/05/06 10:03:50]

Thanks! While you're at it, can you please also create bugs to track the issues
that

1. observing the global object does not currently allowed, due to various issues
(esp with navigation, e.g. notifier context)
2. observing/unobserving in the middle of a splice event does not work correctly
3. change records in cross context scenarios live in different contexts
depending on how they are created
4. the implementation of observe does not handle acceptList accurately (e.g.
observable side effects in accessing .length multiple times)

Some of these might require modifications/extensions to the spec as well.

+  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) {

[Jakob Kummerow, 2014/05/02 19:31:37]

I don't see this function declared in runtime.h, is that intentional?

[adamk, 2014/05/02 20:09:13]

That is not intentional, and combined with the same-naming, it appears that this
caused us to not be doing the right thing with performChange. I've updated the
performChange leak test to actually test that performChange changes contexts.

+  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,