###!!! ASSERTION: stylo: cannot resolve style for canvas from a ServoStyleSet yet: 'Error', file /home/worker/workspace/build/src/dom/canvas/CanvasRenderingContext2D.cpp, line 2697 dom/canvas/crashtests/0px-size-font-667225.html dom/canvas/crashtests/0px-size-font-shadow.html dom/canvas/crashtests/1099143-1.html dom/canvas/crashtests/1190705.html dom/canvas/crashtests/1223740-1.html dom/canvas/crashtests/1225381-1.html dom/canvas/crashtests/1229932-1.html dom/canvas/crashtests/1283113-1.html dom/canvas/crashtests/1284356-1.html dom/canvas/crashtests/1287652-1.html dom/canvas/crashtests/1288872-1.html dom/canvas/crashtests/745699-1.html dom/canvas/crashtests/780392-1.html gfx/tests/crashtests/390476.html gfx/tests/crashtests/766452-1.html gfx/tests/crashtests/950000.html

This is fixed in the latest mozilla-central -> stylo merge.

This didn't get fixed after all (see bug 1334938); sorry for the noise.

From Hiro's analysis in bug 1351978, this is likely causing a lot of tests to fail. Bumping the priority.

Xidorn, what's your opinion on the best way to handle the canvas stuff in [1]. Should we do this over CSSOM, or should we just hard-code this stuff into Servo? [1] http://searchfox.org/mozilla-central/rev/fd99701cafa324669d950ced902d08a7450cd48b/dom/canvas/CanvasRenderingContext2D.cpp#2667

nsIStyleRule isn't actually part of CSSOM. It is a common interface for nsStyleSet to get declarations and settings. You can see the subclasses of this interface [1]. It seems there are two things that canvas wants style system to do: 1. parse a font shorthand, then compute to computed values and fill in an nsStyleFont for filling its internal state [2], and 2. parse a filter value with the given font information (for <length> values I suppose) and output an nsStyleFilter [3] I think rather than hacking around the interface here, it is probably easier to just provide some FFI functions from Servo side to fulfull the requirements above. [1] http://searchfox.org/mozilla-central/search?q=public+nsIStyleRule&case=false&regexp=false&path= [2] http://searchfox.org/mozilla-central/rev/990055a4902952e038cc350c9ffe1ac426d1c943/dom/canvas/CanvasRenderingContext2D.cpp#3647-3689 [3] http://searchfox.org/mozilla-central/rev/990055a4902952e038cc350c9ffe1ac426d1c943/dom/canvas/CanvasRenderingContext2D.cpp#2835-2851

