Revision: 25126
Author: [email protected]
Date: Wed Nov 5 07:23:28 2014 UTC
Log: Allow uncaught exception messaging in Object.observe callbacks.
This also naturally handles pausing on uncaught exceptions in
Object.observe callbacks.
[email protected], [email protected], [email protected]
BUG=chromium:335660
LOG=Y
Review URL: https://codereview.chromium.org/692313003
https://code.google.com/p/v8/source/detail?r=25126
Modified:
/branches/bleeding_edge/src/object-observe.js
/branches/bleeding_edge/src/runtime/runtime-observe.cc
/branches/bleeding_edge/src/runtime/runtime.h
/branches/bleeding_edge/test/cctest/test-api.cc
/branches/bleeding_edge/test/cctest/test-debug.cc
/branches/bleeding_edge/test/cctest/test-object-observe.cc
=======================================
--- /branches/bleeding_edge/src/object-observe.js Wed Oct 29 17:27:09 2014
UTC
+++ /branches/bleeding_edge/src/object-observe.js Wed Nov 5 07:23:28 2014
UTC
@@ -567,12 +567,11 @@
if (!IS_NULL(pendingObservers))
delete pendingObservers[priority];
+ // TODO: combine the following runtime calls for perf optimization.
var delivered = [];
%MoveArrayContents(callbackInfo, delivered);
+ %DeliverObservationChangeRecords(callback, delivered);
- try {
- %_CallFunction(UNDEFINED, delivered, callback);
- } catch (ex) {} // TODO(rossberg): perhaps log uncaught exceptions.
return true;
}
=======================================
--- /branches/bleeding_edge/src/runtime/runtime-observe.cc Mon Oct 20
12:07:45 2014 UTC
+++ /branches/bleeding_edge/src/runtime/runtime-observe.cc Wed Nov 5
07:23:28 2014 UTC
@@ -50,6 +50,28 @@
isolate->RunMicrotasks();
return isolate->heap()->undefined_value();
}
+
+
+RUNTIME_FUNCTION(Runtime_DeliverObservationChangeRecords) {
+ HandleScope scope(isolate);
+ DCHECK(args.length() == 2);
+ CONVERT_ARG_HANDLE_CHECKED(JSFunction, callback, 0);
+ CONVERT_ARG_HANDLE_CHECKED(Object, argument, 1);
+ v8::TryCatch catcher;
+ // We should send a message on uncaught exception thrown during
+ // Object.observe delivery while not interrupting further delivery, thus
+ // we make a call inside a verbose TryCatch.
+ catcher.SetVerbose(true);
+ Handle<Object> argv[] = {argument};
+ USE(Execution::Call(isolate, callback,
isolate->factory()->undefined_value(),
+ arraysize(argv), argv));
+ if (isolate->has_pending_exception()) {
+ isolate->ReportPendingMessages();
+ isolate->clear_pending_exception();
+ isolate->set_external_caught_exception(false);
+ }
+ return isolate->heap()->undefined_value();
+}
RUNTIME_FUNCTION(Runtime_GetObservationState) {
=======================================
--- /branches/bleeding_edge/src/runtime/runtime.h Tue Nov 4 10:02:25 2014
UTC
+++ /branches/bleeding_edge/src/runtime/runtime.h Wed Nov 5 07:23:28 2014
UTC
@@ -350,6 +350,7 @@
F(GetObjectContextObjectObserve, 1, 1) \
F(GetObjectContextObjectGetNotifier, 1, 1) \
F(GetObjectContextNotifierPerformChange, 1, 1) \
+ F(DeliverObservationChangeRecords, 2, 1) \
\
/* Harmony typed arrays */ \
F(ArrayBufferInitialize, 2, 1) \
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Tue Nov 4 10:06:44
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-api.cc Wed Nov 5 07:23:28
2014 UTC
@@ -8809,6 +8809,33 @@
CHECK_EQ(1, report_count);
v8::V8::RemoveMessageListeners(ApiUncaughtExceptionTestListener);
}
+
+
+TEST(ApiUncaughtExceptionInObjectObserve) {
+ v8::internal::FLAG_stack_size = 150;
+ report_count = 0;
+ LocalContext env;
+ v8::Isolate* isolate = env->GetIsolate();
+ v8::HandleScope scope(isolate);
+ v8::V8::AddMessageListener(ApiUncaughtExceptionTestListener);
+ CompileRun(
+ "var obj = {};"
+ "var observe_count = 0;"
+ "function observer1() { ++observe_count; };"
+ "function observer2() { ++observe_count; };"
+ "function observer_throws() { throw new Error(); };"
+ "function stack_overflow() { return (function f(x) { f(x+1); })(0);
};"
+ "Object.observe(obj, observer_throws.bind());"
+ "Object.observe(obj, observer1);"
+ "Object.observe(obj, stack_overflow);"
+ "Object.observe(obj, observer2);"
+ "Object.observe(obj, observer_throws.bind());"
+ "obj.foo = 'bar';");
+ CHECK_EQ(3, report_count);
+ ExpectInt32("observe_count", 2);
+ v8::V8::RemoveMessageListeners(ApiUncaughtExceptionTestListener);
+}
+
static const char* script_resource_name = "ExceptionInNativeScript.js";
static void ExceptionInNativeScriptTestListener(v8::Handle<v8::Message>
message,
=======================================
--- /branches/bleeding_edge/test/cctest/test-debug.cc Wed Oct 8 08:56:57
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-debug.cc Wed Nov 5 07:23:28
2014 UTC
@@ -7594,3 +7594,29 @@
CHECK(CompileRun("r")->Equals(v8_str("rejectedrejection")));
CHECK_EQ(1, exception_event_counter);
}
+
+
+TEST(DebugBreakOnExceptionInObserveCallback) {
+ DebugLocalContext env;
+ v8::Isolate* isolate = env->GetIsolate();
+ v8::HandleScope scope(isolate);
+ v8::Debug::SetDebugEventListener(&DebugEventCountException);
+ // Break on uncaught exception
+ ChangeBreakOnException(false, true);
+ exception_event_counter = 0;
+
+ v8::Handle<v8::FunctionTemplate> fun =
+ v8::FunctionTemplate::New(isolate, ThrowCallback);
+ env->Global()->Set(v8_str("fun"), fun->GetFunction());
+
+ CompileRun(
+ "var obj = {};"
+ "var callbackRan = false;"
+ "Object.observe(obj, function() {"
+ " callbackRan = true;"
+ " throw Error('foo');"
+ "});"
+ "obj.prop = 1");
+ CHECK(CompileRun("callbackRan")->BooleanValue());
+ CHECK_EQ(1, exception_event_counter);
+}
=======================================
--- /branches/bleeding_edge/test/cctest/test-object-observe.cc Tue Aug 26
09:19:24 2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-object-observe.cc Wed Nov 5
07:23:28 2014 UTC
@@ -128,6 +128,59 @@
CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
CHECK_EQ(3, CompileRun("ordering[2]")->Int32Value());
}
+
+
+TEST(DeliveryCallbackThrows) {
+ HandleScope scope(CcTest::isolate());
+ LocalContext context(CcTest::isolate());
+ CompileRun(
+ "var obj = {};"
+ "var ordering = [];"
+ "function observer1() { ordering.push(1); };"
+ "function observer2() { ordering.push(2); };"
+ "function observer_throws() {"
+ " ordering.push(0);"
+ " throw new Error();"
+ " ordering.push(-1);"
+ "};"
+ "Object.observe(obj, observer_throws.bind());"
+ "Object.observe(obj, observer1);"
+ "Object.observe(obj, observer_throws.bind());"
+ "Object.observe(obj, observer2);"
+ "Object.observe(obj, observer_throws.bind());"
+ "obj.foo = 'bar';");
+ CHECK_EQ(5, CompileRun("ordering.length")->Int32Value());
+ CHECK_EQ(0, CompileRun("ordering[0]")->Int32Value());
+ CHECK_EQ(1, CompileRun("ordering[1]")->Int32Value());
+ CHECK_EQ(0, CompileRun("ordering[2]")->Int32Value());
+ CHECK_EQ(2, CompileRun("ordering[3]")->Int32Value());
+ CHECK_EQ(0, CompileRun("ordering[4]")->Int32Value());
+}
+
+
+TEST(DeliveryChangesMutationInCallback) {
+ HandleScope scope(CcTest::isolate());
+ LocalContext context(CcTest::isolate());
+ CompileRun(
+ "var obj = {};"
+ "var ordering = [];"
+ "function observer1(records) {"
+ " ordering.push(100 + records.length);"
+ " records.push(11);"
+ " records.push(22);"
+ "};"
+ "function observer2(records) {"
+ " ordering.push(200 + records.length);"
+ " records.push(33);"
+ " records.push(44);"
+ "};"
+ "Object.observe(obj, observer1);"
+ "Object.observe(obj, observer2);"
+ "obj.foo = 'bar';");
+ CHECK_EQ(2, CompileRun("ordering.length")->Int32Value());
+ CHECK_EQ(101, CompileRun("ordering[0]")->Int32Value());
+ CHECK_EQ(201, CompileRun("ordering[1]")->Int32Value());
+}
TEST(DeliveryOrderingReentrant) {
--
--
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.
