[Stylo] Crash on google doc (blocks Hasal)

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
P1
normal
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: shinglyu, Assigned: manishearth)

Tracking

(Blocks: 1 bug, {crash})

unspecified
x86_64
Linux
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55 fixed, firefox-esr52 unaffected)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 months ago
STR:
Stylo crashes while opening the Hasal test case (google doc):

https://docs.google.com/document/d/1h0WtAA4-QOUfgH6f8FfoK8cawj2CC1ICkdY4TtQrOn0/edit

This will block the Hasal performance testing.

Environment:
- Ubuntu 16.04.1 x64
- hg commit 38ddd770441079bfae4a2ed7a0d41d9670c1f3dc
- Both e10s and non-e10s crashes

Log:

 0:00.12 /home/shinglyu/workspace/stylo/stylo-incubator-2/obj-x86_64-pc-linux-gnu/dist/bin/firefox -no-remote -profile /home/shinglyu/workspace/stylo/stylo-incubator-2/obj-x86_64-pc-linux-gnu/tmp/scratch_user
ERROR:geckoservo::glue: stylo: cannot handle longhand Display from presentation attribute
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: stylo: cannot handle longhand Display from presentation attribute
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: stylo: unknown presentation property with id eCSSProperty__x_lang
ERROR:geckoservo::glue: stylo: unknown presentation property with id eCSSProperty_text_emphasis_position
ERROR:geckoservo::glue: stylo: cannot handle longhand BorderBottomWidth from presentation attribute
ERROR:geckoservo::glue: stylo: cannot handle longhand BorderLeftWidth from presentation attribute
ERROR:geckoservo::glue: stylo: cannot handle longhand BorderRightWidth from presentation attribute
ERROR:geckoservo::glue: stylo: cannot handle longhand BorderTopWidth from presentation attribute
ERROR:geckoservo::glue: stylo: cannot handle longhand Display from presentation attribute
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
ERROR:geckoservo::glue: Unnecessary call to traverse_subtree
[Parent 17819] WARNING: pipe error (77): Connection reset by peer: file /home/shinglyu/workspace/stylo/stylo-incubator-2/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 346
[Parent 17819] WARNING: pipe error (71): Connection reset by peer: file /home/shinglyu/workspace/stylo/stylo-incubator-2/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 346

###!!! [Parent][MessageChannel] Error: (msgtype=0x2C0085,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv
Do you have a stack?
(Reporter)

Comment 2

4 months ago
Created attachment 8839387 [details]
backtrace.log

GDB log, including the backtrace. 

Seems like it's caused by a <span> element during frame construction. But I can't pinpoint the exact element. If I saved the file to a file locally, it doesn't trigger the crash, so looks like it's caused by something that was loaded dynamically.
Flags: needinfo?(emilio+bugs)
So that path is in a Gecko codepath. Is that a debug build? If it's not, it may be worth trying with it to see if it's an assertion failure before that.
Flags: needinfo?(emilio+bugs) → needinfo?(slyu)
(Reporter)

Comment 4

4 months ago
I did some debugging and here is what I found:

It's a servo declaration appear in the gecko code path. It seems to be a <span> element doing frame (re)construction. The <span> element's parent document is an iframe element, but I can't figure out which one it is, there are too many in the google doc page, and it seems to generate more with JavaScript. 

