Summary: | Scrolling state nodes should hold references to GraphicsLayers | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||
Component: | Scrolling | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cmarcelo, ews-watchlist, fred.wang, jamesr, koivisto, luiz, simon.fraser, thorton, tonikitoo, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2019-03-15 22:03:19 PDT
Created attachment 365105 [details]
Patch
Followup in https://trac.webkit.org/changeset/243129/webkit Comment on attachment 365105 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365105&action=review > Source/WebCore/ChangeLog:10 > + GraphicsLayers are refcounted, and the scrolling tree keeps GraphicsLayer pointers, > + so for safely the scrolling tree should store RefPtr<GraphicsLayer> instead. Why is the state tree referencing detached layers? While the change is a good idea in general, as a fix for this issue it appears to be just hiding the actual bug. We should still be asserting this is not happening. (In reply to Antti Koivisto from comment #5) > Comment on attachment 365105 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=365105&action=review > > > Source/WebCore/ChangeLog:10 > > + GraphicsLayers are refcounted, and the scrolling tree keeps GraphicsLayer pointers, > > + so for safely the scrolling tree should store RefPtr<GraphicsLayer> instead. > > Why is the state tree referencing detached layers? While the change is a > good idea in general, as a fix for this issue it appears to be just hiding > the actual bug. We should still be asserting this is not happening. It should never do so, because the scrolling tree is updated at the same time we do compositing updates. I wasn't able to reproduce the crash I saw earlier (possibly because r243120 fixed it). But if we have a bug, I'd prefer it not to turn into a UAF. |