u459114 Reporter
	Description • 7 years ago

When fixing bug 265894, I plan to fix this bug in both stylo-enable and enable mode.

There is a test case in that bug:
https://bug265894.bmoattachments.org/attachment.cgi?id=255916

There is a rule in the test page:
<style type="text/css">
  #gTest circle // a complex selector
  {
    stroke: red;
    stroke-width: 4px;
  }
</style>

Even stylo is enable, I still noticed that SelectorMatchesTree in nsCSSRuleProcessor.cpp been used for style matching, instead of matches_complex_selector_internal in stylo.

	u459114 Reporter
	Comment 1 • 7 years ago

1. ac_add_options --enable-stylo  in .mozconfig
2. visit https://bug265894.bmoattachments.org/attachment.cgi?id=255916
3. Set breakpoint inside SelectorMatchesTree
4. Then you can see "#gTest circle" is matched in SelectorMatchesTree.

	Cameron McCormack (:heycam)
	Comment 2 • 7 years ago

What's the stack trace at that point?

	u459114 Reporter
	Comment 3 • 7 years ago

#0  SelectorMatchesTree (aPrevElement=0x7fffb809eae0, aSelector=0x7fffd3e73240, aTreeMatchContext=..., aFlags=eLookForRelevantLink) at /home/cjcool/repository/mozilla-central-2/layout/style/nsCSSRuleProcessor.cpp:2427
#1  0x00007fffe5177d28 in ContentEnumFunc (value=..., aSelector=0x7fffd3e73290, data=0x7fffffff96f0, nodeContext=..., ancestorFilter=0x7fffffffb900)
    at /home/cjcool/repository/mozilla-central-2/layout/style/nsCSSRuleProcessor.cpp:2585
#2  0x00007fffe517316e in RuleHash::EnumerateAllRules (this=0x7fffc4185800, aElement=0x7fffb809eae0, aData=0x7fffffff96f0, aNodeContext=...) at /home/cjcool/repository/mozilla-central-2/layout/style/nsCSSRuleProcessor.cpp:696
#3  0x00007fffe5177e29 in nsCSSRuleProcessor::RulesMatching (this=0x7fffd34a3510, aData=0x7fffffff96f0) at /home/cjcool/repository/mozilla-central-2/layout/style/nsCSSRuleProcessor.cpp:2606
#4  0x00007fffe5256bc8 in EnumRulesMatching<ElementRuleProcessorData> (aProcessor=0x7fffd34a3510, aData=0x7fffffff96f0) at /home/cjcool/repository/mozilla-central-2/layout/style/nsStyleSet.cpp:802
#5  0x00007fffe523489f in nsStyleSet::FileRules (this=0x7fffc05f44f0, aCollectorFunc=0x7fffe5256b92 <EnumRulesMatching<ElementRuleProcessorData>(nsIStyleRuleProcessor*, void*)>, aData=0x7fffffff96f0, aElement=0x7fffb809eae0, 
    aRuleWalker=0x7fffffff96d0) at /home/cjcool/repository/mozilla-central-2/layout/style/nsStyleSet.cpp:1129
#6  0x00007fffe5235a8f in nsStyleSet::ResolveStyleForInternal (this=0x7fffc05f44f0, aElement=0x7fffb809eae0, aParentContext=0x7fffb6059ca0, aTreeMatchContext=..., aAnimationFlag=nsStyleSet::eWithAnimation)
    at /home/cjcool/repository/mozilla-central-2/layout/style/nsStyleSet.cpp:1362
#7  0x00007fffe5235c6c in nsStyleSet::ResolveStyleFor (this=0x7fffc05f44f0, aElement=0x7fffb809eae0, aParentContext=0x7fffb6059ca0, aTreeMatchContext=...) at /home/cjcool/repository/mozilla-central-2/layout/style/nsStyleSet.cpp:1400
#8  0x00007fffe5381b40 in nsStyleSet::ResolveStyleFor (this=0x7fffc05f44f0, aElement=0x7fffb809eae0, aParentContext=0x7fffb6059ca0, aMayCompute=mozilla::LazyComputeBehavior::Assert, aTreeMatchContext=...)
    at /home/cjcool/repository/mozilla-central-2/layout/style/nsStyleSet.h:139
