Bug 154355 - Resolve style iteratively
Summary: Resolve style iteratively
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 154464
Blocks:
  Show dependency tree / graph
 
Reported: 2016-02-17 13:11 PST by Antti Koivisto
Modified: 2016-02-20 10:30 PST (History)
5 users (show)

See Also:

Attachments
wip (22.06 KB, patch)
2016-02-17 13:11 PST, Antti Koivisto 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-yosemite (713.22 KB, application/zip)
2016-02-17 13:58 PST, Build Bot 		no flags Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (749.71 KB, application/zip)
2016-02-17 14:04 PST, Build Bot 		no flags Details
Archive of layout-test-results from ews113 for mac-yosemite (770.09 KB, application/zip)
2016-02-17 14:20 PST, Build Bot 		no flags Details
patch (27.48 KB, patch)
2016-02-18 15:25 PST, Antti Koivisto 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-yosemite (884.07 KB, application/zip)
2016-02-18 16:35 PST, Build Bot 		no flags Details
for bots (29.15 KB, patch)
2016-02-19 05:09 PST, Antti Koivisto 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews113 for mac-yosemite (938.56 KB, application/zip)
2016-02-19 06:16 PST, Build Bot 		no flags Details
for bots (30.13 KB, patch)
2016-02-19 07:20 PST, Antti Koivisto 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-yosemite (2.00 MB, application/zip)
2016-02-19 08:18 PST, Build Bot 		no flags Details
for bots (31.01 KB, patch)
2016-02-19 08:52 PST, Antti Koivisto 		no flags Details | Formatted Diff | Diff
patch (27.69 KB, patch)
2016-02-20 09:29 PST, Antti Koivisto 		kling: review+
Details | Formatted Diff | Diff
Show Obsolete (11) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2016-02-17 13:11:42 PST 
Less recursion.
Comment 1 Antti Koivisto 2016-02-17 13:11:57 PST 
Created attachment 271583 [details]
wip
Comment 2 WebKit Commit Bot 2016-02-17 13:13:13 PST 
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.
Comment 3 Build Bot 2016-02-17 13:58:35 PST 
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
Comment 4 Build Bot 2016-02-17 13:58:38 PST 
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
Comment 5 Chris Dumez 2016-02-17 14:01:22 PST 
(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
Comment 6 Build Bot 2016-02-17 14:04:36 PST 
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
Comment 7 Build Bot 2016-02-17 14:04:39 PST 
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
Comment 8 Build Bot 2016-02-17 14:20:46 PST 
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
Comment 9 Build Bot 2016-02-17 14:20:49 PST 
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
Comment 10 Antti Koivisto 2016-02-18 15:25:21 PST 
Created attachment 271699 [details]
patch
Comment 11 Antti Koivisto 2016-02-18 15:29:49 PST 
> ah ah, it looks like the w3c tests found a bug:

Yes, nice! Amazing that nothing else caught this.
Comment 12 Simon Fraser (smfr) 2016-02-18 15:31:48 PST 
How does perf compare with the old code?
Comment 13 Build Bot 2016-02-18 16:35:11 PST 
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
Comment 14 Build Bot 2016-02-18 16:35:18 PST 
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
Comment 15 Antti Koivisto 2016-02-18 18:00:13 PST 
(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.
Comment 16 Antti Koivisto 2016-02-19 05:09:44 PST 
Created attachment 271747 [details]
for bots
Comment 17 Build Bot 2016-02-19 06:16:53 PST 
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
Comment 18 Build Bot 2016-02-19 06:16:57 PST 
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
Comment 19 Antti Koivisto 2016-02-19 07:20:08 PST 
Created attachment 271752 [details]
for bots
Comment 20 Build Bot 2016-02-19 08:18:48 PST 
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
Comment 21 Build Bot 2016-02-19 08:18:52 PST 
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
Comment 22 Antti Koivisto 2016-02-19 08:52:41 PST 
Created attachment 271756 [details]
for bots
Comment 23 Antti Koivisto 2016-02-20 09:29:03 PST 
Created attachment 271861 [details]
patch
Comment 24 Andreas Kling 2016-02-20 10:25:01 PST 
Comment on attachment 271861 [details]
patch

r=me
Comment 25 Antti Koivisto 2016-02-20 10:30:03 PST 
https://trac.webkit.org/r196864