Custom element creation #17381

cbrewster · 2017-06-17T04:46:22Z

This implements the CE-related steps when creating elements. is is now support by document.createElement and is stored on Elements. Only synchronously created autonomous elements are supported as async element creation and customized built-in elements both require custom element upgrade reactions.

Spec: https://dom.spec.whatwg.org/#concept-create-element

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #17191 (github issue number if applicable).

There are tests for these changes OR
These changes do not require tests because _____

This change is

highfive · 2017-06-17T04:46:26Z

Heads up! This PR modifies the following files:

@fitzgen: components/script/dom/customelementregistry.rs, components/script/dom/servoparser/mod.rs, components/script/dom/document.rs, components/script/dom/element.rs, components/script/dom/create.rs and 3 more
@KiChjang: components/script/dom/customelementregistry.rs, components/script/dom/servoparser/mod.rs, components/script/dom/document.rs, components/script/dom/element.rs, components/script/dom/create.rs and 3 more

highfive · 2017-06-17T04:46:28Z

Warning

These commits modify unsafe code. Please review it carefully!

cbrewster · 2017-06-17T04:46:30Z

r? @jdm

cbrewster · 2017-06-17T04:47:17Z

tests/wpt/metadata/custom-elements/reactions/Element.html.ini

@@ -1,5 +1,6 @@
 [Element.html]
  type: testharness
+  expected: CRASH


This test and the other crashing test are caused by #17366

jdm

I found these changes pretty straightforward to review; nice job.

jdm · 2017-06-20T18:45:00Z

components/script/dom/create.rs

+                    // Step 6. Recovering from exception.
+                    let global = document.global();
+
+                    // Step 6.1


jdm · 2017-06-20T18:45:00Z

components/script/dom/create.rs

+                        report_pending_exception(global.get_cx(), true);
+                    }
+
+                    // Step 6.2


jdm · 2017-06-20T18:45:00Z

components/script/dom/create.rs

