Closed
Bug 1413361
Opened 7 years ago
Closed 7 years ago
thread panicked at Resolving style on <html>
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
FIXED
mozilla58
| Tracking | Status | |
|---|---|---|
| firefox-esr52 | --- | unaffected |
| firefox56 | --- | unaffected |
| firefox57 | --- | wontfix |
| firefox58 | --- | fixed |
People
(Reporter: tsmith, Assigned: emilio)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, testcase)
Attachments
(3 files, 1 obsolete file)
thread '<unnamed>' panicked at 'Resolving style on <html> (0x60e00024d620) without current styles: ElementData { styles: ElementStyles { primary: Some(Some(StrongRuleNode { p: 0x607000440ac0 })), pseudos: EagerPseudoStyles(None) }, damage: GeckoRestyleDamage(nsChangeHint(0)), hint: RESTYLE_SELF | RESTYLE_DESCENDANTS, flags: (empty) }', /src/servo/ports/geckolib/glue.rs:3460:4
stack backtrace:
0: 0x7f4035f5c163 - std::sys::imp::backtrace::tracing::imp::unwind_backtrace::hfc7985b08e763a82
at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
1: 0x7f4035f569a4 - std::sys_common::backtrace::_print::h16a1db02a59ead63
at /checkout/src/libstd/sys_common/backtrace.rs:71
2: 0x7f4035f69883 - std::panicking::default_hook::{{closure}}::h48ecee46f2eefc30
at /checkout/src/libstd/sys_common/backtrace.rs:60
at /checkout/src/libstd/panicking.rs:381
3: 0x7f4035f695f2 - std::panicking::default_hook::hb4c92ae8d005ca44
at /checkout/src/libstd/panicking.rs:397
4: 0x7f4035f69d47 - std::panicking::rust_panic_with_hook::h25d461655d60b1a5
at /checkout/src/libstd/panicking.rs:611
5: 0x7f4035f69ba4 - std::panicking::begin_panic::h0f6fdd9abfd7dfb9
at /checkout/src/libstd/panicking.rs:572
6: 0x7f4035f69b19 - std::panicking::begin_panic_fmt::ha31e26b280c9e878
at /checkout/src/libstd/panicking.rs:522
7: 0x7f40357ce4a7 - Servo_ResolveStyle
at /src/servo/ports/geckolib/glue.rs:3460
8: 0x7f4030df1727 - _ZN7mozilla13ServoStyleSet17ResolveServoStyleEPNS_3dom7ElementE
at /src/layout/style/ServoStyleSet.cpp:1362
Flags: in-testsuite?
|
Reporter | |
Comment 1•7 years ago
|
||
|
Assignee | |
Updated•7 years ago
|
Flags: needinfo?(emilio)
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Updated•7 years ago
|
Assignee: nobody → emilio
Flags: needinfo?(emilio)
|
||
Comment 4•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8924134 [details]
Bug 1413361: ResolveStyleFor should not flush stuff.
https://reviewboard.mozilla.org/r/195388/#review200560
I don't understand the semantics of this stuff well enough to competently review this absent either better documentation or a better commit message. :(
In particular, the semantics _I_ understood for LazyComputeBehavior::Allow are "we have no idea what the state of the style system looks like, but we need up-to-date style, so update that state as needed".
In practice, most of the callers that use LazyComputeBehavior::Allow seem to be in frame code, so maybe they don't care about "really" getting up-to-date styles (and in fact can't, because flushing style from frame code is not safe). I don't know about the nsComputedDOMStyle::ResolveWithAnimation callsite.
::: commit-message-52e4f:7
(Diff revision 1)
> +
> +To keep the semantics closer between the two LazyComputeBehavior variants. We do
> +assumptions about the stylist and all that stuff being flushed in the other
> +variant, so we can and should do the same here.
> +
> +In particular, the restyle hint here comes from resolving the mapped
What restyle hint? Where "here"?
Attachment #8924134 -
Flags: review?(bzbarsky)
|
|
||
Comment 5•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8924135 [details]
Bug 1413361: EnsureFrameForTextNode shouldn't reconstruct synchronously without up-to-date styles.
https://reviewboard.mozilla.org/r/195390/#review200568
::: layout/base/nsCSSFrameConstructor.h:321
(Diff revision 1)
> void CharacterDataChanged(nsIContent* aContent,
> CharacterDataChangeInfo* aInfo);
>
> // If aContent is a text node that has been optimized away due to being
> - // whitespace next to a block boundary (or for some other reason), stop
> - // doing that and create a frame for it if it should have one. This recreates
> + // whitespace next to a block boundary (or for some other reason), ensure that
> + // a frame for it is created if it should have one in the next flush.
Maybe "is created the next time frames are flushed, if it can possibly have a frame at all"?
Right now it sounds like the call will create the frame "if it should have one in the next flush", which is not what it does.
Attachment #8924135 -
Flags: review?(bzbarsky) → review+
|
|
Assignee | |
Comment 6•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #4)
> In practice, most of the callers that use LazyComputeBehavior::Allow seem to
> be in frame code, so maybe they don't care about "really" getting up-to-date
> styles (and in fact can't, because flushing style from frame code is not
> safe). I don't know about the nsComputedDOMStyle::ResolveWithAnimation
> callsite.
Right.
Note that we don't do a style flush per se (we update the user font set, the stylist, etc), but we apparently rely on flushing stylesheets on that viewport scrollbar override call in order to get the proper scrollbars the first time, which is annoying and we should probably fix (I'll file):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6dd1cefe8f242bc41b774234b448836a621f29a7&selectedJob=141327543
> ::: commit-message-52e4f:7
> (Diff revision 1)
> > +
> > +To keep the semantics closer between the two LazyComputeBehavior variants. We do
> > +assumptions about the stylist and all that stuff being flushed in the other
> > +variant, so we can and should do the same here.
> > +
> > +In particular, the restyle hint here comes from resolving the mapped
>
> What restyle hint? Where "here"?
Sorry, should've commented with more detail. Here we panic because something has posted a restyle for the document element from ResolveMappedAttrDeclarationBlocks.
|
||
Comment 7•7 years ago
|
||
INFO: Last good revision: e16dba457260675669a0e81863849563b31a9ba2
INFO: First bad revision: 86b793bcbcd090a4189814f14204a2e0ea7929ef
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e16dba457260675669a0e81863849563b31a9ba2&tochange=86b793bcbcd090a4189814f14204a2e0ea7929ef
Blocks: 1383332
Has Regression Range: --- → yes
status-firefox56:
--- → unaffected
status-firefox57:
--- → wontfix
status-firefox-esr52:
--- → unaffected
Version: 58 Branch → 57 Branch
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8924134 -
Attachment is obsolete: true
|
|
Assignee | |
Comment 9•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8924135 [details]
Bug 1413361: EnsureFrameForTextNode shouldn't reconstruct synchronously without up-to-date styles.
https://reviewboard.mozilla.org/r/195390/#review200568
> Maybe "is created the next time frames are flushed, if it can possibly have a frame at all"?
>
> Right now it sounds like the call will create the frame "if it should have one in the next flush", which is not what it does.
Much better :)
| Comment hidden (mozreview-request) |
|
||
Comment 11•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.
hg error in cmd: hg rebase -s 0f4ad53b4c90 -d b022ff43f4bb: rebasing 431561:0f4ad53b4c90 "Bug 1413361: EnsureFrameForTextNode shouldn't reconstruct synchronously without up-to-date styles. r=bz" (tip)
merging layout/base/nsCSSFrameConstructor.cpp
merging layout/style/crashtests/crashtests.list
warning: conflicts while merging layout/style/crashtests/crashtests.list! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
| Comment hidden (mozreview-request) |
|
||
Comment 13•7 years ago
|
||
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/043ae7cc7d2e
EnsureFrameForTextNode shouldn't reconstruct synchronously without up-to-date styles. r=bz
|
||
Comment 14•7 years ago
|
||
| bugherder | ||
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•