Sequential layout bug with (possibly) RTL direction

[avadacatavra]

When we use the sequential layout traversal, there's an error with overconstrained_block.html and div_align.html.

Discovered while testing #11713

[avadacatavra]

It seems like there are two main issues:

1. In the sequential code, RTL isn't set properly
   • boxes 2 and 4 should both be RTL, but only 2 is
   • if you set all 4 blocks to be RTL, only 1 and 2 are
2. Neither parallel or sequential match the Firefox reference
   • But parallel mode with a single thread does (y1 screenshot)

Screenshots

[avadacatavra]

@mbrubeck

[pcwalton]

This could have something to do with dirty bits. Perhaps we are checking different dirty bits in parallel mode and sequential mode and incorrectly bailing out earlier in the latter?

[metajack]

Would disabling incremental layout make the problem go away in that case? That seems like an easy test.

[pcwalton]

Yes, it's worth a shot!

[avadacatavra]

Here's the test run in sequential mode with incremental layout disabled. Didn't go away :(

[notriddle]

When I force it to combine sequential restyle with parallel layout, I get both outputs depending on whether Satan manages to get into my computer before Hades blocks the entry off.

Both of these screenshots were produced by running this command:

./mach run -d -y2 tests/wpt/mozilla/tests/css/overconstrained_block.html

And I used this patch:

diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs
index 403f30a..b023187 100644
--- a/components/layout_thread/lib.rs
+++ b/components/layout_thread/lib.rs
@@ -1155,17 +1155,8 @@ impl LayoutThread {
                     self.profiler_metadata(),
                     self.time_profiler_chan.clone(),
                     || {
-                // Perform CSS selector matching and flow construction.
-                match self.parallel_traversal {
-                    None => {
-                        sequential::traverse_dom::<ServoLayoutNode, RecalcStyleAndConstructFlows>(
-                            node, &shared_layout_context);
-                    }
-                    Some(ref mut traversal) => {
-                        parallel::traverse_dom::<ServoLayoutNode, RecalcStyleAndConstructFlows>(
-                            node, &shared_layout_context, traversal);
-                    }
-                }
+                sequential::traverse_dom::<ServoLayoutNode, RecalcStyleAndConstructFlows>(
+                    node, &shared_layout_context);
             });

             // TODO(pcwalton): Measure energy usage of text shaping, perhaps?

I don't think there's anything interesting about the combination of parallel layout and sequential restyle, except for how it forces the race to go.