#9  0x00007fffe5389924 in mozilla::StyleSetHandle::Ptr::ResolveStyleFor (this=0x7fffffff9870, aElement=0x7fffb809eae0, aParentContext=0x7fffb6059ca0, aMayCompute=mozilla::LazyComputeBehavior::Assert, 
    aTreeMatchContext=0x7fffffffb790) at /home/cjcool/repository/mozilla-central-2/obj-debug/dist/include/mozilla/StyleSetHandleInlines.h:96
#10 0x00007fffe5330c5f in nsCSSFrameConstructor::ResolveStyleContext (this=0x7fffa95a4220, aParentStyleContext=0x7fffb6059ca0, aContent=0x7fffb809eae0, aState=0x7fffffffb350, aOriginatingElementOrNull=0x0)
    at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:5125
#11 0x00007fffe5340088 in nsCSSFrameConstructor::AddFCItemsForAnonymousContent (this=0x7fffa95a4220, aState=..., aFrame=0x7fffb605a298, aAnonymousItems=nsTArray<nsIAnonymousContentCreator::ContentInfo> & = {...}, 
    aItemsToConstruct=..., aExtraFlags=0) at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:11069
#12 0x00007fffe53405c8 in nsCSSFrameConstructor::ProcessChildren (this=0x7fffa95a4220, aState=..., aContent=0x7fffac052f90, aStyleContext=0x7fffb6059ca0, aFrame=0x7fffb605a298, aCanHaveGeneratedContent=false, aFrameItems=..., 
    aAllowBlockStyles=false, aPendingBinding=0x0, aPossiblyLeafFrame=0x7fffb605a298) at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:11158
#13 0x00007fffe532ea65 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x7fffa95a4220, aItem=..., aState=..., aParentFrame=0x7fffb6059d50, aFrameItems=...)
    at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:4127
#14 0x00007fffe5334025 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x7fffa95a4220, aState=..., aIter=..., aParentFrame=0x7fffb6059d50, aFrameItems=...)
    at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:6299
#15 0x00007fffe538bd50 in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x7fffa95a4220, aState=..., aItems=..., aParentFrame=0x7fffb6059d50, aFrameItems=...)
    at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:10928
#16 0x00007fffe5340b09 in nsCSSFrameConstructor::ProcessChildren (this=0x7fffa95a4220, aState=..., aContent=0x7fffa95a4a90, aStyleContext=0x7fffb60599c8, aFrame=0x7fffb6059d50, aCanHaveGeneratedContent=false, aFrameItems=..., 
    aAllowBlockStyles=false, aPendingBinding=0x0, aPossiblyLeafFrame=0x7fffb6059d50) at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:11236
#17 0x00007fffe532ea65 in nsCSSFrameConstructor::ConstructFrameFromItemInternal (this=0x7fffa95a4220, aItem=..., aState=..., aParentFrame=0x7fffb6059a78, aFrameItems=...)
    at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:4127
#18 0x00007fffe5334025 in nsCSSFrameConstructor::ConstructFramesFromItem (this=0x7fffa95a4220, aState=..., aIter=..., aParentFrame=0x7fffb6059a78, aFrameItems=...)
    at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:6299
#19 0x00007fffe538bd50 in nsCSSFrameConstructor::ConstructFramesFromItemList (this=0x7fffa95a4220, aState=..., aItems=..., aParentFrame=0x7fffb6059a78, aFrameItems=...)
    at /home/cjcool/repository/mozilla-central-2/layout/base/nsCSSFrameConstructor.cpp:10928

	u459114 Reporter
	Comment 4 • 7 years ago

nsDocument::mType == Type::eGenericXML

	Jeremy Chen (chenpighead@gmail.com)
	Comment 5 • 7 years ago

