Bug 154355

Summary:

Resolve style iteratively

Product:

WebKit

Reporter:

Antti Koivisto <koivisto>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, cdumez, commit-queue, rniwa, simon.fraser

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

154464

Bug Blocks:

Attachments:

Description	Flags
wip	buildbot: commit-queue-
Archive of layout-test-results from ews100 for mac-yosemite	none
Archive of layout-test-results from ews104 for mac-yosemite-wk2	none
Archive of layout-test-results from ews113 for mac-yosemite	none
patch	buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-yosemite	none
for bots	buildbot: commit-queue-
Archive of layout-test-results from ews113 for mac-yosemite	none
for bots	buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-yosemite	none
for bots	none
patch	kling: review+

Antti Koivisto

Reported 2016-02-17 13:11:42 PST

Less recursion.

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 attachment proposed patch, testcase, etc.

Antti Koivisto

Comment 1 2016-02-17 13:11:57 PST

Created attachment 271583 [details] wip

WebKit Commit Bot

Comment 2 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.

Build Bot

Comment 3 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

Build Bot

Comment 4 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

Chris Dumez

Comment 5 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

Build Bot

Comment 6 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

Build Bot

Comment 7 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

Build Bot

Comment 8 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

Build Bot

Comment 9 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

Antti Koivisto

Comment 10 2016-02-18 15:25:21 PST

Created attachment 271699 [details] patch

Antti Koivisto

Comment 11 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.

Simon Fraser (smfr)

Comment 12 2016-02-18 15:31:48 PST

How does perf compare with the old code?

Build Bot

Comment 13 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

Build Bot

Comment 14 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

Antti Koivisto

Comment 15 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.

Antti Koivisto

Comment 16 2016-02-19 05:09:44 PST

Created attachment 271747 [details] for bots

Build Bot

Comment 17 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

Build Bot

Comment 18 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

Antti Koivisto

Comment 19 2016-02-19 07:20:08 PST

Created attachment 271752 [details] for bots

Build Bot

Comment 20 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

Build Bot

Comment 21 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

Antti Koivisto

Comment 22 2016-02-19 08:52:41 PST

Created attachment 271756 [details] for bots

Antti Koivisto

Comment 23 2016-02-20 09:29:03 PST

Created attachment 271861 [details] patch

Andreas Kling

Comment 24 2016-02-20 10:25:01 PST

Comment on attachment 271861 [details] patch r=me

Antti Koivisto

Comment 25 2016-02-20 10:30:03 PST

https://trac.webkit.org/r196864

Note You need to log in before you can comment on or make changes to this bug.