RESOLVED FIXED Bug 202256
ScrollingTreeScrollingNode: use LayerRepresentation for scroll container, scrolled contents layers 
https://bugs.webkit.org/show_bug.cgi?id=202256
Summary ScrollingTreeScrollingNode: use LayerRepresentation for scroll container, scr...
Zan Dobersek
Reported 2019-09-25 22:21:12 PDT
ScrollingTreeScrollingNode: use LayerRepresentation for scroll container, scrolled contents layers
Attachments
Patch (3.45 KB, patch)
2019-09-25 22:24 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
WIP (5.22 KB, patch)
2019-09-26 05:42 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Patch (5.52 KB, patch)
2019-09-26 06:19 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Patch (5.69 KB, patch)
2019-09-29 10:42 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
WIP (8.93 KB, patch)
2019-09-29 11:24 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Patch (8.29 KB, patch)
2019-09-29 12:22 PDT, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Zan Dobersek
Comment 1 2019-09-25 22:24:16 PDT
Created attachment 379620 [details] Patch
Simon Fraser (smfr)
Comment 2 2019-09-26 04:05:51 PDT
I'll review when the tests are passing.
Zan Dobersek
Comment 3 2019-09-26 05:42:23 PDT
Created attachment 379635 [details] WIP
Zan Dobersek
Comment 4 2019-09-26 06:19:57 PDT
Created attachment 379636 [details] Patch
Zan Dobersek
Comment 5 2019-09-26 10:04:41 PDT
(In reply to Simon Fraser (smfr) from comment #2) > I'll review when the tests are passing. Builds fine now. The mac-debug-wk1 failure is not related.
Zan Dobersek
Comment 6 2019-09-26 23:19:54 PDT
Comment on attachment 379636 [details] Patch Clearing flags on attachment: 379636 Committed r250415: <https://trac.webkit.org/changeset/250415>
Zan Dobersek
Comment 7 2019-09-26 23:19:59 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2019-09-26 23:20:18 PDT
<rdar://problem/55772092>
Truitt Savell
Comment 9 2019-09-27 09:22:46 PDT
I looks like the change in https://trac.webkit.org/changeset/250415/webkit caused iOS debug testing to exit early with 50 crashes and 850 API failures. Build: https://build.webkit.org/builders/Apple%20iOS%2013%20Simulator%20Debug%20WK2%20%28Tests%29/builds/160 This will have to be rolled out.
Truitt Savell
Comment 10 2019-09-27 09:25:40 PDT
Reverted r250415 for reason: Broke iOS debug testing with 50 crashes and 850 API failure Committed r250427: <https://trac.webkit.org/changeset/250427>
Zan Dobersek
Comment 11 2019-09-29 02:15:39 PDT
With the switch to LayerRepresentation, an object of this type was used in a null check here: https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeFrameScrollingNodeRemoteIOS.mm#L69 Before the patch, this was testing nullness of a RetainPtr, which works fine. But with LayerRepresentation, the boolean evaluation ended up using the GraphicsLayer::PlatformLayerID operator to retrieve an integral value that could be tested in a boolean context. With the LayerRepresentation object containing a platform layer pointer, the asserts occurred. Bug #202344 has a patch that makes the LayerRepresentation operators explicit for safer but more verbose use, as well as adds a new operator that would be used in boolean contexts, avoiding the problem that caused the regression.
Zan Dobersek
Comment 12 2019-09-29 10:42:06 PDT
Created attachment 379813 [details] Patch Same patch up for re-review, with no problems expected now due to changes in bug #202344.
Zan Dobersek
Comment 13 2019-09-29 11:24:29 PDT
Created attachment 379816 [details] WIP Never mind, some additional iOS changes are required.
Zan Dobersek
Comment 14 2019-09-29 12:22:06 PDT
Created attachment 379817 [details] Patch Now ready.
Zan Dobersek
Comment 15 2019-09-29 22:15:42 PDT
Comment on attachment 379817 [details] Patch Clearing flags on attachment: 379817 Committed r250498: <https://trac.webkit.org/changeset/250498>
Zan Dobersek
Comment 16 2019-09-29 22:15:46 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.