216192 2020-09-04 14:36:52 -0700 REGRESSION (r260571): Scrolling on weather.com in Safari causes the gradient background to flicker (fixed backgrounds) 2020-09-05 10:56:24 -0700 1 1 1 Unclassified WebKit Scrolling WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 simon.fraser simon.fraser cmarcelo darin ews-watchlist fred.wang jamesr luiz simon.fraser thorton tonikitoo webkit-bug-importer oldest_to_newest 1685855 0 simon.fraser 2020-09-04 14:36:52 -0700 REGRESSION (r260571): Scrolling on weather.com in Safari causes the gradient background to flicker (fixed backgrounds) 1685857 1 408025 simon.fraser 2020-09-04 14:39:35 -0700 Created attachment 408025 Patch 1685858 2 simon.fraser 2020-09-04 14:39:37 -0700 <rdar://problem/68192010> 1685929 3 408025 darin 2020-09-04 17:01:50 -0700 Comment on attachment 408025 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408025&action=review > Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp:124 > + if (auto* rootNode = this->rootNode()) > + return !rootNode->hasSynchronousScrollingReasons(); > + > + return true; Here’s how I would write this: auto rootNode = this->rootNode(); return !rootNode || !rootNode->hasSynchronousScrollingReasons(); I actually find this easier to reason about than the return-based one. Not sure others agree. Another option would be: auto rootNode = this->rootNode(); return rootNode && rootNode->hasSynchronousScrollingReasons(); A plus for using auto rather than auto* is that we can switch to returning RefPtr from rootNode if we want to for object lifetime safety without modifying this call site. 1685976 4 simon.fraser 2020-09-04 18:49:11 -0700 (In reply to Darin Adler from comment #3) > Comment on attachment 408025 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=408025&action=review > > > Source/WebCore/page/scrolling/ThreadedScrollingTree.cpp:124 > > + if (auto* rootNode = this->rootNode()) > > + return !rootNode->hasSynchronousScrollingReasons(); > > + > > + return true; > > Here’s how I would write this: > > auto rootNode = this->rootNode(); > return !rootNode || !rootNode->hasSynchronousScrollingReasons(); > > I actually find this easier to reason about than the return-based one. Not > sure others agree. Another option would be: > > auto rootNode = this->rootNode(); > return rootNode && rootNode->hasSynchronousScrollingReasons(); I think you mean !(rootNode && rootNode->hasSynchronousScrollingReasons()); I tried both previous versions as I was writing the patch. > A plus for using auto rather than auto* is that we can switch to returning > RefPtr from rootNode if we want to for object lifetime safety without > modifying this call site. I'm not a fan of 'auto' hiding the fact that it's a pointer. 1686091 5 simon.fraser 2020-09-05 10:56:24 -0700 https://trac.webkit.org/changeset/266664/webkit 408025 2020-09-04 14:39:35 -0700 2020-09-04 14:43:07 -0700 Patch bug-216192-20200904143934.patch text/plain 4241 simon.fraser U3VidmVyc2lvbiBSZXZpc2lvbjogMjY2MzM3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMWQyNjEzMmFjYWNmNGUz NjhjNGEzNGQ3MWRiNzI3MjM4ZjI1YmFjOS4uMWEyNGNiZjZkMDQxMTQzMzA4YzkyMmRkOWFiZjRm ZjJhYTAxMmVlMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDIwLTA5LTA0ICBTaW1v biBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUuY29tPgorCisgICAgICAgIFJFR1JFU1NJT04g KHIyNjA1NzEpOiBTY3JvbGxpbmcgb24gd2VhdGhlci5jb20gaW4gU2FmYXJpIGNhdXNlcyB0aGUg Z3JhZGllbnQgYmFja2dyb3VuZCB0byBmbGlja2VyIChmaXhlZCBiYWNrZ3JvdW5kcykKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIxNjE5MgorICAgICAg ICA8cmRhcjovL3Byb2JsZW0vNjgxOTIwMTA+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgSWYgYSBwYWdlIGhhcyBzbG93LXNjcm9sbGluZyByZWFzb25z LCBsaWtlIGJhY2tncm91bmQtYXR0YWNobWVudDpmaXhlZCBvbiBhIG5vbi1yb290IGVsZW1lbnQs CisgICAgICAgIHRoZW4gd2Ugc2hvdWxkIG5ldmVyIHVwZGF0ZSBsYXllciBwb3NpdGlvbnMgb24g dGhlIHNjcm9sbGluZyB0aHJlYWQsIHNpbmNlIHRoaXMgcmVzdWx0cworICAgICAgICBpbiBzY3Jv bGwgcG9zaXRpb24gYmVpbmcgb3V0IG9mIHN5bmMgd2l0aCB0aGUgcGFpbnRlZCBiYWNrZ3JvdW5k IHBvc2l0aW9uLgorCisgICAgICAgICogcGFnZS9zY3JvbGxpbmcvVGhyZWFkZWRTY3JvbGxpbmdU cmVlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlRocmVhZGVkU2Nyb2xsaW5nVHJlZTo6Y2FuVXBk YXRlTGF5ZXJzT25TY3JvbGxpbmdUaHJlYWQgY29uc3QpOgorICAgICAgICAoV2ViQ29yZTo6VGhy ZWFkZWRTY3JvbGxpbmdUcmVlOjp3YWl0Rm9yUmVuZGVyaW5nVXBkYXRlQ29tcGxldGlvbk9yVGlt ZW91dCk6CisgICAgICAgIChXZWJDb3JlOjpUaHJlYWRlZFNjcm9sbGluZ1RyZWU6OmRlbGF5ZWRS ZW5kZXJpbmdVcGRhdGVEZXRlY3Rpb25UaW1lckZpcmVkKToKKyAgICAgICAgKFdlYkNvcmU6OlRo cmVhZGVkU2Nyb2xsaW5nVHJlZTo6ZGlzcGxheURpZFJlZnJlc2hPblNjcm9sbGluZ1RocmVhZCk6 CisgICAgICAgICogcGFnZS9zY3JvbGxpbmcvVGhyZWFkZWRTY3JvbGxpbmdUcmVlLmg6CisKIDIw MjAtMDgtMzAgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBhcHBsZS5jb20+CiAKICAgICAg ICAgUnViYmVyYmFuZGluZyBpbiBhbiBvdmVyZmxvdzpzY3JvbGwgY2FuIHRyaWdnZXIgcGFnZSBq aWdnbGVzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wYWdlL3Njcm9sbGluZy9UaHJlYWRl ZFNjcm9sbGluZ1RyZWUuY3BwIGIvU291cmNlL1dlYkNvcmUvcGFnZS9zY3JvbGxpbmcvVGhyZWFk ZWRTY3JvbGxpbmdUcmVlLmNwcAppbmRleCA2NmVkMWY3MzZjMjdlODc2ZGFhNmY1MjRhODdjMGJl YTBlMDFhZTBjLi42ZTZiYTViNjg0MThlZWI5ZWI0Y2IxM2I0MzY5ZGE0YTc0Mzc3MDNlIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wYWdlL3Njcm9sbGluZy9UaHJlYWRlZFNjcm9sbGluZ1Ry ZWUuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BhZ2Uvc2Nyb2xsaW5nL1RocmVhZGVkU2Nyb2xs aW5nVHJlZS5jcHAKQEAgLTExNiw2ICsxMTYsMTQgQEAgdm9pZCBUaHJlYWRlZFNjcm9sbGluZ1Ry ZWU6OnByb3BhZ2F0ZVN5bmNocm9ub3VzU2Nyb2xsaW5nUmVhc29ucyhjb25zdCBIYXNoU2V0PFMK ICAgICB9CiB9CiAKK2Jvb2wgVGhyZWFkZWRTY3JvbGxpbmdUcmVlOjpjYW5VcGRhdGVMYXllcnNP blNjcm9sbGluZ1RocmVhZCgpIGNvbnN0Cit7CisgICAgaWYgKGF1dG8qIHJvb3ROb2RlID0gdGhp cy0+cm9vdE5vZGUoKSkKKyAgICAgICAgcmV0dXJuICFyb290Tm9kZS0+aGFzU3luY2hyb25vdXNT Y3JvbGxpbmdSZWFzb25zKCk7CisKKyAgICByZXR1cm4gdHJ1ZTsKK30KKwogdm9pZCBUaHJlYWRl ZFNjcm9sbGluZ1RyZWU6OnNjcm9sbGluZ1RyZWVOb2RlRGlkU2Nyb2xsKFNjcm9sbGluZ1RyZWVT Y3JvbGxpbmdOb2RlJiBub2RlLCBTY3JvbGxpbmdMYXllclBvc2l0aW9uQWN0aW9uIHNjcm9sbGlu Z0xheWVyUG9zaXRpb25BY3Rpb24pCiB7CiAgICAgaWYgKCFtX3Njcm9sbGluZ0Nvb3JkaW5hdG9y KQpAQCAtMjU4LDcgKzI2Niw4IEBAIHZvaWQgVGhyZWFkZWRTY3JvbGxpbmdUcmVlOjp3YWl0Rm9y UmVuZGVyaW5nVXBkYXRlQ29tcGxldGlvbk9yVGltZW91dCgpCiAgICAgICAgIG1fc3RhdGUgPSBT eW5jaHJvbml6YXRpb25TdGF0ZTo6RGVzeW5jaHJvbml6ZWQ7CiAgICAgICAgIC8vIEF0IHRoaXMg cG9pbnQgd2Uga25vdyB0aGUgbWFpbiB0aHJlYWQgaXMgdGFraW5nIHRvbyBsb25nIGluIHRoZSBy ZW5kZXJpbmcgdXBkYXRlLAogICAgICAgICAvLyBzbyB3ZSBnaXZlIHVwIHRyeWluZyB0byBzeW5j IHdpdGggdGhlIG1haW4gdGhyZWFkIGFuZCB1cGRhdGUgbGF5ZXJzIGhlcmUgb24gdGhlIHNjcm9s bGluZyB0aHJlYWQuCi0gICAgICAgIGFwcGx5TGF5ZXJQb3NpdGlvbnNJbnRlcm5hbCgpOworICAg ICAgICBpZiAoY2FuVXBkYXRlTGF5ZXJzT25TY3JvbGxpbmdUaHJlYWQoKSkKKyAgICAgICAgICAg IGFwcGx5TGF5ZXJQb3NpdGlvbnNJbnRlcm5hbCgpOwogICAgICAgICB0cmFjZVBvaW50KFNjcm9s bGluZ1RocmVhZFJlbmRlclVwZGF0ZVN5bmNFbmQsIDEpOwogICAgIH0gZWxzZQogICAgICAgICB0 cmFjZVBvaW50KFNjcm9sbGluZ1RocmVhZFJlbmRlclVwZGF0ZVN5bmNFbmQpOwpAQCAtMjkxLDcg KzMwMCw4IEBAIHZvaWQgVGhyZWFkZWRTY3JvbGxpbmdUcmVlOjpkZWxheWVkUmVuZGVyaW5nVXBk YXRlRGV0ZWN0aW9uVGltZXJGaXJlZCgpCiAgICAgQVNTRVJUKFNjcm9sbGluZ1RocmVhZDo6aXND dXJyZW50VGhyZWFkKCkpOwogCiAgICAgTG9ja0hvbGRlciB0cmVlTG9ja2VyKG1fdHJlZU11dGV4 KTsKLSAgICBhcHBseUxheWVyUG9zaXRpb25zSW50ZXJuYWwoKTsKKyAgICBpZiAoY2FuVXBkYXRl TGF5ZXJzT25TY3JvbGxpbmdUaHJlYWQoKSkKKyAgICAgICAgYXBwbHlMYXllclBvc2l0aW9uc0lu dGVybmFsKCk7CiAgICAgbV9zdGF0ZSA9IFN5bmNocm9uaXphdGlvblN0YXRlOjpEZXN5bmNocm9u aXplZDsKIH0KIApAQCAtMzAyLDcgKzMxMiw3IEBAIHZvaWQgVGhyZWFkZWRTY3JvbGxpbmdUcmVl OjpkaXNwbGF5RGlkUmVmcmVzaE9uU2Nyb2xsaW5nVGhyZWFkKCkKIAogICAgIExvY2tIb2xkZXIg dHJlZUxvY2tlcihtX3RyZWVNdXRleCk7CiAKLSAgICBpZiAobV9zdGF0ZSAhPSBTeW5jaHJvbml6 YXRpb25TdGF0ZTo6SWRsZSkKKyAgICBpZiAobV9zdGF0ZSAhPSBTeW5jaHJvbml6YXRpb25TdGF0 ZTo6SWRsZSAmJiBjYW5VcGRhdGVMYXllcnNPblNjcm9sbGluZ1RocmVhZCgpKQogICAgICAgICBh cHBseUxheWVyUG9zaXRpb25zSW50ZXJuYWwoKTsKIAogICAgIHN3aXRjaCAobV9zdGF0ZSkgewpk aWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGFnZS9zY3JvbGxpbmcvVGhyZWFkZWRTY3JvbGxp bmdUcmVlLmggYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL3Njcm9sbGluZy9UaHJlYWRlZFNjcm9sbGlu Z1RyZWUuaAppbmRleCAwMDcwYmI1M2NhMWU1NjgzNDNlNDgwNjllNWQzMDE0M2U4YzU2ZjYyLi5i MjVmZjE0Y2ExMmNiMjhhN2NhMTlmMDJhYjFhZmFjNDg2YTk1MTNmIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9wYWdlL3Njcm9sbGluZy9UaHJlYWRlZFNjcm9sbGluZ1RyZWUuaAorKysgYi9T b3VyY2UvV2ViQ29yZS9wYWdlL3Njcm9sbGluZy9UaHJlYWRlZFNjcm9sbGluZ1RyZWUuaApAQCAt ODEsNiArODEsOCBAQCBwcml2YXRlOgogCiAgICAgdm9pZCBkaXNwbGF5RGlkUmVmcmVzaE9uU2Ny b2xsaW5nVGhyZWFkKCk7CiAgICAgdm9pZCB3YWl0Rm9yUmVuZGVyaW5nVXBkYXRlQ29tcGxldGlv bk9yVGltZW91dCgpOworICAgIAorICAgIGJvb2wgY2FuVXBkYXRlTGF5ZXJzT25TY3JvbGxpbmdU aHJlYWQoKSBjb25zdDsKIAogICAgIHZvaWQgc2NoZWR1bGVEZWxheWVkUmVuZGVyaW5nVXBkYXRl RGV0ZWN0aW9uVGltZXIoU2Vjb25kcyk7CiAgICAgdm9pZCBkZWxheWVkUmVuZGVyaW5nVXBkYXRl RGV0ZWN0aW9uVGltZXJGaXJlZCgpOwo=