(In reply to Xidorn Quan [:xidorn] UTC+10 (less responsive 15/Apr-3/May) from comment #5) > nsIStyleRule isn't actually part of CSSOM. It is a common interface for > nsStyleSet to get declarations and settings. You can see the subclasses of > this interface [1]. > > It seems there are two things that canvas wants style system to do: > 1. parse a font shorthand, then compute to computed values and fill in an > nsStyleFont for filling its internal state [2], and > 2. parse a filter value with the given font information (for <length> values > I suppose) and output an nsStyleFilter [3] > > I think rather than hacking around the interface here, it is probably easier > to just provide some FFI functions from Servo side to fulfull the > requirements above. It sounds like somewhat similar to what we did for script animations. For parsing a property, we can use Servo_ParseProperty[1]. For computing values with the parsed property, we can write a function similar to cascade_with_rules[2]. For getting nsStyleFont, we can use Servo_GetStyleFont. [1] https://hg.mozilla.org/mozilla-central/file/35c7be9c2db2/servo/ports/geckolib/glue.rs#l905 [2] https://hg.mozilla.org/mozilla-central/file/35c7be9c2db2/servo/components/style/matching.rs#l443 [3] https://hg.mozilla.org/mozilla-central/file/35c7be9c2db2/servo/components/style/gecko_bindings/bindings.rs#l1912

Ok - Hiro, can you take this one?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4249547e0a83ea654c9f879a2ee81c172eba7740 Just fixed font style handling not filter style handling yet.

Comment on attachment 8857794 [details] Bug 1324700 - Servo_ParseProperty() takes nsCSSPropertyID instead of nsACString. https://reviewboard.mozilla.org/r/129782/#review132406

Comment on attachment 8857795 [details] Bug 1324700 - Call PreTraverseSync() before calling ResolveStyleLazily() in ResolveTransientStyle(). https://reviewboard.mozilla.org/r/129784/#review132408

Comment on attachment 8857796 [details] Bug 1324700 - Add ResolveServoTransientStyle to get servo's computed values instead of nsStyleContext. https://reviewboard.mozilla.org/r/129786/#review132410 ::: layout/style/ServoStyleSet.h:164 (Diff revision 1) > dom::Element* aPseudoElement); > > // Resolves style for a (possibly-pseudo) Element without assuming that the > // style has been resolved, and without worrying about setting the style > // context up to live in the style context tree (a null parent is used). > + // |aPeusoTag| and |aPseudoType| must match. aPseudoTag

Comment on attachment 8857797 [details] Bug 1324700 - Add a function which is equivalent to CreateDeclaration() for servo. https://reviewboard.mozilla.org/r/129788/#review132412 ::: dom/canvas/CanvasRenderingContext2D.cpp:2772 (Diff revision 1) > + new URLExtraData(aDocument->GetDocumentURI(), > + aDocument->GetDocBaseURI(), Are these two URIs around the wrong way?

Comment on attachment 8857798 [details] Bug 1324700 - Add a function that checks PropertyDeclarationBlock has a CSSWideKeyword for a given property. https://reviewboard.mozilla.org/r/129790/#review132422 ::: servo/components/style/properties/declaration_block.rs:367 (Diff revision 1) > + self.declarations.iter().find(|&&(ref decl, _)| > + decl.id().is_or_is_longhand_of(property) && > + decl.get_css_wide_keyword().is_some() > + ).is_some() Instead of find, you could use any: self.declarations.iter().any(|&&(ref decl, _)| decl.id().is_or_is_longhand_of(property) && decl.get_css_wide_keyword().is_some() )

Comment on attachment 8857799 [details] Bug 1324700 - Add an FFI which returns computed values for a given declaration block with/without parent_style. https://reviewboard.mozilla.org/r/129792/#review132424

Comment on attachment 8857800 [details] Bug 1324700 - Add a function which is equivalent to GetFontStyleContext() for servo. https://reviewboard.mozilla.org/r/129794/#review132428

Comment on attachment 8857801 [details] Bug 1324700 - Resolve font property for servo. https://reviewboard.mozilla.org/r/129796/#review132430

Comment on attachment 8857802 [details] Bug 1324700 - Add a function which is equivalent to ResolveStyleForFilter for servo. https://reviewboard.mozilla.org/r/129798/#review132432

Comment on attachment 8857803 [details] Bug 1324700 - Resolve filter property for servo. https://reviewboard.mozilla.org/r/129800/#review132434

Comment on attachment 8857804 [details] Bug 1324700 - Drop warnings for stylo. https://reviewboard.mozilla.org/r/129802/#review132436

Comment on attachment 8857805 [details] Bug 1324700 - Update assertion counts which had been caused by font handling in canvas element. https://reviewboard.mozilla.org/r/129804/#review132438

Comment on attachment 8857806 [details] Bug 1324700 - Update reftest expectations. https://reviewboard.mozilla.org/r/129806/#review132440 Great!

A final try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3dd8c92bb80ec9bf4f1e588cd7dde88f11e4f9d1 PR: https://github.com/servo/servo/pull/16470

Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c98b26bb1063 Servo_ParseProperty() takes nsCSSPropertyID instead of nsACString. r=heycam https://hg.mozilla.org/integration/autoland/rev/73c495cad7d6 Call PreTraverseSync() before calling ResolveStyleLazily() in ResolveTransientStyle(). r=heycam https://hg.mozilla.org/integration/autoland/rev/b116710343a7 Add ResolveServoTransientStyle to get servo's computed values instead of nsStyleContext. r=heycam https://hg.mozilla.org/integration/autoland/rev/b38bae9c02fe Add a function which is equivalent to CreateDeclaration() for servo. r=heycam https://hg.mozilla.org/integration/autoland/rev/3e19c81c80fc Add a function that checks PropertyDeclarationBlock has a CSSWideKeyword for a given property. r=heycam https://hg.mozilla.org/integration/autoland/rev/3d2bbd149e04 Add an FFI which returns computed values for a given declaration block with/without parent_style. r=heycam https://hg.mozilla.org/integration/autoland/rev/1876d89c8f37 Add a function which is equivalent to GetFontStyleContext() for servo. r=heycam https://hg.mozilla.org/integration/autoland/rev/e959737822e5 Resolve font property for servo. r=heycam https://hg.mozilla.org/integration/autoland/rev/8dafc925b034 Add a function which is equivalent to ResolveStyleForFilter for servo. r=heycam https://hg.mozilla.org/integration/autoland/rev/6b13b4abe4a9 Resolve filter property for servo. r=heycam https://hg.mozilla.org/integration/autoland/rev/eb6ce1493b85 Drop warnings for stylo. r=heycam https://hg.mozilla.org/integration/autoland/rev/0ba623e978ef Update assertion counts which had been caused by font handling in canvas element. r=heycam https://hg.mozilla.org/integration/autoland/rev/ed88aa504601 Update reftest expectations. r=heycam

https://hg.mozilla.org/mozilla-central/rev/c98b26bb1063 https://hg.mozilla.org/mozilla-central/rev/73c495cad7d6 https://hg.mozilla.org/mozilla-central/rev/b116710343a7 https://hg.mozilla.org/mozilla-central/rev/b38bae9c02fe https://hg.mozilla.org/mozilla-central/rev/3e19c81c80fc https://hg.mozilla.org/mozilla-central/rev/3d2bbd149e04 https://hg.mozilla.org/mozilla-central/rev/1876d89c8f37 https://hg.mozilla.org/mozilla-central/rev/e959737822e5 https://hg.mozilla.org/mozilla-central/rev/8dafc925b034 https://hg.mozilla.org/mozilla-central/rev/6b13b4abe4a9 https://hg.mozilla.org/mozilla-central/rev/eb6ce1493b85 https://hg.mozilla.org/mozilla-central/rev/0ba623e978ef https://hg.mozilla.org/mozilla-central/rev/ed88aa504601