diff options
21 files changed, 491 insertions, 328 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 50e52b29e24..f5768fee4c4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,9 @@ follow this format, even those from core contributors. If you're looking for easy bugs, have a look at the [E-Easy issue tag](https://github.com/servo/servo/labels/E-easy) on GitHub. +See [`HACKING_QUICKSTART.md`](HACKING_QUICKSTART.md) for more information +on how to start working on Servo. + ## Pull Request Checklist - Branch from the master branch and, if needed, rebase to the current master diff --git a/HACKING_QUICKSTART.md b/HACKING_QUICKSTART.md new file mode 100644 index 00000000000..dac6d37aa42 --- /dev/null +++ b/HACKING_QUICKSTART.md @@ -0,0 +1,284 @@ +# Hacking Servo - Quickstart + +This guide covers the basics things one needs to know to start hacking Servo. +It doesn't cover how Servo works (see the [documentation](#documentation) section for that), +but describe how to setup your environment to compile, run and debug Servo. + +## Building Servo + +Building Servo is quite easy. Install the prerequisites described in the [README](./README.md) file. Then type: + +```shell +./mach build -d +``` + +*Note: on Mac, you might run into a SSL issue while compiling. You'll find a solution to this problem [here](https://github.com/sfackler/rust-openssl/issues/255).* + +The `-d` option means "debug build". You can also build with the `-r` option which means "release build". Buidling with `-d` will allow you to use a debugger (lldb). A `-r` build is more performant. Release builds are slower to build. + +You can use and build a release build and a debug build in parallel. + +## Running Servo + +The servo binary is located in `target/debug/servo` (or `target/release/servo`). You can directly run this binary. But we recommend using `./mach` instead: + +``` shell +./mach run -d -- http://github.com +``` + +… is equivalent to: + +``` shell +./target/debug/servo http://github.com +``` + +If you build with `-d`, run with `-d`. If you build with `-r`, run with `-r`. + +## ./mach + +`mach` is a python utility that does plenty of things to make our life easier (build, run, run tests, udpate dependencies… see `./mach --help`). Beside editing files and git commands, everything else is done via `mach`. + +``` shell +./mach run -d [mach options] -- [servo options] +``` + +The `--` separates `mach` options from `servo` options. This is not required, but we recommend it. `mach` and `servo` have some options with the same name (`--help`, `--debug`), the `--` makes it clear where options apply. + +## Mach and Servo options + +This guide only covers the most important options. Be sure to look at all the available mach commands and the servo options: + +``` shell +./mach --help # mach options +./mach run -- --help # servo options +``` + +## Some basic Rust + +Even if you have never seen any Rust code, it's not too hard to read Servo's code. But there are some basics things one must know: + +- [Match](https://doc.rust-lang.org/book/match.html) and [Patterns](https://doc.rust-lang.org/book/patterns.html) +- [Options](http://rustbyexample.com/std/option.html) +- [Expression](http://rustbyexample.com/expression.html) +- [Traits](http://rustbyexample.com/trait.html) +- That doesn't sound important, but be sure to understand how `println!()` works, especially the [formatting traits](https://doc.rust-lang.org/std/fmt/#formatting-traits) + +This won't be enough to do any serious work at first, but if you want to navigate the code and fix basic bugs, that should do it. It's a good starting point, and as you dig into Servo source code, you'll learn more. + +For a more exhaustive documentation: + +- [doc.rust-lang.org](https://doc.rust-lang.org) +- [rust by example](http://rustbyexample.com) + +## Cargo and Crates + +A Rust library is called a crate. Servo uses plenty of crates. These crates are dependencies. They are listed in files called `Cargo.toml`. Servo is split in components and ports (see `components` and `ports` directories). Each has its own dependencies, each has its own `Cargo.toml` file. + +`Cargo.toml` files list the dependencies. You can edit this file. + +For example, `components/net_traits/Cargo.toml` includes: + +``` + [dependencies.stb_image] + git = "https://github.com/servo/rust-stb-image" +``` + +But because `rust-stb-image` API might change over time, it's not safe to compile against the `HEAD` of `rust-stb-image`. A `Cargo.lock` file is a snapshot of a `Cargo.toml` file which includes a reference to an exact revision, ensuring everybody is always compiling with the same configuration: + +``` +[[package]] +name = "stb_image" +source = "git+https://github.com/servo/rust-stb-image#f4c5380cd586bfe16326e05e2518aa044397894b" +``` + +This file should not be edited by hand. In a normal Rust project, to update the git revision, you would use `cargo update -p stb_image`, but in Servo, use `./mach cargo-update -p stb_image`. + +See [Cargo's documentation about Cargo.toml and Cargo.lock files](http://doc.crates.io/guide.html#cargotoml-vs-cargolock). + +## Working on a Crate + +As explained above, Servo depends on a lot of libraries, which makes it very modular. While working on a bug in Servo, you'll often end up in one of its dependencies. You will then want to compile your own version of the dependency (and maybe compiling against the HEAD of the library will fix the issue!). + +For example, I'm trying to bring some cocoa events to Servo. The Servo window on Desktop is constructed with a library named [Glutin](https://github.com/tomaka/glutin). Glutin itself depends on a cocoa library named [cocoa-rs](http://github.com/servo/cocoa-rs). When building Servo, magically, all these dependencies are downloaded and built for you. But because I want to work on this cocoa event feature, I want Servo to use my own version of *glutin* and *cocoa-rs*. + +This is how my projects are laid out: + +``` +~/my-projects/servo/ +~/my-projects/cocoa-rs/ +~/my-projects/glutin/ +``` + +These are all git repositories. + +To make it so that servo uses `~/my-projects/cocoa-rs/` and `~/my-projects/glutin/` , create a `~/my-projects/.cargo/config` file: + +``` shell +$ cat ~/my-projects/.cargo/config +paths = ['glutin', 'cocoa-rs'] +``` + +This will tell any cargo project to not use the online version of the dependency, but your local clone. + +For more details about overriding dependencies, see [Cargo's documentation](http://doc.crates.io/guide.html#overriding-dependencies). + +## Debugging + +### Logging: + +Before starting the debugger right away, you might want get some information about what's happening, how, and when. Luckily, Servo comes with plenty of logs that will help us. Type these 2 commands: + +``` shell +./mach run -d -- --help +./mach run -d -- --debug help +``` + +A typical command might be: + +``` shell +./mach run -d -- -i -y 1 -t 1 --debug dump-layer-tree /tmp/a.html +``` + +… to avoid using too many threads and make things easier to understand. + +On OSX, you can add some Cocoa-specific debug options: + +``` shell +./mach run -d -- /tmp/a.html -- -NSShowAllViews YES +``` + +You can also enable some extra logging (warning: verbose!): + +``` +RUST_LOG="debug" ./mach run -d -- /tmp/a.html +``` + +Using `RUST_LOG="debug"` is usually the very first thing you might want to do if you have no idea what to look for. Because this is very verbose, you can combine these with `ts` (`moreutils` package (apt-get, brew)) to add timestamps and `tee` to save the logs (while keeping them in the console): + +``` +RUST_LOG="debug" ./mach run -d -- -i -y 1 -t 1 /tmp/a.html 2>&1 | ts -s "%.S: " | tee /tmp/log.txt +``` + +You can filter by crate or module, for example `RUST_LOG="layout::inline=debug" ./mach run …`. This is documented at https://rust-lang.github.io/log/env_logger/. + +Use `RUST_BACKTRACE=1` to dump the backtrace when Servo panics. + +### println!() + +You will want to add your own logs. Luckily, many structures [implement the `fmt::Debug` trait](https://doc.rust-lang.org/std/fmt/#fmt::display-vs-fmt::debug), so adding: + +``` rust +println!("foobar: {:?}", foobar) +``` + +usually just works. If it doesn't, maybe foobar's properties implement the right trait. + +### Debugger + +To run the debugger: + +``` shell +./mach -d --debug -- -y 1 -t 1 /tmp/a.html +``` + +This will start `lldb` on Mac, and `gdb` on Linux. + +From here, use: + +``` shell +(lldb) b a_servo_function # add a breakpoint +(lldb) run # run until breakpoint is reached +(lldb) bt # see backtrace +(lldb) frame n # choose the stack frame from the number in the bt +(lldb) thread list +(lldb) next / step / … +(lldb) print varname +``` + +And to search for a function full name/regex: + +```shell +(lldb) image lookup -r -n <name> #lldb +(gdb) info functions <name> #gdb +``` + +See this [lldb turorial](http://lldb.llvm.org/tutorial.html) and this [gdb turorial](http://www.unknownroad.com/rtfm/gdbtut/gdbtoc.html). + +To inspect variables and you are new with lldb, we recommend using the `gui` mode (use left/right to expand variables): + +``` +(lldb) gui +┌──<Variables>───────────────────────────────────────────────────────────────────────────┐ +│ ◆─(&mut gfx::paint_task::PaintTask<Box<CompositorProxy>>) self = 0x000070000163a5b0 │ +│ ├─◆─(msg::constellation_msg::PipelineId) id │ +│ ├─◆─(url::Url) _url │ +│ │ ├─◆─(collections::string::String) scheme │ +│ │ │ └─◆─(collections::vec::Vec<u8>) vec │ +│ │ ├─◆─(url::SchemeData) scheme_data │ +│ │ ├─◆─(core::option::Option<collections::string::String>) query │ +│ │ └─◆─(core::option::Option<collections::string::String>) fragment │ +│ ├─◆─(std::sync::mpsc::Receiver<gfx::paint_task::LayoutToPaintMsg>) layout_to_paint_port│ +│ ├─◆─(std::sync::mpsc::Receiver<gfx::paint_task::ChromeToPaintMsg>) chrome_to_paint_port│ +└────────────────────────────────────────────────────────────────────────────────────────┘ +``` + +If lldb crashes on certain lines involving the `profile()` function, it's not just you. Comment out the profiling code, and only keep the inner function, and that should do it. + +## Tests + +This is boring. But your PR won't get accepted without a test. Tests are located in the `tests` directory. You'll see that there are a lot of files in there, so finding the proper location for your test is not always obvious. + +First, look at the "Testing" section in `./mach --help` to understand the different test categories. You'll also find some `update-*` commands. It's used to update the list of expected results. + +To run a test: + +``` +./mach test-wpt tests/wpt/yourtest +``` + +### Updating a test: + +In some cases extensive tests for the feature you're working on may already exist under tests/wpt: + +- Make a release build +- run `./mach -r test-wpt --log-raw=/path/to/some/logfile` +- run [`update-wpt` on it](https://github.com/servo/servo/blob/master/tests/wpt/README.md#updating-test-expectations) + +This may create a new commit with changes to expectation ini files. If there are lots of changes, +it's likely that your feature had tests in wpt already. + +Include this commit in your pull request. + +### Add a new test: + +If you need to create a new test file, it should be located in `tests/wpt/mozilla/tests` or in `tests/wpt/web-platform-tests` if it's something that doesn't depend on servo-only features. You'll then need to update the list of tests and the list of expected results: + +``` +./mach test-wpt --manifest-update +``` + +## Documentation: + +- Servo's directory structure: [ORGANIZATION.md](./ORGANIZATION.md) +- http://doc.servo.org/servo/index.html +- https://github.com/servo/servo/wiki +- http://rustbyexample.com +- https://doc.rust-lang.org +- Cargo & crates: http://doc.crates.io/guide.html +- mach help: `./mach --help` +- servo options: `./mach run -- --help` +- servo debug options: `./mach run -- --debug help` + +## Ask questions + +### IRC + +IRC channels (irc.mozilla.org): + +- #servo +- #rust +- #cargo + +### Mailing list + +https://lists.mozilla.org/listinfo/dev-servo diff --git a/README.md b/README.md index 115b41a8e78..32a3d3dd86b 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,8 @@ Servo is a prototype web browser engine written in the 64bit OS X, 64bit Linux, Android, and Gonk (Firefox OS). Servo welcomes contribution from everyone. See -[`CONTRIBUTING.md`](CONTRIBUTING.md) for help getting started. +[`CONTRIBUTING.md`](CONTRIBUTING.md) and [`HACKING_QUICKSTART.md`](HACKING_QUICKSTART.md) +for help getting started. ## Prerequisites diff --git a/components/layout/construct.rs b/components/layout/construct.rs index ff1ff42a1d2..7d3a0b748b0 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -327,7 +327,8 @@ impl<'a> FlowConstructor<'a> { } Some(NodeTypeId::Element(ElementTypeId::HTMLElement( HTMLElementTypeId::HTMLCanvasElement))) => { - SpecificFragmentInfo::Canvas(box CanvasFragmentInfo::new(node)) + let data = node.canvas_data().unwrap(); + SpecificFragmentInfo::Canvas(box CanvasFragmentInfo::new(node, data)) } _ => { // This includes pseudo-elements. @@ -1659,7 +1660,9 @@ impl<'ln> ObjectElement<'ln> for ThreadSafeLayoutNode<'ln> { } } -pub trait FlowConstructionUtils { +// This must not be public because only the layout constructor can call these +// methods. +trait FlowConstructionUtils { /// Adds a new flow as a child of this flow. Removes the flow from the given leaf set if /// it's present. fn add_new_child(&mut self, new_child: FlowRef); @@ -1675,8 +1678,6 @@ pub trait FlowConstructionUtils { impl FlowConstructionUtils for FlowRef { /// Adds a new flow as a child of this flow. Fails if this flow is marked as a leaf. - /// - /// This must not be public because only the layout constructor can do this. fn add_new_child(&mut self, mut new_child: FlowRef) { { let kid_base = flow::mut_base(flow_ref::deref_mut(&mut new_child)); @@ -1694,8 +1695,6 @@ impl FlowConstructionUtils for FlowRef { /// /// All flows must be finished at some point, or they will not have their intrinsic inline-sizes /// properly computed. (This is not, however, a memory safety problem.) - /// - /// This must not be public because only the layout constructor can do this. fn finish(&mut self) { if !opts::get().bubble_inline_sizes_separately { flow_ref::deref_mut(self).bubble_inline_sizes() diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index 406b439ad29..1794123bf39 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -1157,11 +1157,11 @@ impl FragmentDisplayListBuilding for Fragment { let height = canvas_fragment_info.replaced_image_fragment_info .computed_block_size.map_or(0, |h| h.to_px() as usize); if width > 0 && height > 0 { - let (sender, receiver) = ipc::channel::<IpcSharedMemory>().unwrap(); let layer_id = self.layer_id(); let canvas_data = match canvas_fragment_info.ipc_renderer { Some(ref ipc_renderer) => { let ipc_renderer = ipc_renderer.lock().unwrap(); + let (sender, receiver) = ipc::channel().unwrap(); ipc_renderer.send(CanvasMsg::FromLayout( FromLayoutMsg::SendPixelContents(sender))).unwrap(); let data = receiver.recv().unwrap(); diff --git a/components/layout/flow.rs b/components/layout/flow.rs index 96d4b4b159a..5be4533dc6a 100644 --- a/components/layout/flow.rs +++ b/components/layout/flow.rs @@ -937,11 +937,6 @@ pub struct BaseFlow { pub flags: FlowFlags, } -#[allow(unsafe_code)] -unsafe impl Send for BaseFlow {} -#[allow(unsafe_code)] -unsafe impl Sync for BaseFlow {} - impl fmt::Debug for BaseFlow { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index 0364e4a2300..ce0047e3ce9 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -28,6 +28,7 @@ use msg::constellation_msg::PipelineId; use net_traits::image::base::Image; use net_traits::image_cache_task::UsePlaceholder; use rustc_serialize::{Encodable, Encoder}; +use script::dom::htmlcanvaselement::HTMLCanvasData; use std::borrow::ToOwned; use std::cmp::{max, min}; use std::collections::LinkedList; @@ -308,13 +309,13 @@ pub struct CanvasFragmentInfo { } impl CanvasFragmentInfo { - pub fn new(node: &ThreadSafeLayoutNode) -> CanvasFragmentInfo { + pub fn new(node: &ThreadSafeLayoutNode, data: HTMLCanvasData) -> CanvasFragmentInfo { CanvasFragmentInfo { replaced_image_fragment_info: ReplacedImageFragmentInfo::new(node, - Some(Au::from_px(node.canvas_width() as i32)), - Some(Au::from_px(node.canvas_height() as i32))), - renderer_id: node.canvas_renderer_id(), - ipc_renderer: node.canvas_ipc_renderer() + Some(Au::from_px(data.width as i32)), + Some(Au::from_px(data.height as i32))), + renderer_id: data.renderer_id, + ipc_renderer: data.ipc_renderer .map(|renderer| Arc::new(Mutex::new(renderer))), } } diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 7d2e31ce217..9c3a3e7ea4c 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -877,25 +877,24 @@ impl LayoutTask { traversal); } - fn process_node_geometry_request<'a>(&'a self, - requested_node: TrustedNodeAddress, - layout_root: &mut FlowRef, - rw_data: &mut RWGuard<'a>) { + fn process_node_geometry_request(&self, + requested_node: TrustedNodeAddress, + layout_root: &mut FlowRef) + -> Rect<i32> { let requested_node: OpaqueNode = OpaqueNodeMethods::from_script_node(requested_node); let mut iterator = FragmentLocatingFragmentIterator::new(requested_node); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); - rw_data.client_rect_response = iterator.client_rect; + iterator.client_rect } - // Compute the resolved value of property for a given (pseudo)element. - // Stores the result in rw_data.resolved_style_response. - // https://drafts.csswg.org/cssom/#resolved-value - fn process_resolved_style_request<'a>(&'a self, - requested_node: TrustedNodeAddress, - pseudo: &Option<PseudoElement>, - property: &Atom, - layout_root: &mut FlowRef, - rw_data: &mut RWGuard<'a>) { + /// Return the resolved value of property for a given (pseudo)element. + /// https://drafts.csswg.org/cssom/#resolved-value + fn process_resolved_style_request(&self, + requested_node: TrustedNodeAddress, + pseudo: &Option<PseudoElement>, + property: &Atom, + layout_root: &mut FlowRef) + -> Option<String> { // FIXME: Isolate this transmutation into a "bridge" module. // FIXME(rust#16366): The following line had to be moved because of a // rustc bug. It should be in the next unsafe block. @@ -918,8 +917,7 @@ impl LayoutTask { // The pseudo doesn't exist, return nothing. Chrome seems to query // the element itself in this case, Firefox uses the resolved value. // https://www.w3.org/Bugs/Public/show_bug.cgi?id=29006 - rw_data.resolved_style_response = None; - return; + return None; } Some(layout_node) => layout_node }; @@ -970,7 +968,7 @@ impl LayoutTask { style.writing_mode); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); - rw_data.resolved_style_response = iterator.result.map(|r| r.to_css_string()); + iterator.result.map(|r| r.to_css_string()) }, atom!("bottom") | atom!("top") | atom!("right") | @@ -1003,20 +1001,19 @@ impl LayoutTask { position); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); - rw_data.resolved_style_response = iterator.result.map(|r| r.to_css_string()); + iterator.result.map(|r| r.to_css_string()) }, // FIXME: implement used value computation for line-height ref property => { - rw_data.resolved_style_response = - style.computed_value_to_string(property.as_slice()).ok(); + style.computed_value_to_string(property.as_slice()).ok() } - }; + } } - fn process_offset_parent_query<'a>(&'a self, - requested_node: TrustedNodeAddress, - layout_root: &mut FlowRef, - rw_data: &mut RWGuard<'a>) { + fn process_offset_parent_query(&self, + requested_node: TrustedNodeAddress, + layout_root: &mut FlowRef) + -> OffsetParentResponse { let requested_node: OpaqueNode = OpaqueNodeMethods::from_script_node(requested_node); let mut iterator = ParentOffsetBorderBoxIterator::new(requested_node); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); @@ -1026,13 +1023,13 @@ impl LayoutTask { let parent = iterator.parent_nodes[parent_info_index].as_ref().unwrap(); let origin = iterator.node_border_box.origin - parent.border_box.origin; let size = iterator.node_border_box.size; - rw_data.offset_parent_response = OffsetParentResponse { + OffsetParentResponse { node_address: Some(parent.node_address.to_untrusted_node_address()), rect: Rect::new(origin, size), - }; + } } None => { - rw_data.offset_parent_response = OffsetParentResponse::empty(); + OffsetParentResponse::empty() } } } @@ -1231,20 +1228,20 @@ impl LayoutTask { if let Some(mut root_flow) = rw_data.layout_root() { match data.query_type { ReflowQueryType::ContentBoxQuery(node) => - process_content_box_request(node, &mut root_flow, &mut rw_data), + rw_data.content_box_response = process_content_box_request(node, &mut root_flow), ReflowQueryType::ContentBoxesQuery(node) => - process_content_boxes_request(node, &mut root_flow, &mut rw_data), + rw_data.content_boxes_response = process_content_boxes_request(node, &mut root_flow), ReflowQueryType::NodeGeometryQuery(node) => - self.process_node_geometry_request(node, &mut root_flow, &mut rw_data), + rw_data.client_rect_response = self.process_node_geometry_request(node, &mut root_flow), ReflowQueryType::ResolvedStyleQuery(node, ref pseudo, ref property) => { - self.process_resolved_style_request(node, - pseudo, - property, - &mut root_flow, - &mut rw_data) + rw_data.resolved_style_response = + self.process_resolved_style_request(node, + pseudo, + property, + &mut root_flow) } ReflowQueryType::OffsetParentQuery(node) => - self.process_offset_parent_query(node, &mut root_flow, &mut rw_data), + rw_data.offset_parent_response = self.process_offset_parent_query(node, &mut root_flow), ReflowQueryType::NoQuery => {} } } diff --git a/components/layout/query.rs b/components/layout/query.rs index 024d3022288..d50e150d0ae 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -10,7 +10,7 @@ use euclid::rect::Rect; use flow_ref::FlowRef; use fragment::{Fragment, FragmentBorderBoxIterator}; use gfx::display_list::{DisplayItemMetadata, OpaqueNode}; -use layout_task::{LayoutTaskData, RWGuard}; +use layout_task::LayoutTaskData; use msg::constellation_msg::ConstellationChan; use msg::constellation_msg::Msg as ConstellationMsg; use opaque_node::OpaqueNodeMethods; @@ -281,27 +281,27 @@ impl FragmentBorderBoxIterator for MarginRetrievingFragmentBorderBoxIterator { } } -pub fn process_content_box_request<'a>(requested_node: TrustedNodeAddress, - layout_root: &mut FlowRef, - rw_data: &mut RWGuard<'a>) { +pub fn process_content_box_request(requested_node: TrustedNodeAddress, + layout_root: &mut FlowRef) + -> Rect<Au> { // FIXME(pcwalton): This has not been updated to handle the stacking context relative // stuff. So the position is wrong in most cases. let requested_node: OpaqueNode = OpaqueNodeMethods::from_script_node(requested_node); let mut iterator = UnioningFragmentBorderBoxIterator::new(requested_node); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); - rw_data.content_box_response = match iterator.rect { + match iterator.rect { Some(rect) => rect, None => Rect::zero() - }; + } } -pub fn process_content_boxes_request<'a>(requested_node: TrustedNodeAddress, - layout_root: &mut FlowRef, - rw_data: &mut RWGuard<'a>) { +pub fn process_content_boxes_request(requested_node: TrustedNodeAddress, + layout_root: &mut FlowRef) + -> Vec<Rect<Au>> { // FIXME(pcwalton): This has not been updated to handle the stacking context relative // stuff. So the position is wrong in most cases. let requested_node: OpaqueNode = OpaqueNodeMethods::from_script_node(requested_node); let mut iterator = CollectingFragmentBorderBoxIterator::new(requested_node); sequential::iterate_through_flow_tree_fragment_border_boxes(layout_root, &mut iterator); - rw_data.content_boxes_response = iterator.rects; + iterator.rects } diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index e2c0bd89f19..c59fb978e38 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -30,13 +30,11 @@ #![allow(unsafe_code)] -use canvas_traits::CanvasMsg; use context::SharedLayoutContext; use data::{LayoutDataFlags, LayoutDataWrapper, PrivateLayoutData}; use gfx::display_list::OpaqueNode; use gfx::text::glyph::CharIndex; use incremental::RestyleDamage; -use ipc_channel::ipc::IpcSender; use msg::constellation_msg::PipelineId; use opaque_node::OpaqueNodeMethods; use script::dom::attr::AttrValue; @@ -47,7 +45,7 @@ use script::dom::bindings::js::LayoutJS; use script::dom::characterdata::LayoutCharacterDataHelpers; use script::dom::element; use script::dom::element::{Element, LayoutElementHelpers, RawLayoutElementHelpers}; -use script::dom::htmlcanvaselement::LayoutHTMLCanvasElementHelpers; +use script::dom::htmlcanvaselement::{LayoutHTMLCanvasElementHelpers, HTMLCanvasData}; use script::dom::htmliframeelement::HTMLIFrameElement; use script::dom::htmlimageelement::LayoutHTMLImageElementHelpers; use script::dom::htmlinputelement::{HTMLInputElement, LayoutHTMLInputElementHelpers}; @@ -945,31 +943,10 @@ impl<'ln> ThreadSafeLayoutNode<'ln> { } } - pub fn canvas_renderer_id(&self) -> Option<usize> { + pub fn canvas_data(&self) -> Option<HTMLCanvasData> { unsafe { let canvas_element = self.get_jsmanaged().downcast(); - canvas_element.and_then(|elem| elem.get_renderer_id()) - } - } - - pub fn canvas_ipc_renderer(&self) -> Option<IpcSender<CanvasMsg>> { - unsafe { - let canvas_element = self.get_jsmanaged().downcast(); - canvas_element.and_then(|elem| elem.get_ipc_renderer()) - } - } - - pub fn canvas_width(&self) -> u32 { - unsafe { - let canvas_element = self.get_jsmanaged().downcast(); - canvas_element.unwrap().get_canvas_width() - } - } - - pub fn canvas_height(&self) -> u32 { - unsafe { - let canvas_element = self.get_jsmanaged().downcast(); - canvas_element.unwrap().get_canvas_height() + canvas_element.map(|canvas| canvas.data()) } } @@ -1076,9 +1053,7 @@ pub fn layout_node_to_unsafe_layout_node(node: &LayoutNode) -> UnsafeLayoutNode } } -// FIXME(#3044): This should be updated to use a real lifetime instead of -// faking one. -pub unsafe fn layout_node_from_unsafe_layout_node(node: &UnsafeLayoutNode) -> LayoutNode<'static> { +pub unsafe fn layout_node_from_unsafe_layout_node(node: &UnsafeLayoutNode) -> LayoutNode { let (node, _) = *node; mem::transmute(node) } diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index 7abebcac971..a2ec90033ba 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -273,6 +273,22 @@ impl<T: HeapGCValue> HeapSizeOf for MutHeap<T> { } } +impl<T: Reflectable> PartialEq for MutHeap<JS<T>> { + fn eq(&self, other: &Self) -> bool { + unsafe { + *self.val.get() == *other.val.get() + } + } +} + +impl<T: Reflectable + PartialEq> PartialEq<T> for MutHeap<JS<T>> { + fn eq(&self, other: &T) -> bool { + unsafe { + **self.val.get() == *other + } + } +} + /// A holder that provides interior mutability for GC-managed values such as /// `JS<T>`, with nullability represented by an enclosing Option wrapper. /// Essentially a `Cell<Option<JS<T>>>`, but safer. @@ -334,6 +350,23 @@ impl<T: Reflectable> MutNullableHeap<JS<T>> { *self.ptr.get() = val.map(|p| JS::from_ref(p)); } } + +} + +impl<T: Reflectable> PartialEq for MutNullableHeap<JS<T>> { + fn eq(&self, other: &Self) -> bool { + unsafe { + *self.ptr.get() == *other.ptr.get() + } + } +} + +impl<'a, T: Reflectable> PartialEq<Option<&'a T>> for MutNullableHeap<JS<T>> { + fn eq(&self, other: &Option<&T>) -> bool { + unsafe { + *self.ptr.get() == other.map(JS::from_ref) + } + } } impl<T: HeapGCValue> Default for MutNullableHeap<T> { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 23d455a1903..771342d0f36 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -1147,7 +1147,7 @@ impl Document { }) } - fn get_element_by_id(&self, id: &Atom) -> Option<Root<Element>> { + pub fn get_element_by_id(&self, id: &Atom) -> Option<Root<Element>> { self.idmap.borrow().get(&id).map(|ref elements| (*elements)[0].root()) } } diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs index d6e484a2f9d..2fdb8a129e6 100644 --- a/components/script/dom/htmlcanvaselement.rs +++ b/components/script/dom/htmlcanvaselement.rs @@ -89,48 +89,41 @@ impl HTMLCanvasElement { } } +pub struct HTMLCanvasData { + pub renderer_id: Option<usize>, + pub ipc_renderer: Option<IpcSender<CanvasMsg>>, + pub width: u32, + pub height: u32, +} + pub trait LayoutHTMLCanvasElementHelpers { - #[allow(unsafe_code)] - unsafe fn get_renderer_id(&self) -> Option<usize>; - #[allow(unsafe_code)] - unsafe fn get_ipc_renderer(&self) -> Option<IpcSender<CanvasMsg>>; - #[allow(unsafe_code)] - unsafe fn get_canvas_width(&self) -> u32; - #[allow(unsafe_code)] - unsafe fn get_canvas_height(&self) -> u32; + fn data(&self) -> HTMLCanvasData; } impl LayoutHTMLCanvasElementHelpers for LayoutJS<HTMLCanvasElement> { #[allow(unsafe_code)] - unsafe fn get_renderer_id(&self) -> Option<usize> { - let ref canvas = *self.unsafe_get(); - canvas.context.borrow_for_layout().as_ref().map(|context| { - match *context { - CanvasContext::Context2d(ref context) => context.to_layout().get_renderer_id(), - CanvasContext::WebGL(ref context) => context.to_layout().get_renderer_id(), - } - }) - } + fn data(&self) -> HTMLCanvasData { + unsafe { + let canvas = &*self.unsafe_get(); + let (renderer_id, ipc_renderer) = match canvas.context.borrow_for_layout().as_ref() { + Some(&CanvasContext::Context2d(ref context)) => { + let context = context.to_layout(); + (Some(context.get_renderer_id()), Some(context.get_ipc_renderer())) + }, + Some(&CanvasContext::WebGL(ref context)) => { + let context = context.to_layout(); + (Some(context.get_renderer_id()), Some(context.get_ipc_renderer())) + }, + None => (None, None), + }; - #[allow(unsafe_code)] - unsafe fn get_ipc_renderer(&self) -> Option<IpcSender<CanvasMsg>> { - let ref canvas = *self.unsafe_get(); - canvas.context.borrow_for_layout().as_ref().map(|context| { - match *context { - CanvasContext::Context2d(ref context) => context.to_layout().get_ipc_renderer(), - CanvasContext::WebGL(ref context) => context.to_layout().get_ipc_renderer(), + HTMLCanvasData { + renderer_id: renderer_id, + ipc_renderer: ipc_renderer, + width: canvas.width.get(), + height: canvas.height.get(), } - }) - } - - #[allow(unsafe_code)] - unsafe fn get_canvas_width(&self) -> u32 { - (*self.unsafe_get()).width.get() - } - - #[allow(unsafe_code)] - unsafe fn get_canvas_height(&self) -> u32 { - (*self.unsafe_get()).height.get() + } } } diff --git a/components/script/dom/htmlelement.rs b/components/script/dom/htmlelement.rs index 34a7847187a..b04082eae4d 100644 --- a/components/script/dom/htmlelement.rs +++ b/components/script/dom/htmlelement.rs @@ -296,6 +296,26 @@ impl HTMLElement { let local_name = Atom::from_slice(&to_snake_case(local_name)); self.upcast::<Element>().remove_attribute(&ns!(""), &local_name); } + + // https://html.spec.whatwg.org/multipage/#category-label + pub fn is_labelable_element(&self) -> bool { + // Note: HTMLKeygenElement is omitted because Servo doesn't currently implement it + match self.upcast::<Node>().type_id() { + NodeTypeId::Element(ElementTypeId::HTMLElement(type_id)) => + match type_id { + HTMLElementTypeId::HTMLInputElement => + self.downcast::<HTMLInputElement>().unwrap().Type() != "hidden", + HTMLElementTypeId::HTMLButtonElement | + HTMLElementTypeId::HTMLMeterElement | + HTMLElementTypeId::HTMLOutputElement | + HTMLElementTypeId::HTMLProgressElement | + HTMLElementTypeId::HTMLSelectElement | + HTMLElementTypeId::HTMLTextAreaElement => true, + _ => false, + }, + _ => false, + } + } } impl VirtualMethods for HTMLElement { diff --git a/components/script/dom/htmllabelelement.rs b/components/script/dom/htmllabelelement.rs index 3beefc3db9a..bcda8df24cc 100644 --- a/components/script/dom/htmllabelelement.rs +++ b/components/script/dom/htmllabelelement.rs @@ -2,13 +2,18 @@ * 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::attr::AttrValue; use dom::bindings::codegen::Bindings::HTMLLabelElementBinding; use dom::bindings::codegen::Bindings::HTMLLabelElementBinding::HTMLLabelElementMethods; +use dom::bindings::conversions::Castable; use dom::bindings::js::Root; use dom::document::Document; +use dom::element::Element; use dom::htmlelement::HTMLElement; use dom::htmlformelement::{FormControl, HTMLFormElement}; -use dom::node::Node; +use dom::node::{document_from_node, Node}; +use dom::virtualmethods::VirtualMethods; +use string_cache::Atom; use util::str::DOMString; #[dom_struct] @@ -40,6 +45,54 @@ impl HTMLLabelElementMethods for HTMLLabelElement { fn GetForm(&self) -> Option<Root<HTMLFormElement>> { self.form_owner() } + + // https://html.spec.whatwg.org/multipage/#dom-label-htmlfor + make_getter!(HtmlFor, "for"); + + // https://html.spec.whatwg.org/multipage/#dom-label-htmlfor + make_atomic_setter!(SetHtmlFor, "for"); + + // https://html.spec.whatwg.org/multipage/#dom-label-control + fn GetControl(&self) -> Option<Root<HTMLElement>> { + if !self.upcast::<Node>().is_in_doc() { + return None; + } + + let for_attr = match self.upcast::<Element>().get_attribute(&ns!(""), &atom!("for")) { + Some(for_attr) => for_attr, + None => return self.first_labelable_descendant(), + }; + + let for_value = for_attr.value(); + document_from_node(self).get_element_by_id(for_value.as_atom()) + .and_then(Root::downcast::<HTMLElement>) + .into_iter() + .filter(|e| e.is_labelable_element()) + .next() + } +} + +impl VirtualMethods for HTMLLabelElement { + fn super_type(&self) -> Option<&VirtualMethods> { + Some(self.upcast::<HTMLElement>() as &VirtualMethods) + } + + fn parse_plain_attribute(&self, name: &Atom, value: DOMString) -> AttrValue { + match name { + &atom!("for") => AttrValue::from_atomic(value), + _ => self.super_type().unwrap().parse_plain_attribute(name, value), + } + } +} + +impl HTMLLabelElement { + fn first_labelable_descendant(&self) -> Option<Root<HTMLElement>> { + self.upcast::<Node>() + .traverse_preorder() + .filter_map(Root::downcast::<HTMLElement>) + .filter(|elem| elem.is_labelable_element()) + .next() + } } impl FormControl for HTMLLabelElement {} diff --git a/components/script/dom/virtualmethods.rs b/components/script/dom/virtualmethods.rs index da80f414da4..32406205e5d 100644 --- a/components/script/dom/virtualmethods.rs +++ b/components/script/dom/virtualmethods.rs @@ -25,6 +25,7 @@ use dom::htmlheadelement::HTMLHeadElement; use dom::htmliframeelement::HTMLIFrameElement; use dom::htmlimageelement::HTMLImageElement; use dom::htmlinputelement::HTMLInputElement; +use dom::htmllabelelement::HTMLLabelElement; use dom::htmllinkelement::HTMLLinkElement; use dom::htmlmetaelement::HTMLMetaElement; use dom::htmlobjectelement::HTMLObjectElement; @@ -164,6 +165,9 @@ pub fn vtable_for(node: &Node) -> &VirtualMethods { NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLInputElement)) => { node.downcast::<HTMLInputElement>().unwrap() as &VirtualMethods } + NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLLabelElement)) => { + node.downcast::<HTMLLabelElement>().unwrap() as &VirtualMethods + } NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLLinkElement)) => { node.downcast::<HTMLLinkElement>().unwrap() as &VirtualMethods } diff --git a/components/script/dom/webidls/HTMLLabelElement.webidl b/components/script/dom/webidls/HTMLLabelElement.webidl index 00900df77fc..22659c26e76 100644 --- a/components/script/dom/webidls/HTMLLabelElement.webidl +++ b/components/script/dom/webidls/HTMLLabelElement.webidl @@ -6,6 +6,6 @@ // https://html.spec.whatwg.org/multipage/#htmllabelelement interface HTMLLabelElement : HTMLElement { readonly attribute HTMLFormElement? form; - // attribute DOMString htmlFor; - //readonly attribute HTMLElement? control; + attribute DOMString htmlFor; + readonly attribute HTMLElement? control; }; diff --git a/tests/wpt/metadata/html/dom/interfaces.html.ini b/tests/wpt/metadata/html/dom/interfaces.html.ini index d45ac708e52..a72593fcf48 100644 --- a/tests/wpt/metadata/html/dom/interfaces.html.ini +++ b/tests/wpt/metadata/html/dom/interfaces.html.ini @@ -4869,18 +4869,6 @@ [HTMLLabelElement interface: existence and properties of interface object] expected: FAIL - [HTMLLabelElement interface: attribute htmlFor] - expected: FAIL - - [HTMLLabelElement interface: attribute control] - expected: FAIL - - [HTMLLabelElement interface: document.createElement("label") must inherit property "htmlFor" with the proper type (1)] - expected: FAIL - - [HTMLLabelElement interface: document.createElement("label") must inherit property "control" with the proper type (2)] - expected: FAIL - [HTMLInputElement interface: existence and properties of interface object] expected: FAIL diff --git a/tests/wpt/metadata/html/dom/reflection-forms.html.ini b/tests/wpt/metadata/html/dom/reflection-forms.html.ini index 851591ec145..21effce681d 100644 --- a/tests/wpt/metadata/html/dom/reflection-forms.html.ini +++ b/tests/wpt/metadata/html/dom/reflection-forms.html.ini @@ -2913,135 +2913,6 @@ [label.tabIndex: IDL set to -2147483648 followed by getAttribute()] expected: FAIL - [label.htmlFor (<label for>): typeof IDL attribute] - expected: FAIL - - [label.htmlFor (<label for>): IDL get with DOM attribute unset] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to "" followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to " \\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07 \\b\\t\\n\\v\\f\\r\\x0e\\x0f \\x10\\x11\\x12\\x13\\x14\\x15\\x16\\x17 \\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f foo " followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to undefined followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to 7 followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to 1.5 followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to true followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to false followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to object "[object Object\]" followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to NaN followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to Infinity followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to -Infinity followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to "\\0" followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to null followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to object "test-toString" followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): setAttribute() to object "test-valueOf" followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to "" followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to " \\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07 \\b\\t\\n\\v\\f\\r\\x0e\\x0f \\x10\\x11\\x12\\x13\\x14\\x15\\x16\\x17 \\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f foo " followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to undefined followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to undefined followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to 7 followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to 7 followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to 1.5 followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to 1.5 followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to true followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to true followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to false followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to false followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to object "[object Object\]" followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to object "[object Object\]" followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to NaN followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to NaN followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to Infinity followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to Infinity followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to -Infinity followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to -Infinity followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to "\\0" followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to null followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to null followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to object "test-toString" followed by getAttribute()] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to object "test-toString" followed by IDL get] - expected: FAIL - - [label.htmlFor (<label for>): IDL set to object "test-valueOf" followed by IDL get] - expected: FAIL - [label.itemScope: typeof IDL attribute] expected: FAIL diff --git a/tests/wpt/metadata/html/semantics/forms/the-label-element/label-attributes.html.ini b/tests/wpt/metadata/html/semantics/forms/the-label-element/label-attributes.html.ini index 30ba26e5898..655c489ce75 100644 --- a/tests/wpt/metadata/html/semantics/forms/the-label-element/label-attributes.html.ini +++ b/tests/wpt/metadata/html/semantics/forms/the-label-element/label-attributes.html.ini @@ -1,23 +1,8 @@ [label-attributes.html] type: testharness - [A label element with a 'for' attribute should only be associated with a labelable element.] - expected: FAIL - - [A label element not in a document can not label any element in the document.] - expected: FAIL - - [The labeled control for a label element that has no 'for' attribute is the first labelable element which is a descendant of that label element.] - expected: FAIL - - [The 'for' attribute points to an inexistent id.] - expected: FAIL - [A non-control follows by a control with same ID.] expected: FAIL - [The 'for' attribute is an empty string.] - expected: FAIL - [A form control has multiple labels.] expected: FAIL @@ -27,6 +12,3 @@ [A form control has no label 2.] expected: FAIL - [A label's htmlFor attribute must reflect the for content attribute] - expected: FAIL - diff --git a/tests/wpt/metadata/html/semantics/forms/the-label-element/labelable-elements.html.ini b/tests/wpt/metadata/html/semantics/forms/the-label-element/labelable-elements.html.ini index d31ee3c61c8..be4d7c998ba 100644 --- a/tests/wpt/metadata/html/semantics/forms/the-label-element/labelable-elements.html.ini +++ b/tests/wpt/metadata/html/semantics/forms/the-label-element/labelable-elements.html.ini @@ -1,41 +1,5 @@ [labelable-elements.html] type: testharness - [Check if the output element is a labelable element] - expected: FAIL - - [Check if the progress element is a labelable element] - expected: FAIL - - [Check if the select element is a labelable element] - expected: FAIL - - [Check if the textarea element is a labelable form-element] - expected: FAIL - - [Check if the button element is a labelable element] - expected: FAIL - - [Check if the hidden input element is not a labelable element.] - expected: FAIL - - [Check if the input element in radio state is a labelable element] - expected: FAIL - [Check if the keygen element is a labelable element] expected: FAIL - [Check if the meter element is a labelable element] - expected: FAIL - - [Check if the fieldset element is not a labelable element] - expected: FAIL - - [Check if the label element is not a labelable element] - expected: FAIL - - [Check if the object element is not a labelable element] - expected: FAIL - - [Check if the img element is not a labelable element] - expected: FAIL - |