(In reply to C.J. Ku[:cjku](UTC+8) from comment #4)
> nsDocument::mType == Type::eGenericXML

AFAICT, we haven't support stylo backend for eGenericXML yet.
http://searchfox.org/mozilla-central/rev/d441cb24482c2e5448accaf07379445059937080/dom/base/nsDocument.cpp#13175

	Cameron McCormack (:heycam)
	Comment 6 • 7 years ago

And I think it is a generic XML document because of the <meta http-equiv="Content-Type" value="application/xml">.

	Cameron McCormack (:heycam)
	Comment 7 • 7 years ago

I did a try push to enable stylo for generic XML document types (which is the last type apart from XUL documents where stylo still isn't enabled), and there are a few failures, including:

* a couple of XUL documents loaded as .xml files

* a few CSSWG reftests that happen to use .xml files

* a bunch of failures in test_value_computation.html, which uses .xml files in frames for its "unstyled" subtests

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a89f182319a0e0f9e07ac902032187c6ca50d64

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 8 • 7 years ago

(In reply to Cameron McCormack (:heycam) from comment #7)
> I did a try push to enable stylo for generic XML document types (which is
> the last type apart from XUL documents where stylo still isn't enabled), and
> there are a few failures, including:
> 
> * a couple of XUL documents loaded as .xml files
> 
> * a few CSSWG reftests that happen to use .xml files
> 
> * a bunch of failures in test_value_computation.html, which uses .xml files
> in frames for its "unstyled" subtests

We haven't enabled test_value_computation.html yet (that we expect there are failures, but we don't track the number of it). It seems the main problem here is that we somehow call SimpleTest.finish() multiple times, and failures appear after finish don't seem to be correctly collected by failure pattern mechanism.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 9 • 7 years ago

Just pushed a new try push, and it seems everything is pretty good now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a58e96993be71ddd22e9885bee1383e0bc4e63c1

There is only one new failure, layout/reftests/w3c-css/received/css-namespaces-3/syntax-013.xml (which causes R2, Rs3, and Wr2 to be orange). There is also a crashtest layout/xul/crashtests/140218-1.xml asserts 3 times for not supporting XUL, but that's totally fine.

I'll try this again after I land bug 1383992.

	Cameron McCormack (:heycam)
	Comment 11 • 7 years ago
mozreview-review

Comment on attachment 8895585 [details]
Bug 1370508 - Enable stylo on generic XML documents.

https://reviewboard.mozilla.org/r/166786/#review172052

::: layout/reftests/w3c-css/received/reftest.list:208
(Diff revision 1)
>  == css-namespaces-3/syntax-008.xml css-namespaces-3/reftest/ref-lime-2.xml
>  == css-namespaces-3/syntax-009.xml css-namespaces-3/reftest/ref-lime-1.xml
>  == css-namespaces-3/syntax-010.xml css-namespaces-3/reftest/ref-lime-3.xml
>  == css-namespaces-3/syntax-011.xml css-namespaces-3/reftest/ref-lime-6.xml
>  == css-namespaces-3/syntax-012.xml css-namespaces-3/reftest/ref-lime-3.xml
> -== css-namespaces-3/syntax-013.xml css-namespaces-3/reftest/ref-lime-5.xml
> +fails-if(stylo||styloVsGecko) == css-namespaces-3/syntax-013.xml css-namespaces-3/reftest/ref-lime-5.xml # bug 1388911

I think this change should be in w3c-css/failures.list instead.

	Pulsebot
	Comment 13 • 7 years ago

Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/78d87b59f5a7
Enable stylo on generic XML documents. r=heycam

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 14 • 7 years ago

Backed out for unexpected pass of wpt-reftest /css/css-namespaces-3/syntax-013.xml:

https://hg.mozilla.org/integration/autoland/rev/e5475f9a70ea7e6581ddb1eefdbbd4ac87c0f1dd

Push which ran "failing" tests: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f03833f24817e9cf748a89508eefc347033cb462&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=runnable
"Failure" log: https://treeherder.mozilla.org/logviewer.html#?job_id=122195971&repo=autoland

> TEST-UNEXPECTED-PASS | /css/css-namespaces-3/syntax-013.xml | Testing http://web-platform.test:8000/css/css-namespaces-3/syntax-013.xml == http://web-platform.test:8000/css/css-namespaces-3/reftest/ref-lime-5.xml

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 15 • 7 years ago

mochitest-e10s-9 looks good now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ccee5f3c743d0d2e3be510ff7ddfa93bdde64ef

	Pulsebot
	Comment 17 • 7 years ago

Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1f7c5273ef9b
Enable stylo on generic XML documents. r=heycam

	Carsten Book [:Tomcat]
	Comment 18 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/1f7c5273ef9b