What I'm trying to find is 1) When and how the declaration was generated, why is it a Servo one? 2) Why did we choose to go down the Gecko path? I had no luck reducing the testcase, so I'm spending a lot of time waiting for rr to reverse-continue :(
Flags: needinfo?(slyu)
> What I'm trying to find is 1) When and how the declaration was generated, why is it a Servo one? 2) Why did we choose to go down the Gecko path? I had no luck reducing the testcase, so I'm spending a lot of time waiting for rr to reverse-continue :(

Thanks for reminding me about this bug!

So what seems to happen is the following:

 * Create <span> element in Servo-styled document, add a "style" attribute (which is a Servo declaration).
 * Adopt the <span> from a Gecko-styled document. This can be an SVG document, or other kind of stuff we don't support right now, see nsLayoutUtils::SupportsServoStyleBackendType[1].
 * Try to style that element from Gecko's frame constructor, that goes to Gecko's StyleSet, that ends up calling nsHTMLCSSStyleSheet::ElementRulesMatching, that grabs the style declaration at[2], and assumes it's Gecko.

We should presumably re-parse the style attribute when the node is adopted from another document or similar.

[1]: http://searchfox.org/mozilla-central/rev/9c1c7106eef137e3413fd867fc1ddfb1d3f6728c/layout/base/nsLayoutUtils.cpp#9168
[2]: http://searchfox.org/mozilla-central/rev/9c1c7106eef137e3413fd867fc1ddfb1d3f6728c/layout/style/nsHTMLCSSStyleSheet.cpp#69
(Reporter)

Comment 6

4 months ago
Oh additional clue:

I dump the JS stack and found it was in the i18n file `2236602414-kix_main_i18n_kix_core__zh_tw.js` (Google select the Traditional Chinese version based on my IP). The code in there was minified, but the API looks like some form of JavaScript version of ICU [1].

Can you tell me more about the "adoption" of node? All I can find is the DOM `document.adoptNode()` and `importNode()` API, is that what you are refering to?

[1] http://site.icu-project.org/
Flags: needinfo?(emilio+bugs)
(Reporter)

Comment 7

4 months ago
JS Stack: 

From the arguments, it appears to be setting the CSS on an element using JavaScript? 

------------------------------------
(rr) call DumpJSStack()
0 cib(a = [object Object], b = "font-size:14.666666666666666px;font-family:Arial;color:#000000;background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;", c = "�", d = [object Object], e = [object HTMLSpanElement]) ["https://docs.google.com/static/document/client/js/1540849194-kix_main_i18n_kix_core__zh_tw.js":1696]
    this = [object Window]
1 fib(a = [object Object], b = [object Object], c = [object Object], d = true) ["https://docs.google.com/static/document/client/js/1540849194-kix_main_i18n_kix_core__zh_tw.js":1698]
    this = [object Window]
2 x.Eu(a = [object Object], b = "�", c = true) ["https://docs.google.com/static/document/client/js/1540849194-kix_main_i18n_kix_core__zh_tw.js":1697]
    this = [object Object]
3 Bzb(a = [object Object], b = [object Object], c = "�", d = true) ["https://docs.google.com/static/document/client/js/1540849194-kix_main_i18n_kix_core__zh_tw.js":2051]
    this = [object Window]
4 Azb(a = [object Object], b = [function], c = [object Object], d = "�", e = true) ["https://docs.google.com/static/document/client/js/1540849194-kix_main_i18n_kix_core__zh_tw.js":2051]
    this = [object Window]
5 zzb.prototype.Eu(a = [object Object], b = "�", c = true) ["https://docs.google.com/static/document/client/js/1540849194-kix_main_i18n_kix_core__zh_tw.js":2051]
    this = [object Object]
6 jWa(a = [object Object], b = [object Object]) ["https://docs.google.com/static/document/client/js/1540849194-kix_main_i18n_kix_core__zh_tw.js":1051]
    this = [object Window]
7 xWa(a = [object Object], b = [object Object], c = 1, d = 1, e = true, f = [object Object], h = true, k = [object Object], l = 0, n = false, p = 624) ["https://docs.google.com/static/document/client/js/1540849194-kix_main_i18n_kix_core__zh_tw.js":1063]
    this = [object Window]
8 rWa.prototype.Ua(a = 1, b = 1, c = 1, d = 624, e = 624, f = [object Object], h = null, k = "�", l = 0, n = 1, p = false, r = 0, u = 0, v = "
", y = false, C = true, E = false) ["https:/
> Can you tell me more about the "adoption" of node? All I can find is the DOM `document.adoptNode()` and `importNode()` API, is that what you are refering to?

Well, those APIs may do it, but presumably something simpler that may also trigger this (haven't actually tested it) is:

<!doctype html>
<span style="color: green">Foo</span>
<iframe src="something/where/stylo/is/not/supported"></iframe>
<script>
document.querySelector('iframe').contentWindow.getComputedStyle(document.querySelector('span'))
</script>
Flags: needinfo?(emilio+bugs)
Manish is fixing up the adopt case in bug 1330051.
Depends on: 1330051
P1 because this crash blocks Stylo from running Hasal's Google Docs performance test.
Keywords: crash
Priority: -- → P1
Confirmed that this crash fixed by commit for bug 1330051.
Assignee: nobody → manishearth
Status: NEW → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
status-firefox54: --- → unaffected
status-firefox55: --- → fixed
status-firefox-esr52: --- → unaffected
You need to log in before you can comment on or make changes to this bug.