+                Ok(element) => element,
+                Err(error) => {
+                    // Step 6. Recovering from exception.
+                    let global = document.global();


This should probably be GlobalScope::current() instead.

After that change I got this assertion failure running one of the tests:

▶ TIMEOUT [expected OK] /custom-elements/parser/parser-fallsback-to-unknown-element.html │ │ VMware, Inc. │ Gallium 0.4 on softpipe │ 3.3 (Core Profile) Mesa 12.0.1 │ assertion failed: !global.is_null() (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) }, at /Users/connor/Development/servo/servo/components/script/dom/globalscope.rs:577) │ stack backtrace: │ 0: 0x10b9e6664 - backtrace::backtrace::trace<closure> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.2/src/backtrace/mod.rs:42 │ 1: 0x10b9e15bf - backtrace::capture::Backtrace::new::h08bbd1a61f799546 │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.2/src/capture.rs:64 │ 2: 0x106aac6d6 - servo::main::{{closure}} │ at /Users/connor/Development/servo/servo/ports/servo/main.rs:129 │ 3: 0x10c3393a6 - std::panicking::rust_panic_with_hook::h8b9b25777425677b │ at src/libstd/panicking.rs:550 │ 4: 0x107dc3135 - std::panicking::begin_panic<&str> │ at /Users/travis/build/rust-lang/rust/src/libstd/panicking.rs:511 │ 5: 0x109a28737 - script::dom::globalscope::global_scope_from_global │ at /Users/connor/Development/servo/servo/components/script/dom/globalscope.rs:577 │ 6: 0x109a285cb - script::dom::globalscope::GlobalScope::current::ha8895ac41f143906 │ at /Users/connor/Development/servo/servo/components/script/dom/globalscope.rs:551 │ 7: 0x109174cbf - script::dom::create::create_html_element │ at /Users/connor/Development/servo/servo/components/script/dom/create.rs:133 │ 8: 0x10917b5de - script::dom::create::create_element │ at /Users/connor/Development/servo/servo/components/script/dom/create.rs:324 │ 9: 0x1099fad7d - script::dom::element::Element::create::hfb3b8bc9fcafdc2c │ at /Users/connor/Development/servo/servo/components/script/dom/element.rs:210 │ 10: 0x109a913e2 - _$LT$script..dom..servoparser..Sink$u20$as$u20$markup5ever..interface..tree_builder..TreeSink$GT$::create_element::h6ba8e182ae6c3b46 │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/mod.rs:785 │ 11: 0x10840b9a9 - markup5ever::interface::tree_builder::create_element<script::dom::servoparser::Sink> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/markup5ever-0.3.0/interface/tree_builder.rs:86 │ 12: 0x108d44536 - html5ever::tree_builder::actions::{{impl}}::insert_element<script::dom::bindings::js::JS<script::dom::node::Node>,script::dom::servoparser::Sink> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tree_builder/actions.rs:794 │ 13: 0x108d4ea6f - html5ever::tree_builder::actions::{{impl}}::insert_element_for<script::dom::bindings::js::JS<script::dom::node::Node>,script::dom::servoparser::Sink> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tree_builder/actions.rs:826 │ 14: 0x108d33d93 - html5ever::tree_builder::rules::{{impl}}::step<script::dom::bindings::js::JS<script::dom::node::Node>,script::dom::servoparser::Sink> │ at /Users/connor/Development/servo/servo/target/debug/build/html5ever-65d8b6ff1eb7d5d3/out/rules.rs:780 │ 15: 0x108fdb945 - html5ever::tree_builder::{{impl}}::process_to_completion<script::dom::bindings::js::JS<script::dom::node::Node>,script::dom::servoparser::Sink> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tree_builder/mod.rs:311 │ 16: 0x108d072c1 - html5ever::tree_builder::{{impl}}::process_token<script::dom::bindings::js::JS<script::dom::node::Node>,script::dom::servoparser::Sink> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tree_builder/mod.rs:474 │ 17: 0x1096e0a7f - html5ever::tokenizer::{{impl}}::process_token<html5ever::tree_builder::TreeBuilder<script::dom::bindings::js::JS<script::dom::node::Node>, script::dom::servoparser::Sink>> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tokenizer/mod.rs:233 │ 18: 0x1096e308f - html5ever::tokenizer::{{impl}}::emit_current_tag<html5ever::tree_builder::TreeBuilder<script::dom::bindings::js::JS<script::dom::node::Node>, script::dom::servoparser::Sink>> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tokenizer/mod.rs:426 │ 19: 0x1096f14e4 - html5ever::tokenizer::{{impl}}::step<html5ever::tree_builder::TreeBuilder<script::dom::bindings::js::JS<script::dom::node::Node>, script::dom::servoparser::Sink>> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tokenizer/mod.rs:780 │ 20: 0x1096e7ad0 - html5ever::tokenizer::{{impl}}::run<html5ever::tree_builder::TreeBuilder<script::dom::bindings::js::JS<script::dom::node::Node>, script::dom::servoparser::Sink>> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tokenizer/mod.rs:362 │ 21: 0x1096e7d52 - html5ever::tokenizer::{{impl}}::feed<html5ever::tree_builder::TreeBuilder<script::dom::bindings::js::JS<script::dom::node::Node>, script::dom::servoparser::Sink>> │ at /Users/connor/Development/servo/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/html5ever-0.18.0/src/tokenizer/mod.rs:220 │ 22: 0x10835dbea - script::dom::servoparser::html::{{impl}}::feed │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/html.rs:79 │ 23: 0x109a8dc19 - script::dom::servoparser::{{impl}}::feed │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/mod.rs:505 │ 24: 0x109a8ccf1 - script::dom::servoparser::{{impl}}::do_parse_sync::{{closure}} │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/mod.rs:395 │ 25: 0x109a8d42b - script::dom::servoparser::{{impl}}::tokenize<closure> │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/mod.rs:432 │ 26: 0x109a8cb34 - script::dom::servoparser::{{impl}}::do_parse_sync │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/mod.rs:395 │ 27: 0x109a8c917 - script::dom::servoparser::{{impl}}::parse_sync::{{closure}} │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/mod.rs:381 │ 28: 0x10943b382 - profile_traits::time::profile<(),closure> │ at /Users/connor/Development/servo/servo/components/profile_traits/time.rs:121 │ 29: 0x109a8c87b - script::dom::servoparser::{{impl}}::parse_sync │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/mod.rs:378 │ 30: 0x109a8aad7 - script::dom::servoparser::ServoParser::resume_with_pending_parsing_blocking_script::h2e887e36734e91cc │ at /Users/connor/Development/servo/servo/components/script/dom/servoparser/mod.rs:231 │ 31: 0x10918e9f8 - script::dom::document::{{impl}}::process_pending_parsing_blocking_script │ at /Users/connor/Development/servo/servo/components/script/dom/document.rs:1753 │ 32: 0x10918e6ce - script::dom::document::Document::pending_parsing_blocking_script_loaded::heb8dda2113bcde85 │ at /Users/connor/Development/servo/servo/components/script/dom/document.rs:1740 │ 33: 0x109a4bc87 - _$LT$script..dom..htmlscriptelement..ScriptContext$u20$as$u20$net_traits..FetchResponseListener$GT$::process_response_eof::hfebbb0c034d491d3 │ at /Users/connor/Development/servo/servo/components/script/dom/htmlscriptelement.rs:221 │ 34: 0x1097d35c5 - net_traits::{{impl}}::process<script::dom::htmlscriptelement::ScriptContext> │ at /Users/connor/Development/servo/servo/components/net_traits/lib.rs:254 │ 35: 0x1092693b9 - script::network_listener::{{impl}}::handler<net_traits::FetchResponseMsg,script::dom::htmlscriptelement::ScriptContext> │ at /Users/connor/Development/servo/servo/components/script/network_listener.rs:63 │ 36: 0x10926a428 - script::script_thread::Runnable::main_thread_handler<script::network_listener::ListenerRunnable<net_traits::FetchResponseMsg, script::dom::htmlscriptelement::ScriptContext>> │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:231 │ 37: 0x109ae0460 - script::script_thread::{{impl}}::main_thread_handler<script::network_listener::ListenerRunnable<net_traits::FetchResponseMsg, script::dom::htmlscriptelement::ScriptContext>> │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:219 │ 38: 0x109aece76 - script::script_thread::{{impl}}::handle_msg_from_script │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:1192 │ 39: 0x109ae9307 - script::script_thread::{{impl}}::handle_msgs::{{closure}} │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:997 │ 40: 0x109aeac0e - script::script_thread::{{impl}}::profile_event<closure,core::option::Option<bool>> │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:1106 │ 41: 0x109ae777a - script::script_thread::{{impl}}::handle_msgs │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:990 │ 42: 0x109ae5cc5 - script::script_thread::ScriptThread::start::h1b0b3aba5125085e │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:829 │ 43: 0x10926a4b4 - script::script_thread::{{impl}}::create::{{closure}}::{{closure}} │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:583 │ 44: 0x108d638b3 - profile_traits::mem::{{impl}}::run_with_memory_reporting<closure,fn(profile_traits::mem::ReportsChan) -> script::script_runtime::CommonScriptMsg,script::script_runtime::CommonScriptMsg,std::sync::mpsc::Sender<script::script_thread::MainThreadScriptMsg>> │ at /Users/connor/Development/servo/servo/components/profile_traits/mem.rs:63 │ 45: 0x10836eedb - script::script_thread::{{impl}}::create::{{closure}} │ at /Users/connor/Development/servo/servo/components/script/script_thread.rs:582 │ 46: 0x107dc297a - std::sys_common::backtrace::__rust_begin_short_backtrace<closure,()> │ at /Users/travis/build/rust-lang/rust/src/libstd/sys_common/backtrace.rs:136 │ 47: 0x10865ddd3 - std::thread::{{impl}}::spawn::{{closure}}::{{closure}}<closure,()> │ at /Users/travis/build/rust-lang/rust/src/libstd/thread/mod.rs:364 │ 48: 0x10882444a - std::panic::{{impl}}::call_once<(),closure> │ at /Users/travis/build/rust-lang/rust/src/libstd/panic.rs:296 │ 49: 0x107fe4b59 - std::panicking::try::do_call<std::panic::AssertUnwindSafe<closure>,()> │ at /Users/travis/build/rust-lang/rust/src/libstd/panicking.rs:454 │ 50: 0x10c33a5ba - __rust_maybe_catch_panic │ at src/libpanic_unwind/lib.rs:98 │ 51: 0x107e61b4c - std::panicking::try<(),std::panic::AssertUnwindSafe<closure>> │ at /Users/travis/build/rust-lang/rust/src/libstd/panicking.rs:433 │ 52: 0x108652515 - std::panic::catch_unwind<std::panic::AssertUnwindSafe<closure>,()> │ at /Users/travis/build/rust-lang/rust/src/libstd/panic.rs:361 │ 53: 0x1095676db - std::thread::{{impl}}::spawn::{{closure}}<closure,()> │ at /Users/travis/build/rust-lang/rust/src/libstd/thread/mod.rs:363 │ 54: 0x1096d9813 - alloc::boxed::{{impl}}::call_box<(),closure> │ at /Users/travis/build/rust-lang/rust/src/liballoc/boxed.rs:648 │ 55: 0x10c336555 - std::sys::imp::thread::{{impl}}::new::thread_start │ at src/libstd/sys_common/thread.rs:21 │ 56: 0x119c6993a - _pthread_body │ 57: 0x119c69886 - _pthread_start └ ERROR:servo: assertion failed: !global.is_null()

Since GlobalScope::current now returns an option, you should be able to fallback to the given document's global if it's not available.

jdm · 2017-06-20T18:45:00Z