Bug 222187

Summary: REGRESSION(iOS 14): Author shadow DOM invalidated unnecessarily on pseudo element change
Product: WebKit Reporter: Liam DeBeasi <ldebeasi>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, dino, emilio, esprehn+autocc, ews-watchlist, gyuyoung.kim, jonesr, koivisto, marc, mifenton, rniwa, sam, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 14   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 225187    
Bug Blocks: 148695    
Attachments:
Description Flags
Code Reproduction
none
wip
ews-feeder: commit-queue-
patch
ews-feeder: commit-queue-
patch none

Liam DeBeasi
Reported 2021-02-19 12:14:24 PST
Created attachment 421007 [details] Code Reproduction When using Web Components inside of the Shadow DOM that have CSS Variables set on the host, there is a significant delay in response to clicks/touches. It seems to be related to the number of instances of the component as well as the number of CSS Variables set on the host. I am only able to reproduce this on iOS 14.0+. This performance issue does not appear on earlier versions of iOS. I can also reproduce this on Safari 14 on macOS, but it is a bit easier to reproduce on iOS. Steps to reproduce: 1. Open code reproduction on an iOS device running iOS 14. 2. Tap the "Click Me" checkbox label. Notice that there is a delay between when the tap highlight appears and when the checkbox becomes checked. Repeat this process on a device running iOS 13 and you should notice that this delay does not occur. Additional Information: - I can reproduce this on STP 120 as well as iOS 14.5. - I cannot reproduce this in Chrome 88. - I took a Timeline sample and it appears that WebKit is spending significant amounts of time doing Style Recalculations whenever you try to check the checkbox.
Attachments
Code Reproduction (2.78 KB, text/html)
2021-02-19 12:14 PST, Liam DeBeasi 		no flags
Details
wip (6.41 KB, patch)
2021-05-04 05:58 PDT, Antti Koivisto 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
patch (13.02 KB, patch)
2021-05-04 10:11 PDT, Antti Koivisto 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
patch (13.25 KB, patch)
2021-05-04 11:09 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
marc
Comment 1 2021-02-22 02:34:56 PST
I can confirm that I can reproduce this too, it is causing a lot of headaches for us and our product becomes unusable because of it. Please can this be addressed ASAP? Thanks,
Ryosuke Niwa
Comment 2 2021-02-22 14:18:01 PST
Could you clarify the impact of this performance problem? Is it affecting a top website in UK? Is it affecting some major business operation? Any information regarding how much of a wide impact this bug has will help us prioritize it.
marc
Comment 3 2021-02-23 01:02:22 PST
We have 5 iOS apps that this is affecting. We've only had a few complaints so far, but as more people update their devices this issue is going to become a bit of a show stopper for us.
Ryosuke Niwa
Comment 4 2021-02-23 11:36:45 PST
(In reply to marc from comment #3) > We have 5 iOS apps that this is affecting. We've only had a few complaints > so far, but as more people update their devices this issue is going to > become a bit of a show stopper for us. Ah, didn't notice this was a iOS 14 regression. Would you be able to share which iOS apps they're?
Radar WebKit Bug Importer
Comment 6 2021-02-26 12:15:13 PST
<rdar://problem/74801314>
Antti Koivisto
Comment 7 2021-03-05 00:57:41 PST
The test case has lots of custom element instances (some 6000) that each have a :host rule with a bunch of variable declarations. For some (immediately non-obvious) reason the button click causes a full style recalc on all shadow trees. That takes a while due to large number of style scopes as well as our suboptimal variables implementation.
Antti Koivisto
Comment 8 2021-03-05 00:58:29 PST
If there is a regression here it is probably in style invalidation as a result of the checkbox click.
marc
Comment 9 2021-03-05 01:12:49 PST
Personally, I'm not sure whether this is limited to click events. For the reason that the more and more elements you add, the slower scrolling and re-draws become. If you do a hard scroll down the page, all you see is the background colour and then the components appear after some time. This, as well as the click event issue, becomes worse as you increase the number of items on the page. Unless the click event is somehow causing that too. Thanks,
Antti Koivisto
Comment 10 2021-03-05 01:57:36 PST
Yes, anything that triggers a style recalc is going to be slow with this content.
Antti Koivisto
Comment 11 2021-03-05 01:59:44 PST
We really should implement shared style resolver for shadow trees with identical style.
marc
Comment 12 2021-03-15 02:12:44 PDT
Hi all, I was wondering if there has been any movement on this issue? We are getting more and more reports of it as clients update to 14. Thanks,
Antti Koivisto
Comment 13 2021-05-04 05:58:11 PDT
Created attachment 427654 [details] wip
Antti Koivisto
Comment 14 2021-05-04 06:05:48 PDT
The issue is that hovering the element causes us to invalidate the entire shadow tree style. Since everything here is in a shadow tree that's a big invalidation. The invalidation is triggered by the UA stylesheet having some :hover rules targeting pseudo elements in UA shadow tree. This should not require invalidation except for those specific elements.
Antti Koivisto
Comment 15 2021-05-04 10:11:41 PDT
Created attachment 427681 [details] patch
Antti Koivisto
Comment 16 2021-05-04 11:09:20 PDT
Created attachment 427688 [details] patch
Ryosuke Niwa
Comment 17 2021-05-04 13:59:17 PDT
Comment on attachment 427688 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=427688&action=review > LayoutTests/fast/shadow-dom/shadow-style-invalidation-pseudo-element.html:1 > +<script src="../../resources/testharness.js"></script> Missing DOCTYPE?
EWS
Comment 18 2021-05-04 21:38:26 PDT
Committed r277001 (237321@main): <https://commits.webkit.org/237321@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427688 [details].
Liam DeBeasi
Comment 19 2021-05-26 19:27:03 PDT
I am testing this in STP 125 and it is better, but I can still reproduce the issue in other ways. If you try out my code reproduction and click the "Click Me" label (not the checkbox) multiple times rapidly you should see a slowdown again. Should I open another issue?
Antti Koivisto
Comment 20 2021-05-26 23:03:40 PDT
Please file a new bug, especially if it is worse than other browsers. The content here is inherently heavy though (some 6000 shadow trees with dozens of variables each).
marc
Comment 21 2021-05-27 01:10:55 PDT
@Antti, I would disagree that Liam should be raise a new bug, as the reported issue still exists, it's just slightly better than it was. So I would say that this issue is only partially fixed. We've already waited over 2 months for someone to look into this, are we going to have to wait another 2 months if another bug is raised?
Liam DeBeasi
Comment 22 2021-05-27 06:56:41 PDT
I filed a new bug: https://bugs.webkit.org/show_bug.cgi?id=226330 Code example is the same, but steps to reproduce are slightly different. Let me know if you have questions. I appreciate all the work you have put into this so far!
Note You need to log in before you can comment on or make changes to this bug.