Bug 163721 - Style resolver should be updated lazily
Summary: Style resolver should be updated lazily
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:
Blocks: 168655
  Show dependency treegraph
 
Reported: 2016-10-20 03:03 PDT by Antti Koivisto
Modified: 2017-02-21 09:19 PST (History)
5 users (show)

See Also:


Attachments
patch (37.77 KB, patch)
2016-10-20 03:23 PDT, Antti Koivisto
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (921.70 KB, application/zip)
2016-10-20 04:31 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (933.38 KB, application/zip)
2016-10-20 04:34 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-yosemite (1.81 MB, application/zip)
2016-10-20 04:41 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (9.49 MB, application/zip)
2016-10-20 04:45 PDT, Build Bot
no flags Details
patch (39.62 KB, patch)
2016-10-20 07:22 PDT, Antti Koivisto
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews113 for mac-yosemite (1.82 MB, application/zip)
2016-10-20 08:39 PDT, Build Bot
no flags Details
patch (41.24 KB, patch)
2016-10-20 14:13 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (41.23 KB, patch)
2016-10-20 14:14 PDT, Antti Koivisto
kling: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (326.17 KB, application/zip)
2016-10-20 15:32 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-yosemite (2.07 MB, application/zip)
2016-10-20 16:46 PDT, Build Bot
no flags Details
patch (44.40 KB, patch)
2016-10-21 02:56 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (45.08 KB, patch)
2016-10-21 03:38 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (45.05 KB, patch)
2016-10-21 04:32 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2016-10-20 03:03:59 PDT
Currently when stylesheets change in some way we generally update style resolvers and invalidate style immediately. We should do this lazily to avoid unnecessary work.
Comment 1 Antti Koivisto 2016-10-20 03:23:33 PDT
Created attachment 292163 [details]
patch
Comment 2 Build Bot 2016-10-20 04:31:12 PDT
Comment on attachment 292163 [details]
patch

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

New failing tests:
media/track/track-user-stylesheet.html
media/track/track-css-user-override.html
inspector/css/stylesheet-events-inspector-stylesheet.html
printing/page-format-data.html
Comment 3 Build Bot 2016-10-20 04:31:15 PDT
Created attachment 292167 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 4 Build Bot 2016-10-20 04:34:26 PDT
Comment on attachment 292163 [details]
patch

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

New failing tests:
media/track/track-user-stylesheet.html
media/track/track-css-user-override.html
inspector/css/stylesheet-events-inspector-stylesheet.html
printing/page-format-data.html
Comment 5 Build Bot 2016-10-20 04:34:28 PDT
Created attachment 292168 [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 6 Build Bot 2016-10-20 04:40:58 PDT
Comment on attachment 292163 [details]
patch

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

New failing tests:
media/track/track-css-user-override.html
printing/page-format-data.html
inspector/css/stylesheet-events-inspector-stylesheet.html
media/track/track-user-stylesheet.html
Comment 7 Build Bot 2016-10-20 04:41:00 PDT
Created attachment 292169 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 8 Build Bot 2016-10-20 04:45:12 PDT
Comment on attachment 292163 [details]
patch

Attachment 292163 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2330007

New failing tests:
media/track/track-user-stylesheet.html
media/track/track-css-user-override.html
Comment 9 Build Bot 2016-10-20 04:45:15 PDT
Created attachment 292170 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 10 Antti Koivisto 2016-10-20 07:22:03 PDT
Created attachment 292177 [details]
patch
Comment 11 Build Bot 2016-10-20 08:39:07 PDT
Comment on attachment 292177 [details]
patch

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

New failing tests:
fast/media/mq-relative-constraints-08.html
Comment 12 Build Bot 2016-10-20 08:39:10 PDT
Created attachment 292199 [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 13 Andreas Kling 2016-10-20 10:32:54 PDT
Comment on attachment 292177 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292177&action=review

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2360
> +    document.styleScope().flushPendingUpdate();

IIUC this is needed for hasPendingForcedStyleRecalc() to work correctly. Is that accurate? If so, do we also need to do this in Element::needsStyleInvalidation()?

> Source/WebCore/testing/Internals.cpp:385
> +    page.group().captionPreferences().setTestingMode(true);

This deserves a ChangeLog comment at least. :P
Comment 14 Antti Koivisto 2016-10-20 12:13:35 PDT
> IIUC this is needed for hasPendingForcedStyleRecalc() to work correctly. Is
> that accurate? If so, do we also need to do this in
> Element::needsStyleInvalidation()?

No because needsStyleInvalidation() is used to avoid invalidation work when we know we are already invalidated. Calling flushPendingUpdates() would catch more cases like that but it is way more expensive than per-element work that needsStyleInvalidation() is trying to optimize.

> 
> > Source/WebCore/testing/Internals.cpp:385
> > +    page.group().captionPreferences().setTestingMode(true);
> 
> This deserves a ChangeLog comment at least. :P

Yea. It is about avoiding inserting magical caption UA sheet that screws up some inspector tests. The existing code that was avoiding that was racy.
Comment 15 Antti Koivisto 2016-10-20 14:13:05 PDT
Created attachment 292260 [details]
patch
Comment 16 Antti Koivisto 2016-10-20 14:14:57 PDT
Created attachment 292262 [details]
patch
Comment 17 Andreas Kling 2016-10-20 14:57:54 PDT
Comment on attachment 292262 [details]
patch

r=me
Comment 18 Build Bot 2016-10-20 15:32:38 PDT
Comment on attachment 292262 [details]
patch

Attachment 292262 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2333649

Number of test failures exceeded the failure limit.
Comment 19 Build Bot 2016-10-20 15:32:41 PDT
Created attachment 292269 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 20 Build Bot 2016-10-20 16:46:24 PDT
Comment on attachment 292262 [details]
patch

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

New failing tests:
svg/text/text-outline-rgba.html
Comment 21 Build Bot 2016-10-20 16:46:27 PDT
Created attachment 292283 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 22 Antti Koivisto 2016-10-21 02:56:47 PDT
Created attachment 292335 [details]
patch
Comment 23 Antti Koivisto 2016-10-21 03:38:57 PDT
Created attachment 292338 [details]
patch
Comment 24 Antti Koivisto 2016-10-21 04:32:19 PDT
Created attachment 292343 [details]
patch
Comment 25 WebKit Commit Bot 2016-10-21 06:39:31 PDT
Comment on attachment 292343 [details]
patch

Clearing flags on attachment: 292343

Committed r207669: <http://trac.webkit.org/changeset/207669>
Comment 26 WebKit Commit Bot 2016-10-21 06:39:36 PDT
All reviewed patches have been landed.  Closing bug.