From 5ea893074c4e5879ad9ec7f8fe10dcdf41f64fcd Mon Sep 17 00:00:00 2001 From: "adamk@chromium.org" Date: Fri, 2 May 2014 21:29:15 +0000 Subject: [PATCH] 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 --- src/object-observe.js | 7 ++++--- src/runtime.cc | 23 +++++++++++++---------- src/runtime.h | 5 +++-- test/cctest/test-object-observe.cc | 9 +++++++-- test/mjsunit/fuzz-natives-part2.js | 5 ----- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/object-observe.js b/src/object-observe.js index 0528a6e..532b0d2 100644 --- 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 --- 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(object->GetCreationContext(), isolate); Handle 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(object_info->GetCreationContext(), isolate); Handle function(context->native_object_notifier_perform_change(), isolate); - Handle call_args[] = { change_type, change_fn }; + Handle call_args[] = { object_info, change_type, change_fn }; Handle 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 --- 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 --- 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 object = CompileRun("obj"); + Handle 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 --- 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 = { -- 2.7.4