Bug 154355 – Resolve style iteratively

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

Less recursion.

[Antti Koivisto]

Created attachment 271583 [details]
wip

[WebKit Commit Bot]

Attachment 271583 [details] did not pass style-queue:


ERROR: Source/WebCore/style/StyleTreeResolver.cpp:815:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/dom/ElementAndTextDescendantIterator.h:107:  Wrong number of spaces before statement. (expected: 4)  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/ElementAndTextDescendantIterator.h:119:  Wrong number of spaces before statement. (expected: 4)  [whitespace/indent] [4]
Total errors found: 3 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 271583 [details]
wip

Attachment 271583 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/846508

New failing tests:
imported/w3c/web-platform-tests/dom/nodes/Element-classlist.html

[Build Bot]

Created attachment 271585 [details]
Archive of layout-test-results from ews100 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Chris Dumez]

(In reply to comment #3)
> Comment on attachment 271583 [details]
> wip
> 
> Attachment 271583 [details] did not pass mac-ews (mac):
> Output: http://webkit-queues.webkit.org/results/846508
> 
> New failing tests:
> imported/w3c/web-platform-tests/dom/nodes/Element-classlist.html

ah ah, it looks like the w3c tests found a bug:
 PASS classList.add should not add a token if it already exists 
 PASS classList.remove removes arguments passed, if they are present. 
 PASS classList.remove must remove existing tokens 
-PASS classList.remove must not break case-sensitive CSS selector matching 
+FAIL classList.remove must not break case-sensitive CSS selector matching assert_not_equals: got disallowed value "italic"
 PASS classList.remove must remove duplicated tokens 
 PASS classList.remove must collapse whitespace around removed tokens 
 PASS classList.remove must collapse whitespaces around each token 
@@ -51,7 +51,7 @@
 PASS classList.toggle must not break case-sensitive CSS selector matching 
 PASS classList.toggle must be able to remove tokens 
 PASS classList.toggle must be case-sensitive when removing tokens 
-PASS CSS class selectors must stop matching when all classes have been removed 
+FAIL CSS class selectors must stop matching when all classes have been removed assert_not_equals: got disallowed value "italic"
 PASS className must be empty when all classes have been removed 
 PASS classList must stringify to an empty string when all classes have been removed 
 PASS classList.item(0) must return null when all classes have been removed

[Build Bot]

Comment on attachment 271583 [details]
wip

Attachment 271583 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/846519

New failing tests:
imported/w3c/web-platform-tests/dom/nodes/Element-classlist.html

[Build Bot]

Created attachment 271586 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 271583 [details]
wip

Attachment 271583 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/846511

New failing tests:
imported/w3c/web-platform-tests/dom/nodes/Element-classlist.html

[Build Bot]

Created attachment 271591 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Antti Koivisto]

Created attachment 271699 [details]
patch

[Antti Koivisto]

> ah ah, it looks like the w3c tests found a bug:

Yes, nice! Amazing that nothing else caught this.

[Simon Fraser (smfr)]

How does perf compare with the old code?

[Build Bot]

Comment on attachment 271699 [details]
patch

Attachment 271699 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/851821

New failing tests:
http/tests/media/video-redirect.html

[Build Bot]

Created attachment 271716 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Antti Koivisto]

(In reply to comment #12)
> How does perf compare with the old code?

Traversal should be faster due to reduced function call overhead and use of stateful iterator. I don't think it makes any real difference though. Actually resolving the style is many orders of magnitude more costly than iteration. Hopefully this will allow better optimizations later.

[Antti Koivisto]

Created attachment 271747 [details]
for bots

[Build Bot]

Comment on attachment 271747 [details]
for bots

Attachment 271747 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/854470

New failing tests:
http/tests/media/video-redirect.html

[Build Bot]

Created attachment 271749 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Antti Koivisto]

Created attachment 271752 [details]
for bots

[Build Bot]

Comment on attachment 271752 [details]
for bots

Attachment 271752 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/854817

New failing tests:
http/tests/media/video-redirect.html

[Build Bot]

Created attachment 271754 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Antti Koivisto]

Created attachment 271756 [details]
for bots

[Antti Koivisto]

Created attachment 271861 [details]
patch

[Andreas Kling]

Comment on attachment 271861 [details]
patch

r=me

[Antti Koivisto]

https://trac.webkit.org/r196864