Bug 193754 - Allow scrolling tree nodes to exist in a detached state
Summary: Allow scrolling tree nodes to exist in a detached state
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-23 18:44 PST by Simon Fraser (smfr)
Modified: 2019-01-29 08:17 PST (History)
8 users (show)

See Also:


Attachments
Patch (32.24 KB, patch)
2019-01-26 13:08 PST, Simon Fraser (smfr)
zalan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2019-01-23 18:44:09 PST
To avoid tricky ordering dependencies between compositing/scrolling tree updates in child and parent frames, we need to make it possible to create ScrollingStateNodes in a detached state (basically calling ScrollingCoordinator::attachToStateTree() with no parent node). This will allow subframes to have a detached subtree of scrolling nodes, then attach this subtree when the iframe layers are hooked up by RenderLayerCompositor.
Comment 1 Simon Fraser (smfr) 2019-01-23 18:54:23 PST
This is really a consequence of scrolling tree nodes being hidden behind the ScrollingCoordinator interface; a client can't hang on to a scrolling tree node; all it has to hang onto is a ScrollingNodeID.
Comment 2 Simon Fraser (smfr) 2019-01-26 13:08:08 PST
Created attachment 360248 [details]
Patch
Comment 3 Radar WebKit Bug Importer 2019-01-26 17:53:55 PST
<rdar://problem/47577565>
Comment 4 Simon Fraser (smfr) 2019-01-26 20:32:44 PST
https://trac.webkit.org/changeset/240551/webkit
Comment 5 Shawn Roberts 2019-01-28 13:28:55 PST
It appears that the changes in https://trac.webkit.org/changeset/240551/webkit

Has caused this test css3/filters/blur-filter-page-scroll-self.html

To become a flaky crash on Mac Debug WK2

History https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=css3%2Ffilters%2Fblur-filter-page-scroll-self.html

I reproduce this with run-webkit-tests --root debug-240551 css3/filters/blur-filter-page-scroll-self.html --iterations 500 -f --debug

I was able to reproduce crashes on 240551, but unable to reproduce crashes on 240550
Comment 6 Frédéric Wang (:fredw) 2019-01-29 01:09:20 PST
(In reply to Shawn Roberts from comment #5)
> It appears that the changes in
> https://trac.webkit.org/changeset/240551/webkit
> 
> Has caused this test css3/filters/blur-filter-page-scroll-self.html
> 
> To become a flaky crash on Mac Debug WK2
> 
> History
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=css3%2Ffilters%2Fblur-filter-page-scroll-self.
> html
> 
> I reproduce this with run-webkit-tests --root debug-240551
> css3/filters/blur-filter-page-scroll-self.html --iterations 500 -f --debug
> 
> I was able to reproduce crashes on 240551, but unable to reproduce crashes
> on 240550

That seems to be bug 193925, do you still see it after r240609?
Comment 7 Frédéric Wang (:fredw) 2019-01-29 02:53:31 PST
Comment on attachment 360248 [details]
Patch

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

> Source/WebCore/page/scrolling/ScrollingStateTree.cpp:222
> +        for (auto child : *children) {

This was modified later r240610 (bug 193930 but I don't have access to it). And the children variable is no longer used.
Comment 8 Frédéric Wang (:fredw) 2019-01-29 03:01:01 PST
Committed r240651: <https://trac.webkit.org/changeset/240651>
Comment 9 Shawn Roberts 2019-01-29 08:17:06 PST
(In reply to Frédéric Wang (:fredw) from comment #8)
> Committed r240651: <https://trac.webkit.org/changeset/240651>

It appears it has been resolved. Thank you!