Fix ObjectNotifierPerformChange leak after r21126

Due to overlapping names of natives and runtime functions, the wrong
context was used for Notifier.prototype.performChange. The leak test
has been augmented to properly cover the leaky case, and the test
now passes.

Also tightened up type checks in runtime.cc and removed Object.observe
functions from knownIssues in fuzz-natives-part2.js.

R=jkummerow@chromium.org

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

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

diff --git a/src/object-observe.js b/src/object-observe.js
index 0528a6e..532b0d2 100644 (file)
--- a/src/object-observe.js
+++ b/src/object-observe.js
@@ -365,7 +365,7 @@ function ObjectObserve(object, callback, acceptList) {
   if (!AcceptArgIsValid(acceptList))
     throw MakeTypeError("observe_accept_invalid");
 
-  return %NativeObjectObserve(object, callback, acceptList);
+  return %ObjectObserveInObjectContext(object, callback, acceptList);
 }
 
 function NativeObjectObserve(object, callback, acceptList) {
@@ -539,7 +539,8 @@ function ObjectNotifierPerformChange(changeType, changeFn) {
   if (!IS_SPEC_FUNCTION(changeFn))
     throw MakeTypeError("observe_perform_non_function");
 
-  return %NativeObjectNotifierPerformChange(objectInfo, changeType, changeFn)
+  return %ObjectNotifierPerformChangeInObjectContext(
+      objectInfo, changeType, changeFn);
 }
 
 function NativeObjectNotifierPerformChange(objectInfo, changeType, changeFn) {
@@ -566,7 +567,7 @@ function ObjectGetNotifier(object) {
 
   if (!%ObjectWasCreatedInCurrentOrigin(object)) return null;
 
-  return %NativeObjectGetNotifier(object);
+  return %ObjectGetNotifierInObjectContext(object);
 }
 
 function NativeObjectGetNotifier(object) {

diff --git a/src/runtime.cc b/src/runtime.cc
index 7c5d9c7..07c7c79 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -14996,12 +14996,13 @@ RUNTIME_FUNCTION(Runtime_ObjectWasCreatedInCurrentOrigin) {
 }
 
 
-RUNTIME_FUNCTION(Runtime_NativeObjectObserve) {
+RUNTIME_FUNCTION(Runtime_ObjectObserveInObjectContext) {
   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(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);
@@ -15011,12 +15012,13 @@ RUNTIME_FUNCTION(Runtime_NativeObjectObserve) {
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, result,
       Execution::Call(isolate, function,
-          handle(context->object_function(), isolate), 3, call_args, true));
+          handle(context->object_function(), isolate),
+          ARRAY_SIZE(call_args), call_args, true));
   return *result;
 }
 
 
-RUNTIME_FUNCTION(Runtime_NativeObjectGetNotifier) {
+RUNTIME_FUNCTION(Runtime_ObjectGetNotifierInObjectContext) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 1);
   CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 0);
@@ -15029,28 +15031,29 @@ RUNTIME_FUNCTION(Runtime_NativeObjectGetNotifier) {
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, result,
       Execution::Call(isolate, function,
-          handle(context->object_function(), isolate), 1, call_args, true));
+          handle(context->object_function(), isolate),
+          ARRAY_SIZE(call_args), call_args, true));
   return *result;
 }
 
 
-RUNTIME_FUNCTION(Runtime_NativeObjectNotifierPerformChange) {
+RUNTIME_FUNCTION(Runtime_ObjectNotifierPerformChangeInObjectContext) {
   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);
+  CONVERT_ARG_HANDLE_CHECKED(String, change_type, 1);
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, 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> call_args[] = { object_info, 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));
+                      ARRAY_SIZE(call_args), call_args, true));
   return *result;
 }
 

diff --git a/src/runtime.h b/src/runtime.h
index 2ccf801..acbe201 100644 (file)
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -313,8 +313,9 @@ namespace internal {
   F(ObservationWeakMapCreate, 0, 1) \
   F(ObserverObjectAndRecordHaveSameOrigin, 3, 1) \
   F(ObjectWasCreatedInCurrentOrigin, 1, 1) \
-  F(NativeObjectObserve, 3, 1) \
-  F(NativeObjectGetNotifier, 1, 1) \
+  F(ObjectObserveInObjectContext, 3, 1) \
+  F(ObjectGetNotifierInObjectContext, 1, 1) \
+  F(ObjectNotifierPerformChangeInObjectContext, 3, 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 3e9dcd7..a7b346f 100644 (file)
--- a/test/cctest/test-object-observe.cc
+++ b/test/cctest/test-object-observe.cc
@@ -692,14 +692,19 @@ TEST(DontLeakContextOnNotifierPerformChange) {
   context->SetSecurityToken(foo);
   CompileRun("var obj = {};");
   Handle<Value> object = CompileRun("obj");
+  Handle<Value> notifier = CompileRun("Object.getNotifier(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() {});");
+    context2->Global()->Set(String::NewFromUtf8(CcTest::isolate(), "notifier"),
+                            notifier);
+    CompileRun("var obj2 = {};"
+               "var notifier2 = Object.getNotifier(obj2);"
+               "notifier2.performChange.call("
+                   "notifier, 'foo', function(){})");
   }
 
   v8::V8::ContextDisposedNotification();

diff --git a/test/mjsunit/fuzz-natives-part2.js b/test/mjsunit/fuzz-natives-part2.js
index 024791c..103e132 100644 (file)
--- a/test/mjsunit/fuzz-natives-part2.js
+++ b/test/mjsunit/fuzz-natives-part2.js
@@ -188,11 +188,6 @@ var knownProblems = {
 
   // Only applicable to DataViews.
   "_DataViewInitialize": true,
-
-  // Internal calls from the Object.observe API
-  "NativeObjectObserve": true,
-  "NativeObjectNotifierPerformChange": true,
-  "NativeObjectGetNotifier": true
 };
 
 var currentlyUncallable = {