Summary: | [ContentChangeObserver] didFinishTransition triggers a nested style recalc via isConsideredClickable | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | zalan <zalan> | ||||||||
Component: | Layout and Rendering | Assignee: | zalan <zalan> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, koivisto, rniwa, simon.fraser, webkit-bug-importer, zalan | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
zalan
2019-07-04 16:19:44 PDT
Created attachment 373475 [details]
Patch
Created attachment 373476 [details]
Patch
Created attachment 373477 [details]
Patch
^^more reliable test case Another option would be to add a parameter to willRespondToMouseClickEvents that prevents style update. computeEditability() where the nested recalc is entered already takes such parameter. (In reply to Antti Koivisto from comment #5) > Another option would be to add a parameter to willRespondToMouseClickEvents > that prevents style update. computeEditability() where the nested recalc is > entered already takes such parameter. There's another isConsideredClickable() callsite (which I can't make async) and I was planning to do exactly that in there. Comment on attachment 373477 [details] Patch Clearing flags on attachment: 373477 Committed r247147: <https://trac.webkit.org/changeset/247147> All reviewed patches have been landed. Closing bug. (In reply to zalan from comment #6) > (In reply to Antti Koivisto from comment #5) > > Another option would be to add a parameter to willRespondToMouseClickEvents > > that prevents style update. computeEditability() where the nested recalc is > > entered already takes such parameter. > There's another isConsideredClickable() callsite (which I can't make async) > and I was planning to do exactly that in there. Is that guaranteed to be correct? computeEditability needs the up-to-date style. In cases where computeEditability is called and we don't end up updating the style due to this flag, is the style computeEditability uses guaranteed to be up-to-date? Presumably the heuristics would trigger when the style actually changes, if needed. Actually the other callsite is at RenderTreeUpdater::updateElementRenderer() (ContentChangeObserver::StyleChangeScope -> ~StyleChangeScope). I assume by the time we get here all the styles are resolved and it's safe to call computeEditability. Antti? (In reply to zalan from comment #12) > Actually the other callsite is at RenderTreeUpdater::updateElementRenderer() > (ContentChangeObserver::StyleChangeScope -> ~StyleChangeScope). I assume by > the time we get here all the styles are resolved and it's safe to call > computeEditability. Antti? Why can't the code in StyleChangeScope::~StyleChangeScope() be pushed as a post style update task?? (In reply to zalan from comment #12) > Actually the other callsite is at RenderTreeUpdater::updateElementRenderer() > (ContentChangeObserver::StyleChangeScope -> ~StyleChangeScope). I assume by > the time we get here all the styles are resolved and it's safe to call > computeEditability. Antti? Yes, it should be safe. Style has just been updated by updateElementRenderer when ~StyleChangeScope runs. Cutting close though. |