262934 2023-10-09 20:30:30 -0700 AX: AXIsolatedTree::children() should not re-initialize m_children unless they've changed 2023-10-10 14:52:25 -0700 1 1 1 Unclassified WebKit Accessibility Other Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 tyler_w tyler_w aboxhall andresg_22 apinheiro cfleizach dmazzoni ews-watchlist jcraig jdiggs samuel_white webkit-bug-importer oldest_to_newest 1983899 0 tyler_w 2023-10-09 20:30:30 -0700 This shows up heavily on samples for some sites. 1983900 1 webkit-bug-importer 2023-10-09 20:30:40 -0700 <rdar://problem/116715426> 1983901 2 468141 tyler_w 2023-10-09 20:35:38 -0700 Created attachment 468141 Patch 1983925 3 468141 cfleizach 2023-10-09 22:10:03 -0700 Comment on attachment 468141 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468141&action=review > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:463 > + m_childrenIDs = WTFMove(ids); Can we avoid dirty children if the IDs are same in same order? 1983928 4 tyler_w 2023-10-09 22:15:37 -0700 (In reply to chris fleizach from comment #3) > Comment on attachment 468141 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=468141&action=review > > > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:463 > > + m_childrenIDs = WTFMove(ids); > > Can we avoid dirty children if the IDs are same in same order? I thought about this too...but ended up deciding against it since it forces an O(n) iteration for what is likely a very uncommon case. Additionally, this proposed iteration would have to happen while we hold the m_changeLogLock, potentially increasing lock contention. 1983977 5 andresg_22 2023-10-10 06:01:17 -0700 (In reply to Tyler Wilcock from comment #2) > Created attachment 468141 [details] > Patch Are we covering the unconnected objects case? We don't update m_pendingchildrenUpdates for those objects. 1984047 6 tyler_w 2023-10-10 10:39:22 -0700 (In reply to Andres Gonzalez from comment #5) > (In reply to Tyler Wilcock from comment #2) > > Created attachment 468141 [details] > > Patch > > Are we covering the unconnected objects case? We don't update > m_pendingchildrenUpdates for those objects. I don't think that's necessary here. The only time we need to mark children as dirty is when AXIsolatedObject::m_childrenIDs changes, which addUnconnectedNode doesn't do. 1984134 7 ews-feeder 2023-10-10 14:52:22 -0700 Committed 269170@main (4d35a6d87939): <https://commits.webkit.org/269170@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 468141. 468141 2023-10-09 20:35:38 -0700 2023-10-10 14:52:24 -0700 Patch bug-262934-20231009203537.patch text/plain 4654 tyler_w RnJvbSBjY2U3MWIzYWZhMGM0OTgyYmU2NGU1ZTY2ZDgxZGM4MzQ0MWJjYTlkIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBUeWxlciBXaWxjb2NrIDx0eWxlcl93QGFwcGxlLmNvbT4KRGF0 ZTogTW9uLCA5IE9jdCAyMDIzIDIwOjMzOjAyIC0wNzAwClN1YmplY3Q6IFtQQVRDSF0gQVg6IEFY SXNvbGF0ZWRUcmVlOjpjaGlsZHJlbigpIHNob3VsZCBub3QgcmUtaW5pdGlhbGl6ZQogbV9jaGls ZHJlbiB1bmxlc3MgdGhleSd2ZSBjaGFuZ2VkCiBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MjYyOTM0IHJkYXI6Ly9wcm9ibGVtLzExNjcxNTQyNgoKUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCgpUaGlzIHNhdmVzIDQuM2sgc2FtcGxlcyBvbiBhbiBpbnRlcm5hbCB3 ZWJwYWdlLgoKKiBTb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L2lzb2xhdGVkdHJlZS9BWElz b2xhdGVkT2JqZWN0LmNwcDoKKFdlYkNvcmU6OkFYSXNvbGF0ZWRPYmplY3Q6OmRldGFjaFJlbW90 ZVBhcnRzKToKKFdlYkNvcmU6OkFYSXNvbGF0ZWRPYmplY3Q6OnNldENoaWxkcmVuSURzKToKKFdl YkNvcmU6OkFYSXNvbGF0ZWRPYmplY3Q6OmNoaWxkcmVuKToKKiBTb3VyY2UvV2ViQ29yZS9hY2Nl c3NpYmlsaXR5L2lzb2xhdGVkdHJlZS9BWElzb2xhdGVkT2JqZWN0Lmg6CiogU291cmNlL1dlYkNv cmUvYWNjZXNzaWJpbGl0eS9pc29sYXRlZHRyZWUvQVhJc29sYXRlZFRyZWUuY3BwOgooV2ViQ29y ZTo6QVhJc29sYXRlZFRyZWU6OmFwcGx5UGVuZGluZ0NoYW5nZXMpOgotLS0KIC4uLi9pc29sYXRl ZHRyZWUvQVhJc29sYXRlZE9iamVjdC5jcHAgICAgICAgICB8IDIwICsrKysrKysrKysrKysrLS0t LS0KIC4uLi9pc29sYXRlZHRyZWUvQVhJc29sYXRlZE9iamVjdC5oICAgICAgICAgICB8ICAyICsr CiAuLi4vaXNvbGF0ZWR0cmVlL0FYSXNvbGF0ZWRUcmVlLmNwcCAgICAgICAgICAgfCAgMiArLQog MyBmaWxlcyBjaGFuZ2VkLCAxOCBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQoKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvaXNvbGF0ZWR0cmVlL0FYSXNvbGF0 ZWRPYmplY3QuY3BwIGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9pc29sYXRlZHRyZWUv QVhJc29sYXRlZE9iamVjdC5jcHAKaW5kZXggYTcxYWY5ZDY5ODdlLi5jOTI1OWE3NTEyMzYgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvaXNvbGF0ZWR0cmVlL0FYSXNv bGF0ZWRPYmplY3QuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvaXNvbGF0 ZWR0cmVlL0FYSXNvbGF0ZWRPYmplY3QuY3BwCkBAIC00NDIsNiArNDQyLDcgQEAgdm9pZCBBWElz b2xhdGVkT2JqZWN0OjpkZXRhY2hSZW1vdGVQYXJ0cyhBY2Nlc3NpYmlsaXR5RGV0YWNobWVudFR5 cGUpCiAgICAgICAgICAgICBjaGlsZC0+ZGV0YWNoRnJvbVBhcmVudCgpOwogICAgIH0KICAgICBt X2NoaWxkcmVuSURzLmNsZWFyKCk7CisgICAgbV9jaGlsZHJlbkRpcnR5ID0gdHJ1ZTsKIH0KIAog I2lmICFQTEFURk9STShNQUMpCkBAIC00NTcsNiArNDU4LDEyIEBAIHZvaWQgQVhJc29sYXRlZE9i amVjdDo6ZGV0YWNoRnJvbVBhcmVudCgpCiAgICAgbV9wYXJlbnRJRCA9IHsgfTsKIH0KIAordm9p ZCBBWElzb2xhdGVkT2JqZWN0OjpzZXRDaGlsZHJlbklEcyhWZWN0b3I8QVhJRD4mJiBpZHMpCit7 CisgICAgbV9jaGlsZHJlbklEcyA9IFdURk1vdmUoaWRzKTsKKyAgICBtX2NoaWxkcmVuRGlydHkg PSB0cnVlOworfQorCiBjb25zdCBBWENvcmVPYmplY3Q6OkFjY2Vzc2liaWxpdHlDaGlsZHJlblZl Y3RvciYgQVhJc29sYXRlZE9iamVjdDo6Y2hpbGRyZW4oYm9vbCB1cGRhdGVDaGlsZHJlbklmTmVl ZGVkKQogewogI2lmIFVTRShBUFBMRV9JTlRFUk5BTF9TREspCkBAIC00NzEsMTEgKzQ3OCwxNCBA QCBjb25zdCBBWENvcmVPYmplY3Q6OkFjY2Vzc2liaWxpdHlDaGlsZHJlblZlY3RvciYgQVhJc29s YXRlZE9iamVjdDo6Y2hpbGRyZW4oYm9vbAogICAgICAgICBwcm90ZWN0ZWRUaGlzID0gdGhpczsK ICAgICAgICAgdXBkYXRlQmFja2luZ1N0b3JlKCk7CiAKLSAgICAgICAgbV9jaGlsZHJlbiA9IFdU Rjo6Y29tcGFjdE1hcChtX2NoaWxkcmVuSURzLCBbJl0oYXV0byYgY2hpbGRJRCkgLT4gc3RkOjpv cHRpb25hbDxSZWZQdHI8QVhDb3JlT2JqZWN0Pj4gewotICAgICAgICAgICAgaWYgKFJlZlB0ciBj aGlsZCA9IHRyZWUoKS0+b2JqZWN0Rm9ySUQoY2hpbGRJRCkpCi0gICAgICAgICAgICAgICAgcmV0 dXJuIGNoaWxkOwotICAgICAgICAgICAgcmV0dXJuIHN0ZDo6bnVsbG9wdDsKLSAgICAgICAgfSk7 CisgICAgICAgIGlmIChtX2NoaWxkcmVuRGlydHkpIHsKKyAgICAgICAgICAgIG1fY2hpbGRyZW4g PSBXVEY6OmNvbXBhY3RNYXAobV9jaGlsZHJlbklEcywgWyZdKGF1dG8mIGNoaWxkSUQpIC0+IHN0 ZDo6b3B0aW9uYWw8UmVmUHRyPEFYQ29yZU9iamVjdD4+IHsKKyAgICAgICAgICAgICAgICBpZiAo UmVmUHRyIGNoaWxkID0gdHJlZSgpLT5vYmplY3RGb3JJRChjaGlsZElEKSkKKyAgICAgICAgICAg ICAgICAgICAgcmV0dXJuIGNoaWxkOworICAgICAgICAgICAgICAgIHJldHVybiBzdGQ6Om51bGxv cHQ7CisgICAgICAgICAgICB9KTsKKyAgICAgICAgICAgIG1fY2hpbGRyZW5EaXJ0eSA9IGZhbHNl OworICAgICAgICB9CiAgICAgICAgIEFTU0VSVChtX2NoaWxkcmVuLnNpemUoKSA9PSBtX2NoaWxk cmVuSURzLnNpemUoKSk7CiAgICAgfQogICAgIHJldHVybiBtX2NoaWxkcmVuOwpkaWZmIC0tZ2l0 IGEvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9pc29sYXRlZHRyZWUvQVhJc29sYXRlZE9i amVjdC5oIGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9pc29sYXRlZHRyZWUvQVhJc29s YXRlZE9iamVjdC5oCmluZGV4IDU5YWE4MDdiMGRiZC4uNmE2OThkOGFlMTVjIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L2lzb2xhdGVkdHJlZS9BWElzb2xhdGVkT2Jq ZWN0LmgKKysrIGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9pc29sYXRlZHRyZWUvQVhJ c29sYXRlZE9iamVjdC5oCkBAIC0zMTMsNiArMzEzLDcgQEAgcHJpdmF0ZToKICAgICB2b2lkIHNl dFNlbGVjdGVkQ2hpbGRyZW4oY29uc3QgQWNjZXNzaWJpbGl0eUNoaWxkcmVuVmVjdG9yJikgZmlu YWw7CiAgICAgQWNjZXNzaWJpbGl0eUNoaWxkcmVuVmVjdG9yIHZpc2libGVDaGlsZHJlbigpIGZp bmFsIHsgcmV0dXJuIHRyZWUoKS0+b2JqZWN0c0ZvcklEcyh2ZWN0b3JBdHRyaWJ1dGVWYWx1ZTxB WElEPihBWFByb3BlcnR5TmFtZTo6VmlzaWJsZUNoaWxkcmVuKSk7IH0KICAgICBBdG9tU3RyaW5n IHRhZ05hbWUoKSBjb25zdCBmaW5hbDsKKyAgICB2b2lkIHNldENoaWxkcmVuSURzKFZlY3RvcjxB WElEPiYmKTsKICAgICBjb25zdCBBY2Nlc3NpYmlsaXR5Q2hpbGRyZW5WZWN0b3ImIGNoaWxkcmVu KGJvb2wgdXBkYXRlQ2hpbGRyZW5JZk5lZWRlZCA9IHRydWUpIGZpbmFsOwogICAgIHZvaWQgdXBk YXRlQ2hpbGRyZW5JZk5lY2Vzc2FyeSgpIGZpbmFsOwogICAgIGJvb2wgaXNEZXRhY2hlZEZyb21Q YXJlbnQoKSBmaW5hbDsKQEAgLTUzMCw2ICs1MzEsNyBAQCBwcml2YXRlOgogICAgIC8vIEZJWE1F OiBNYWtlIHRoaXMgYSBUaHJlYWRTYWZlV2Vha1B0cjxBWElzb2xhdGVkVHJlZT4uCiAgICAgUmVm UHRyPEFYSXNvbGF0ZWRUcmVlPiBtX2NhY2hlZFRyZWU7CiAgICAgQVhJRCBtX3BhcmVudElEOwor ICAgIGJvb2wgbV9jaGlsZHJlbkRpcnR5IHsgdHJ1ZSB9OwogICAgIFZlY3RvcjxBWElEPiBtX2No aWxkcmVuSURzOwogICAgIFZlY3RvcjxSZWZQdHI8QVhDb3JlT2JqZWN0Pj4gbV9jaGlsZHJlbjsK ICAgICBBWFByb3BlcnR5TWFwIG1fcHJvcGVydHlNYXA7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi Q29yZS9hY2Nlc3NpYmlsaXR5L2lzb2xhdGVkdHJlZS9BWElzb2xhdGVkVHJlZS5jcHAgYi9Tb3Vy Y2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L2lzb2xhdGVkdHJlZS9BWElzb2xhdGVkVHJlZS5jcHAK aW5kZXggZjYyMTdmZjhhMjI5Li4yYmRlOWVlN2YzMDIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL2FjY2Vzc2liaWxpdHkvaXNvbGF0ZWR0cmVlL0FYSXNvbGF0ZWRUcmVlLmNwcAorKysgYi9T b3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L2lzb2xhdGVkdHJlZS9BWElzb2xhdGVkVHJlZS5j cHAKQEAgLTEwOTUsNyArMTA5NSw3IEBAIHZvaWQgQVhJc29sYXRlZFRyZWU6OmFwcGx5UGVuZGlu Z0NoYW5nZXMoKQogICAgIGZvciAoYXV0byYgdXBkYXRlIDogbV9wZW5kaW5nQ2hpbGRyZW5VcGRh dGVzKSB7CiAgICAgICAgIEFYTE9HKG1ha2VTdHJpbmcoInVwZGF0aW5nIGNoaWxkcmVuIGZvciBh eElEICIsIHVwZGF0ZS5maXJzdC5sb2dnaW5nU3RyaW5nKCkpKTsKICAgICAgICAgaWYgKFJlZlB0 ciBvYmplY3QgPSBvYmplY3RGb3JJRCh1cGRhdGUuZmlyc3QpKQotICAgICAgICAgICAgb2JqZWN0 LT5tX2NoaWxkcmVuSURzID0gV1RGTW92ZSh1cGRhdGUuc2Vjb25kKTsKKyAgICAgICAgICAgIG9i amVjdC0+c2V0Q2hpbGRyZW5JRHMoV1RGTW92ZSh1cGRhdGUuc2Vjb25kKSk7CiAgICAgfQogICAg IG1fcGVuZGluZ0NoaWxkcmVuVXBkYXRlcy5jbGVhcigpOwogCg==