Reviewers: Jakob,
Description:
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.
Please review this at https://codereview.chromium.org/264793015/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+26, -17 lines):
M src/object-observe.js
M src/runtime.h
M src/runtime.cc
M test/cctest/test-object-observe.cc
Index: src/object-observe.js
diff --git a/src/object-observe.js b/src/object-observe.js
index
0528a6e7929a97541e2765e473c851beb2ead64f..532b0d25254c28dfee157e61da3e55c175861002
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) {
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
7c5d9c71572bf6161047ca7060826af2e65ec573..1535207245a2c6e776469db3dfed60ea798a4837
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -14996,11 +14996,11 @@
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);
Handle<Context> context(object->GetCreationContext(), isolate);
@@ -15011,12 +15011,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 +15030,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;
}
Index: src/runtime.h
diff --git a/src/runtime.h b/src/runtime.h
index
2ccf80171bf9f1aa0203bbc3d1613ba89d890116..acbe20153dc67238bc722da4c2825ee9cbe2029a
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)\
Index: test/cctest/test-object-observe.cc
diff --git a/test/cctest/test-object-observe.cc
b/test/cctest/test-object-observe.cc
index
3e9dcd7711847a805adf4b9b095251edb7bd6b5f..a7b346fc6fea07da5cf3d0ff78b05d42698eead6
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<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();
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
