Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run WPT with Layout 2020 on CI #24871

Merged
merged 13 commits into from Nov 27, 2019
Merged

Run WPT with Layout 2020 on CI #24871

merged 13 commits into from Nov 27, 2019

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Nov 26, 2019

… and gate PRs on the result.

SimonSapin added 5 commits Nov 26, 2019
… and has a private enum for its contents.

Privacy forces the rest of the code to go through methods
rather than matching on the enum,
reducing accidental layout-mode-specific behavior.
@jdm
Copy link
Member

jdm commented Nov 26, 2019

r? @nox

SimonSapin and others added 4 commits Nov 26, 2019
@SimonSapin SimonSapin force-pushed the 2020-ci branch from b5cbb1a to 858bc5a Nov 26, 2019
@nox
Copy link
Member

nox commented Nov 26, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Nov 26, 2019

📌 Commit 858bc5a has been approved by nox

@SimonSapin
Copy link
Member Author

SimonSapin commented Nov 26, 2019

@nox We’ve got intermittent failures :(

  ▶ TIMEOUT [expected CRASH] /css/CSS2/box-display/block-in-inline-relpos-001.xht
  │ 
  │ 
  │ 
  │ 
  │ No protocol specified
  │ assertion failed: self.font_size.map_or(true, |f| f == font_size) (thread LayoutThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }, at components/style/rule_cache.rs:29)
  │ stack backtrace:
  │    0: servo::main::{{closure}}
  │    1: std::panicking::rust_panic_with_hook
  │              at src/libstd/panicking.rs:468
  │    2: std::panicking::begin_panic
  │    3: style::values::specified::length::FontRelativeLength::to_computed_value
  │    4: style::values::computed::length::<impl style::values::computed::ToComputedValue for style::values::specified::length::LengthPercentage>::to_computed_value
  │    5: style::properties::longhands::margin_left::cascade_property
  │    6: style::properties::cascade::Cascade::apply_properties
  │    7: _ZN5style10properties7cascade13cascade_rules17h96faf43c84170c77E.llvm.7578349383558476548
  │    8: style::stylist::Stylist::cascade_style_and_visited
  │    9: style::style_resolver::StyleResolverForElement<E>::cascade_primary_style
  │   10: style::style_resolver::StyleResolverForElement<E>::resolve_style_with_default_parents
  │   11: style::traversal::compute_style
  │   12: <layout::traversal::RecalcStyle as style::traversal::DomTraversal<E>>::process_preorder
  │   13: style::driver::traverse_dom
  │   14: layout_thread::LayoutThread::handle_reflow
  │   15: profile_traits::time::profile
  │   16: layout_thread::LayoutThread::handle_request_helper
  │   17: layout_thread::LayoutThread::start
  │   18: profile_traits::mem::ProfilerChan::run_with_memory_reporting
  │   19: std::sys_common::backtrace::__rust_begin_short_backtrace
  │   20: _ZN3std9panicking3try7do_call17ha66e0d91de444c54E.llvm.1363856088101530016
  │   21: __rust_maybe_catch_panic
  │              at src/libpanic_unwind/lib.rs:81
  │   22: core::ops::function::FnOnce::call_once{{vtable.shim}}
  │   23: <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once
  │              at /rustc/1bd30ce2aac40c7698aa4a1b9520aa649ff2d1c5/src/liballoc/boxed.rs:942
  │   24: <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once
  │              at /rustc/1bd30ce2aac40c7698aa4a1b9520aa649ff2d1c5/src/liballoc/boxed.rs:942
  │       std::sys_common::thread::start_thread
  │              at src/libstd/sys_common/thread.rs:13
  │       std::sys::unix::thread::Thread::new::thread_start
  │              at src/libstd/sys/unix/thread.rs:79
  │   25: start_thread
  │   26: __clone
  │ 
  │ [2019-11-26T21:59:23Z ERROR servo] assertion failed: self.font_size.map_or(true, |f| f == font_size)
  │ Redirecting call to abort() to mozalloc_abort
  └ 
@jdm
Copy link
Member

jdm commented Nov 26, 2019

@bors-servo r-
Removing from the queue so the authors can decide what to do about it at a reasonable hour of the day.

@nox
Copy link
Member

nox commented Nov 27, 2019

@SimonSapin I've seen this exactly once and could never reproduce it again.

@SimonSapin
Copy link
Member Author

SimonSapin commented Nov 27, 2019

I was able to reproduce with ./mach test-wpt --release --layout-2020 --repeat-until-unexpected, though after several repetitions. On CI however I didn’t get a single successful run:

These may not all be with the same expectations, I don’t remember exactly. But the problem is that the result for block-in-inline-relpos-*.xht is sometimes "crash" and sometimes "timeout". In both cases there’s a stack trace assertion failed: self.font_size.map_or(true, |f| f == font_size), but maybe after that the process sometimes hangs instead of exiting?

So we have two issues (and would ideally fix both):

  • A failing debug assertion
  • Intermittent hangs after a LayoutThread panics
@nox
Copy link
Member

nox commented Nov 27, 2019

I do not think this is an issue in the layout code, rather I suspect that we are doing something wrong when recalculating styles before we do the layout. I'm not familiar with this part of Stylo and have little idea where to start looking to debug the issue.

@nox
Copy link
Member

nox commented Nov 27, 2019

I added a commit that makes the assertion more helpful, and I got the following panic:

 0:03.59 pid:11674 assertion failed: `(left == right)`
 0:03.59 pid:11674   left: `NonNegative(16.0 px)`,
 0:03.59 pid:11674  right: `NonNegative(20.0 px)` (thread LayoutThread PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }, at components/style/rule_cache.rs:30)
@nox nox force-pushed the 2020-ci branch from 4ec79a9 to 9b4baf2 Nov 27, 2019
@nox
Copy link
Member

nox commented Nov 27, 2019

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2019

Trying commit 9b4baf2 with merge 0787371...

bors-servo added a commit that referenced this pull request Nov 27, 2019
Run WPT with Layout 2020 on CI

… and gate PRs on the result.
This also needs support in Homu’s configuration file
@SimonSapin
Copy link
Member Author

SimonSapin commented Nov 27, 2019

Homu forgot about this try because I pushed another commit, but results will still be at https://community-tc.services.mozilla.com/tasks/groups/bMEFWdVlTkW2hTgOIlFDFA

(This reachable through the orange/green/red dot near “bors-servo added a commit that referenced this pull request”.)

@SimonSapin
Copy link
Member Author

SimonSapin commented Nov 27, 2019

It’s green \o/

@bors-servo r=nox,SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2019

📌 Commit 9a4bc23 has been approved by nox,SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2019

Testing commit 9a4bc23 with merge 83bb967...

bors-servo added a commit that referenced this pull request Nov 27, 2019
Run WPT with Layout 2020 on CI

… and gate PRs on the result.
@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2019

☀️ Test successful - status-taskcluster
Approved by: nox,SimonSapin
Pushing 83bb967 to master...

@bors-servo bors-servo merged commit 9a4bc23 into master Nov 27, 2019
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@bors-servo bors-servo deleted the 2020-ci branch Nov 27, 2019
@SimonSapin SimonSapin added this to Done / resolved in Layout 2020 Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Layout 2020
  
Merged / resolved
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.