From 159235b3d00fcb1718becf2840186b5fa8f878ce Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Tue, 9 Sep 2014 15:13:18 -0400 Subject: [PATCH] Add handling for unreported exceptions when invoking callback objects. --- components/script/dom/bindings/callback.rs | 54 +++++++++++++++++++--- components/script/dom/eventdispatcher.rs | 8 ++-- components/script/dom/treewalker.rs | 4 +- components/script/timers.rs | 4 +- components/servo/Cargo.lock | 2 +- ports/cef/Cargo.lock | 2 +- ports/gonk/Cargo.lock | 2 +- 7 files changed, 58 insertions(+), 18 deletions(-) diff --git a/components/script/dom/bindings/callback.rs b/components/script/dom/bindings/callback.rs index c9718b6e026..4cee0c5a2cd 100644 --- a/components/script/dom/bindings/callback.rs +++ b/components/script/dom/bindings/callback.rs @@ -9,19 +9,21 @@ use dom::bindings::global::global_object_for_js_object; use dom::bindings::js::JSRef; use dom::bindings::utils::Reflectable; -use js::jsapi::{JSContext, JSObject, JS_WrapObject, JS_ObjectIsCallable}; -use js::jsapi::JS_GetProperty; +use js::jsapi::{JSContext, JSObject, JS_WrapObject, JS_ObjectIsCallable, JS_GetGlobalObject}; +use js::jsapi::{JS_GetProperty, JS_IsExceptionPending, JS_ReportPendingException}; +use js::jsapi::{JS_GetPendingException}; use js::jsval::{JSVal, UndefinedValue}; +use js::rust::with_compartment; use std::ptr; /// The exception handling used for a call. -#[deriving(Copy)] +#[deriving(Copy, PartialEq)] pub enum ExceptionHandling { /// Report any exception and don't throw it to the caller code. - ReportExceptions, + Report, /// Throw any exception to the caller code. - RethrowExceptions + Rethrow } /// A common base class for representing IDL callback function types. @@ -135,7 +137,7 @@ pub struct CallSetup { /// The `JSContext` used for the call. cx: *mut JSContext, /// The exception handling used for the call. - _handling: ExceptionHandling + handling: ExceptionHandling, } impl CallSetup { @@ -145,9 +147,10 @@ impl CallSetup { let global = global_object_for_js_object(callback.callback()); let global = global.root(); let cx = global.r().get_cx(); + CallSetup { cx: cx, - _handling: handling + handling: handling, } } @@ -156,3 +159,40 @@ impl CallSetup { self.cx } } + +impl Drop for CallSetup { + fn drop(&mut self) { + let mut need_to_deal_with_exception = unsafe { JS_IsExceptionPending(self.cx) } != 0; + if self.handling == ExceptionHandling::Rethrow && need_to_deal_with_exception { + let mut exn = UndefinedValue(); + let got_exn = unsafe { + JS_GetPendingException(self.cx, &mut exn) != 0 + }; + + if got_exn { + //TODO: actually rethrow instead of eagerly reporting. + // Gecko stores a mutable reference to an ErrorResult + // abstraction that can store a JS exception and + // report it at content boundaries. Our return value + // wrappers around Result make that more difficult. + unsafe { + JS_ReportPendingException(self.cx); + } + need_to_deal_with_exception = false; + } + } + + if need_to_deal_with_exception { + // Either we're supposed to report our exceptions, or we're supposed to + // re-throw them but we failed to JS_GetPendingException. Either way, + // just report the pending exception, if any. + + unsafe { + let old_global = JS_GetGlobalObject(self.cx); + with_compartment(self.cx, old_global, || { + JS_ReportPendingException(self.cx) + }); + } + } + } +} diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index d76c3002d8f..3518ff20eb6 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use dom::bindings::callback::ExceptionHandling::ReportExceptions; +use dom::bindings::callback::ExceptionHandling::Report; use dom::bindings::codegen::Bindings::EventBinding::EventMethods; use dom::bindings::codegen::InheritTypes::{EventTargetCast, NodeCast, NodeDerived}; use dom::bindings::js::{JS, JSRef, OptionalRootable, Root}; @@ -48,7 +48,7 @@ pub fn dispatch_event<'a, 'b>(target: JSRef<'a, EventTarget>, event.set_current_target(cur_target.r()); for listener in listeners.iter() { // Explicitly drop any exception on the floor. - let _ = listener.HandleEvent_(cur_target.r(), event, ReportExceptions); + let _ = listener.HandleEvent_(cur_target.r(), event, Report); if event.stop_immediate() { break; @@ -74,7 +74,7 @@ pub fn dispatch_event<'a, 'b>(target: JSRef<'a, EventTarget>, for listeners in opt_listeners.iter() { for listener in listeners.iter() { // Explicitly drop any exception on the floor. - let _ = listener.HandleEvent_(target, event, ReportExceptions); + let _ = listener.HandleEvent_(target, event, Report); if event.stop_immediate() { break; @@ -93,7 +93,7 @@ pub fn dispatch_event<'a, 'b>(target: JSRef<'a, EventTarget>, event.set_current_target(cur_target.r()); for listener in listeners.iter() { // Explicitly drop any exception on the floor. - let _ = listener.HandleEvent_(cur_target.r(), event, ReportExceptions); + let _ = listener.HandleEvent_(cur_target.r(), event, Report); if event.stop_immediate() { break; diff --git a/components/script/dom/treewalker.rs b/components/script/dom/treewalker.rs index 8cae9d08269..b578823acd6 100644 --- a/components/script/dom/treewalker.rs +++ b/components/script/dom/treewalker.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use dom::bindings::callback::ExceptionHandling::RethrowExceptions; +use dom::bindings::callback::ExceptionHandling::Rethrow; use dom::bindings::codegen::Bindings::TreeWalkerBinding; use dom::bindings::codegen::Bindings::TreeWalkerBinding::TreeWalkerMethods; use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods; @@ -325,7 +325,7 @@ impl<'a> PrivateTreeWalkerHelpers<'a> for JSRef<'a, TreeWalker> { match self.filter { Filter::None => Ok(NodeFilterConstants::FILTER_ACCEPT), Filter::Native(f) => Ok((f)(node)), - Filter::JS(callback) => callback.AcceptNode_(self, node, RethrowExceptions) + Filter::JS(callback) => callback.AcceptNode_(self, node, Rethrow) } } diff --git a/components/script/timers.rs b/components/script/timers.rs index c9a5495b30d..9c6ec47ffef 100644 --- a/components/script/timers.rs +++ b/components/script/timers.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use dom::bindings::cell::DOMRefCell; -use dom::bindings::callback::ExceptionHandling::ReportExceptions; +use dom::bindings::callback::ExceptionHandling::Report; use dom::bindings::codegen::Bindings::FunctionBinding::Function; use dom::bindings::js::JSRef; use dom::bindings::utils::Reflectable; @@ -189,7 +189,7 @@ impl TimerManager { // TODO: Must handle rooting of funval and args when movable GC is turned on match data.callback { TimerCallback::FunctionTimerCallback(function) => { - let _ = function.Call_(this, data.args, ReportExceptions); + let _ = function.Call_(this, data.args, Report); } TimerCallback::StringTimerCallback(code_str) => { this.evaluate_js_on_global_with_result(code_str.as_slice()); diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock index 7be6dbf8046..d9769c32e5e 100644 --- a/components/servo/Cargo.lock +++ b/components/servo/Cargo.lock @@ -419,7 +419,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.0" -source = "git+https://github.com/servo/rust-mozjs#e01a846241bd98ac424878e3f8d3e9b989c79242" +source = "git+https://github.com/servo/rust-mozjs#5a69e377d6ab7ea8601f711443994f1c8172c7a8" dependencies = [ "mozjs-sys 0.0.0 (git+https://github.com/servo/mozjs)", ] diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock index 76c0b0e4322..329a79c7563 100644 --- a/ports/cef/Cargo.lock +++ b/ports/cef/Cargo.lock @@ -387,7 +387,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.0" -source = "git+https://github.com/servo/rust-mozjs#e01a846241bd98ac424878e3f8d3e9b989c79242" +source = "git+https://github.com/servo/rust-mozjs#5a69e377d6ab7ea8601f711443994f1c8172c7a8" dependencies = [ "mozjs-sys 0.0.0 (git+https://github.com/servo/mozjs)", ] diff --git a/ports/gonk/Cargo.lock b/ports/gonk/Cargo.lock index 8e47e8f0b3d..35a85b26cbd 100644 --- a/ports/gonk/Cargo.lock +++ b/ports/gonk/Cargo.lock @@ -341,7 +341,7 @@ dependencies = [ [[package]] name = "js" version = "0.1.0" -source = "git+https://github.com/servo/rust-mozjs#e01a846241bd98ac424878e3f8d3e9b989c79242" +source = "git+https://github.com/servo/rust-mozjs#5a69e377d6ab7ea8601f711443994f1c8172c7a8" dependencies = [ "mozjs-sys 0.0.0 (git+https://github.com/servo/mozjs)", ]