From: aandrey@chromium.org Date: Wed, 5 Nov 2014 07:23:28 +0000 (+0000) Subject: Allow uncaught exception messaging in Object.observe callbacks. X-Git-Tag: upstream/4.7.83~5897 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cb0694e7657e570181ab2d9f40b05d16de45ee86;p=platform%2Fupstream%2Fv8.git Allow uncaught exception messaging in Object.observe callbacks. This also naturally handles pausing on uncaught exceptions in Object.observe callbacks. R=adamk@chromium.org, yangguo@chromium.org, yurys@chromium.org BUG=chromium:335660 LOG=Y Review URL: https://codereview.chromium.org/692313003 Cr-Commit-Position: refs/heads/master@{#25126} git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@25126 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/object-observe.js b/src/object-observe.js index 3af2d51..01ce805 100644 --- a/src/object-observe.js +++ b/src/object-observe.js @@ -567,12 +567,11 @@ function CallbackDeliverPending(callback) { 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; } diff --git a/src/runtime/runtime-observe.cc b/src/runtime/runtime-observe.cc index ab7f250..4579136 100644 --- a/src/runtime/runtime-observe.cc +++ b/src/runtime/runtime-observe.cc @@ -52,6 +52,28 @@ RUNTIME_FUNCTION(Runtime_RunMicrotasks) { } +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 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) { SealHandleScope shs(isolate); DCHECK(args.length() == 0); diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 448010a..5d6ccac 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -350,6 +350,7 @@ namespace internal { F(GetObjectContextObjectObserve, 1, 1) \ F(GetObjectContextObjectGetNotifier, 1, 1) \ F(GetObjectContextNotifierPerformChange, 1, 1) \ + F(DeliverObservationChangeRecords, 2, 1) \ \ /* Harmony typed arrays */ \ F(ArrayBufferInitialize, 2, 1) \ diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 068a07e..5fa2a2f 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8810,6 +8810,33 @@ TEST(ApiUncaughtException) { 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 message, v8::Handle) { diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index 575f938..5d38a16 100644 --- a/test/cctest/test-debug.cc +++ b/test/cctest/test-debug.cc @@ -7594,3 +7594,29 @@ TEST(DebugPromiseRejectedByCallback) { 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 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); +} diff --git a/test/cctest/test-object-observe.cc b/test/cctest/test-object-observe.cc index d208a26..8851d88 100644 --- a/test/cctest/test-object-observe.cc +++ b/test/cctest/test-object-observe.cc @@ -130,6 +130,59 @@ TEST(DeliveryOrdering) { } +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) { HandleScope scope(CcTest::isolate()); LocalContext context(CcTest::isolate());