Auto merge of #14963 - jdm:script_current_line, r=asajeffrey

Report meaningful line numbers for inline script errors

Rebased from #14661.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12744 and partially #9604
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14963)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-01-11 18:11:52 -08:00 committed by GitHub
commit eb72c0ec7b
11 changed files with 61 additions and 22 deletions

View file

@ -145,10 +145,25 @@ impl fmt::Debug for Element {
#[derive(PartialEq, HeapSizeOf)] #[derive(PartialEq, HeapSizeOf)]
pub enum ElementCreator { pub enum ElementCreator {
ParserCreated, ParserCreated(u64),
ScriptCreated, ScriptCreated,
} }
impl ElementCreator {
pub fn is_parser_created(&self) -> bool {
match *self {
ElementCreator::ParserCreated(_) => true,
ElementCreator::ScriptCreated => false,
}
}
pub fn return_line_number(&self) -> u64 {
match *self {
ElementCreator::ParserCreated(l) => l,
ElementCreator::ScriptCreated => 1,
}
}
}
pub enum AdjacentPosition { pub enum AdjacentPosition {
BeforeBegin, BeforeBegin,
AfterEnd, AfterEnd,

View file

@ -339,13 +339,13 @@ impl GlobalScope {
/// Evaluate JS code on this global scope. /// Evaluate JS code on this global scope.
pub fn evaluate_js_on_global_with_result( pub fn evaluate_js_on_global_with_result(
&self, code: &str, rval: MutableHandleValue) { &self, code: &str, rval: MutableHandleValue) {
self.evaluate_script_on_global_with_result(code, "", rval) self.evaluate_script_on_global_with_result(code, "", rval, 1)
} }
/// Evaluate a JS script on this global scope. /// Evaluate a JS script on this global scope.
#[allow(unsafe_code)] #[allow(unsafe_code)]
pub fn evaluate_script_on_global_with_result( pub fn evaluate_script_on_global_with_result(
&self, code: &str, filename: &str, rval: MutableHandleValue) { &self, code: &str, filename: &str, rval: MutableHandleValue, line_number: u32) {
let metadata = time::TimerMetadata { let metadata = time::TimerMetadata {
url: if filename.is_empty() { url: if filename.is_empty() {
self.get_url().as_str().into() self.get_url().as_str().into()
@ -367,7 +367,7 @@ impl GlobalScope {
let _ac = JSAutoCompartment::new(cx, globalhandle.get()); let _ac = JSAutoCompartment::new(cx, globalhandle.get());
let _aes = AutoEntryScript::new(self); let _aes = AutoEntryScript::new(self);
let options = CompileOptionsWrapper::new(cx, filename.as_ptr(), 1); let options = CompileOptionsWrapper::new(cx, filename.as_ptr(), line_number);
unsafe { unsafe {
if !Evaluate2(cx, options.ptr, code.as_ptr(), if !Evaluate2(cx, options.ptr, code.as_ptr(),
code.len() as libc::size_t, code.len() as libc::size_t,

View file

@ -59,7 +59,7 @@ impl HTMLLinkElement {
HTMLLinkElement { HTMLLinkElement {
htmlelement: HTMLElement::new_inherited(local_name, prefix, document), htmlelement: HTMLElement::new_inherited(local_name, prefix, document),
rel_list: Default::default(), rel_list: Default::default(),
parser_inserted: Cell::new(creator == ElementCreator::ParserCreated), parser_inserted: Cell::new(creator.is_parser_created()),
stylesheet: DOMRefCell::new(None), stylesheet: DOMRefCell::new(None),
cssom_stylesheet: MutNullableJS::new(None), cssom_stylesheet: MutNullableJS::new(None),
pending_loads: Cell::new(0), pending_loads: Cell::new(0),

View file

@ -60,6 +60,9 @@ pub struct HTMLScriptElement {
/// Document of the parser that created this element /// Document of the parser that created this element
parser_document: JS<Document>, parser_document: JS<Document>,
/// Track line line_number
line_number: u64,
} }
impl HTMLScriptElement { impl HTMLScriptElement {
@ -69,10 +72,11 @@ impl HTMLScriptElement {
htmlelement: htmlelement:
HTMLElement::new_inherited(local_name, prefix, document), HTMLElement::new_inherited(local_name, prefix, document),
already_started: Cell::new(false), already_started: Cell::new(false),
parser_inserted: Cell::new(creator == ElementCreator::ParserCreated), parser_inserted: Cell::new(creator.is_parser_created()),
non_blocking: Cell::new(creator != ElementCreator::ParserCreated), non_blocking: Cell::new(!creator.is_parser_created()),
ready_to_be_parser_executed: Cell::new(false), ready_to_be_parser_executed: Cell::new(false),
parser_document: JS::from_ref(document), parser_document: JS::from_ref(document),
line_number: creator.return_line_number(),
} }
} }
@ -508,7 +512,7 @@ impl HTMLScriptElement {
let window = window_from_node(self); let window = window_from_node(self);
rooted!(in(window.get_cx()) let mut rval = UndefinedValue()); rooted!(in(window.get_cx()) let mut rval = UndefinedValue());
window.upcast::<GlobalScope>().evaluate_script_on_global_with_result( window.upcast::<GlobalScope>().evaluate_script_on_global_with_result(
&script.text, script.url.as_str(), rval.handle_mut()); &script.text, script.url.as_str(), rval.handle_mut(), self.line_number as u32);
// Step 6. // Step 6.
document.set_current_script(old_script.r()); document.set_current_script(old_script.r());

View file

@ -50,8 +50,8 @@ impl HTMLStyleElement {
htmlelement: HTMLElement::new_inherited(local_name, prefix, document), htmlelement: HTMLElement::new_inherited(local_name, prefix, document),
stylesheet: DOMRefCell::new(None), stylesheet: DOMRefCell::new(None),
cssom_stylesheet: MutNullableJS::new(None), cssom_stylesheet: MutNullableJS::new(None),
parser_inserted: Cell::new(creator == ElementCreator::ParserCreated), parser_inserted: Cell::new(creator.is_parser_created()),
in_stack_of_open_elements: Cell::new(creator == ElementCreator::ParserCreated), in_stack_of_open_elements: Cell::new(creator.is_parser_created()),
pending_loads: Cell::new(0), pending_loads: Cell::new(0),
any_failed_load: Cell::new(false), any_failed_load: Cell::new(false),
} }

View file

@ -52,6 +52,7 @@ impl Tokenizer {
let sink = Sink { let sink = Sink {
base_url: url, base_url: url,
document: JS::from_ref(document), document: JS::from_ref(document),
current_line: 1,
}; };
let options = TreeBuilderOpts { let options = TreeBuilderOpts {
@ -122,6 +123,7 @@ unsafe impl JSTraceable for HtmlTokenizer<TreeBuilder<JS<Node>, Sink>> {
struct Sink { struct Sink {
base_url: ServoUrl, base_url: ServoUrl,
document: JS<Document>, document: JS<Document>,
current_line: u64,
} }
impl TreeSink for Sink { impl TreeSink for Sink {
@ -156,7 +158,7 @@ impl TreeSink for Sink {
fn create_element(&mut self, name: QualName, attrs: Vec<Attribute>) fn create_element(&mut self, name: QualName, attrs: Vec<Attribute>)
-> JS<Node> { -> JS<Node> {
let elem = Element::create(name, None, &*self.document, let elem = Element::create(name, None, &*self.document,
ElementCreator::ParserCreated); ElementCreator::ParserCreated(self.current_line));
for attr in attrs { for attr in attrs {
elem.set_attribute_from_parser(attr.name, DOMString::from(String::from(attr.value)), None); elem.set_attribute_from_parser(attr.name, DOMString::from(String::from(attr.value)), None);
@ -234,6 +236,10 @@ impl TreeSink for Sink {
} }
} }
fn set_current_line(&mut self, line_number: u64) {
self.current_line = line_number;
}
fn pop(&mut self, node: JS<Node>) { fn pop(&mut self, node: JS<Node>) {
let node = Root::from_ref(&*node); let node = Root::from_ref(&*node);
vtable_for(&node).pop(); vtable_for(&node).pop();

View file

@ -134,8 +134,9 @@ impl TreeSink for Sink {
ns: name.namespace_url, ns: name.namespace_url,
local: name.local, local: name.local,
}; };
//TODO: Add ability to track lines to API of xml5ever
let elem = Element::create(name, prefix, &*self.document, let elem = Element::create(name, prefix, &*self.document,
ElementCreator::ParserCreated); ElementCreator::ParserCreated(1));
for attr in attrs { for attr in attrs {
let name = QualName { let name = QualName {

View file

@ -1,5 +0,0 @@
[window-onerror-parse-error.html]
type: testharness
[correct line number passed to window.onerror]
expected: FAIL

View file

@ -1,5 +0,0 @@
[window-onerror-runtime-error.html]
type: testharness
[correct line number passed to window.onerror]
expected: FAIL

View file

@ -15200,6 +15200,12 @@
"url": "/_mozilla/mozilla/trace_null.html" "url": "/_mozilla/mozilla/trace_null.html"
} }
], ],
"mozilla/track_line.html": [
{
"path": "mozilla/track_line.html",
"url": "/_mozilla/mozilla/track_line.html"
}
],
"mozilla/union.html": [ "mozilla/union.html": [
{ {
"path": "mozilla/union.html", "path": "mozilla/union.html",

View file

@ -0,0 +1,17 @@
<!doctype html>
<meta charset="utf-8">
<title>Test that errors from inline scripts report meaningful line numbers</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
setup({allow_uncaught_exception:true});
var t = async_test("error event has proper line number");
window.addEventListener('error', t.step_func(function(e) {
assert_true(e instanceof ErrorEvent);
assert_equals(e.lineno, 16);
t.done();
}), true);
</script>
<script>
this_is_a_js_error
</script>