196324 2019-03-27 15:42:58 -0700 Web Inspector: Canvas: unbinding a canvas should always remove the agent as an observer 2019-03-28 11:08:11 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 hi hi cdumez commit-queue esprehn+autocc ews-watchlist gyuyoung.kim hi inspector-bugzilla-changes joepeck mattbaker webkit-bug-importer oldest_to_newest 1521878 0 hi 2019-03-27 15:42:58 -0700 `InspectorCanvasAgent::unbindCanvas` is currently called in two places: - `InspectorCanvasAgent::frameNavigated`, right after the `InspectorCanvasAgent` removes itself as an observer from the canvas in question - `InspectorCanvasAgent::canvasDestroyed`, which is called on all observers when the canvas is about to be destructed Previously, in order to avoid modification-while-iterating, we wouldn't remove the `InspectorCanvasAgent` from the canvas inside `InspectorCanvasAgent::canvasDestroyed`, since the canvas is about to be destructed anyways. This isn't a very good practice, and should be "corrected" (e.g. the observer shouldn't have any idea about the specifics of how the canvas treats/notifies its observers). 1521882 1 webkit-bug-importer 2019-03-27 15:47:57 -0700 <rdar://problem/49357109> 1521886 2 366119 hi 2019-03-27 15:51:48 -0700 Created attachment 366119 Patch 1521951 3 366119 mattbaker 2019-03-27 18:10:24 -0700 Comment on attachment 366119 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366119&action=review r=me > Source/WebCore/html/CanvasBase.cpp:74 > observer->canvasChanged(*this, rect); I noticed this is a widely used pattern in WebKit. Is it to safeguard against m_observers mutating during iteration? 1522098 4 366119 hi 2019-03-28 10:38:41 -0700 Comment on attachment 366119 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366119&action=review >> Source/WebCore/html/CanvasBase.cpp:74 >> observer->canvasChanged(*this, rect); > > I noticed this is a widely used pattern in WebKit. Is it to safeguard against m_observers mutating during iteration? Yup! 1522114 5 366119 commit-queue 2019-03-28 11:08:10 -0700 Comment on attachment 366119 Patch Clearing flags on attachment: 366119 Committed r243611: <https://trac.webkit.org/changeset/243611> 1522115 6 commit-queue 2019-03-28 11:08:11 -0700 All reviewed patches have been landed. Closing bug. 366119 2019-03-27 15:51:48 -0700 2019-03-28 11:08:10 -0700 Patch bug-196324-20190327155142.patch text/plain 3887 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA3Njg1ZTUzODYzZjYxMmM4MDg5ZjVkZDNlNzg3ZmNlMGYyZTg1YWI1Li4z OTJmN2VkYTM2NmUzMTAzY2UwNTc3ODQ3MDc3M2EwYWI1ZDk5NWQ4IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMjMgQEAKKzIwMTktMDMtMjcgIERldmluIFJvdXNzbyAgPGRyb3Vzc29AYXBwbGUuY29t PgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IENhbnZhczogdW5iaW5kaW5nIGEgY2FudmFzIHNo b3VsZCBhbHdheXMgcmVtb3ZlIHRoZSBhZ2VudCBhcyBhbiBvYnNlcnZlcgorICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk2MzI0CisgICAgICAgIDxyZGFy Oi8vcHJvYmxlbS80OTM1NzEwOT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBObyBjaGFuZ2UgaW4gZnVuY3Rpb25hbGl0eS4KKworICAgICAgICAqIGh0 bWwvQ2FudmFzQmFzZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpDYW52YXNCYXNlOjpub3RpZnlP YnNlcnZlcnNDYW52YXNDaGFuZ2VkKToKKyAgICAgICAgKFdlYkNvcmU6OkNhbnZhc0Jhc2U6Om5v dGlmeU9ic2VydmVyc0NhbnZhc1Jlc2l6ZWQpOgorICAgICAgICAoV2ViQ29yZTo6Q2FudmFzQmFz ZTo6bm90aWZ5T2JzZXJ2ZXJzQ2FudmFzRGVzdHJveWVkKToKKworICAgICAgICAqIGluc3BlY3Rv ci9hZ2VudHMvSW5zcGVjdG9yQ2FudmFzQWdlbnQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SW5z cGVjdG9yQ2FudmFzQWdlbnQ6OmZyYW1lTmF2aWdhdGVkKToKKyAgICAgICAgKFdlYkNvcmU6Oklu c3BlY3RvckNhbnZhc0FnZW50OjpiaW5kQ2FudmFzKToKKyAgICAgICAgKFdlYkNvcmU6Okluc3Bl Y3RvckNhbnZhc0FnZW50Ojp1bmJpbmRDYW52YXMpOgorCiAyMDE5LTAzLTI3ICBTaGF3biBSb2Jl cnRzICA8c3JvYmVydHNAYXBwbGUuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQsIHJvbGxpbmcg b3V0IHIyNDMzNDYuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9odG1sL0NhbnZhc0Jhc2Uu Y3BwIGIvU291cmNlL1dlYkNvcmUvaHRtbC9DYW52YXNCYXNlLmNwcAppbmRleCBhOGFlODBhODI5 MGNjMDJhOTM0ODRkOTQwYWE5ZGQzM2IxOTg4NmQ0Li5iYjIyMDM5NDJmOGVlNDgyN2YwYmYwYmMy Mzg5ZTJmYjVkNWQ3M2QwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9odG1sL0NhbnZhc0Jh c2UuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvQ2FudmFzQmFzZS5jcHAKQEAgLTMxLDYg KzMxLDcgQEAKICNpbmNsdWRlICJFbGVtZW50LmgiCiAjaW5jbHVkZSAiRmxvYXRSZWN0LmgiCiAj aW5jbHVkZSAiSW5zcGVjdG9ySW5zdHJ1bWVudGF0aW9uLmgiCisjaW5jbHVkZSA8d3RmL1ZlY3Rv ci5oPgogCiBuYW1lc3BhY2UgV2ViQ29yZSB7CiAKQEAgLTY5LDEzICs3MCwxMyBAQCB2b2lkIENh bnZhc0Jhc2U6OnJlbW92ZU9ic2VydmVyKENhbnZhc09ic2VydmVyJiBvYnNlcnZlcikKIAogdm9p ZCBDYW52YXNCYXNlOjpub3RpZnlPYnNlcnZlcnNDYW52YXNDaGFuZ2VkKGNvbnN0IEZsb2F0UmVj dCYgcmVjdCkKIHsKLSAgICBmb3IgKGF1dG8mIG9ic2VydmVyIDogbV9vYnNlcnZlcnMpCisgICAg Zm9yIChhdXRvJiBvYnNlcnZlciA6IGNvcHlUb1ZlY3RvcihtX29ic2VydmVycykpCiAgICAgICAg IG9ic2VydmVyLT5jYW52YXNDaGFuZ2VkKCp0aGlzLCByZWN0KTsKIH0KIAogdm9pZCBDYW52YXNC YXNlOjpub3RpZnlPYnNlcnZlcnNDYW52YXNSZXNpemVkKCkKIHsKLSAgICBmb3IgKGF1dG8mIG9i c2VydmVyIDogbV9vYnNlcnZlcnMpCisgICAgZm9yIChhdXRvJiBvYnNlcnZlciA6IGNvcHlUb1Zl Y3RvcihtX29ic2VydmVycykpCiAgICAgICAgIG9ic2VydmVyLT5jYW52YXNSZXNpemVkKCp0aGlz KTsKIH0KIApAQCAtODMsNyArODQsNyBAQCB2b2lkIENhbnZhc0Jhc2U6Om5vdGlmeU9ic2VydmVy c0NhbnZhc0Rlc3Ryb3llZCgpCiB7CiAgICAgQVNTRVJUKCFtX2RpZE5vdGlmeU9ic2VydmVyc0Nh bnZhc0Rlc3Ryb3llZCk7CiAKLSAgICBmb3IgKGF1dG8mIG9ic2VydmVyIDogbV9vYnNlcnZlcnMp CisgICAgZm9yIChhdXRvJiBvYnNlcnZlciA6IGNvcHlUb1ZlY3RvcihtX29ic2VydmVycykpCiAg ICAgICAgIG9ic2VydmVyLT5jYW52YXNEZXN0cm95ZWQoKnRoaXMpOwogCiAgICAgbV9vYnNlcnZl cnMuY2xlYXIoKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9hZ2VudHMv SW5zcGVjdG9yQ2FudmFzQWdlbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL2FnZW50 cy9JbnNwZWN0b3JDYW52YXNBZ2VudC5jcHAKaW5kZXggZDA4N2IwNTMzYWNhNDlmZmM2MmUyOTBm MDY0MzM5NWE5NTMxNTU5MS4uOTM2N2E0ZTUwNTRhMDA0MzZmNzMyY2YzMDlkZWQ4ZGQ0MWUxMTVj YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL2FnZW50cy9JbnNwZWN0b3JD YW52YXNBZ2VudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL2FnZW50cy9JbnNw ZWN0b3JDYW52YXNBZ2VudC5jcHAKQEAgLTM3OCw4ICszNzgsNiBAQCB2b2lkIEluc3BlY3RvckNh bnZhc0FnZW50OjpmcmFtZU5hdmlnYXRlZChGcmFtZSYgZnJhbWUpCiAgICAgfQogCiAgICAgZm9y IChhdXRvKiBpbnNwZWN0b3JDYW52YXMgOiBpbnNwZWN0b3JDYW52YXNlcykgewotICAgICAgICBp bnNwZWN0b3JDYW52YXMtPmNvbnRleHQoKS5jYW52YXNCYXNlKCkucmVtb3ZlT2JzZXJ2ZXIoKnRo aXMpOwotCiAgICAgICAgIFN0cmluZyBpZGVudGlmaWVyID0gdW5iaW5kQ2FudmFzKCppbnNwZWN0 b3JDYW52YXMpOwogICAgICAgICBtX2Zyb250ZW5kRGlzcGF0Y2hlci0+Y2FudmFzUmVtb3ZlZChp ZGVudGlmaWVyKTsKICAgICB9CkBAIC02NDcsMTEgKzY0NSwxMSBAQCB2b2lkIEluc3BlY3RvckNh bnZhc0FnZW50OjpjbGVhckNhbnZhc0RhdGEoKQogCiBJbnNwZWN0b3JDYW52YXMmIEluc3BlY3Rv ckNhbnZhc0FnZW50OjpiaW5kQ2FudmFzKENhbnZhc1JlbmRlcmluZ0NvbnRleHQmIGNvbnRleHQs IGJvb2wgY2FwdHVyZUJhY2t0cmFjZSkKIHsKLSAgICBjb250ZXh0LmNhbnZhc0Jhc2UoKS5hZGRP YnNlcnZlcigqdGhpcyk7Ci0KICAgICBhdXRvIGluc3BlY3RvckNhbnZhcyA9IEluc3BlY3RvckNh bnZhczo6Y3JlYXRlKGNvbnRleHQpOwogICAgIG1faWRlbnRpZmllclRvSW5zcGVjdG9yQ2FudmFz LnNldChpbnNwZWN0b3JDYW52YXMtPmlkZW50aWZpZXIoKSwgaW5zcGVjdG9yQ2FudmFzLmNvcHlS ZWYoKSk7CiAKKyAgICBpbnNwZWN0b3JDYW52YXMtPmNvbnRleHQoKS5jYW52YXNCYXNlKCkuYWRk T2JzZXJ2ZXIoKnRoaXMpOworCiAgICAgbV9mcm9udGVuZERpc3BhdGNoZXItPmNhbnZhc0FkZGVk KGluc3BlY3RvckNhbnZhcy0+YnVpbGRPYmplY3RGb3JDYW52YXMoY2FwdHVyZUJhY2t0cmFjZSkp OwogCiAjaWYgRU5BQkxFKFdFQkdMKQpAQCAtNjgyLDYgKzY4MCw4IEBAIFN0cmluZyBJbnNwZWN0 b3JDYW52YXNBZ2VudDo6dW5iaW5kQ2FudmFzKEluc3BlY3RvckNhbnZhcyYgaW5zcGVjdG9yQ2Fu dmFzKQogICAgICAgICB1bmJpbmRQcm9ncmFtKCppbnNwZWN0b3JQcm9ncmFtKTsKICNlbmRpZgog CisgICAgaW5zcGVjdG9yQ2FudmFzLmNvbnRleHQoKS5jYW52YXNCYXNlKCkucmVtb3ZlT2JzZXJ2 ZXIoKnRoaXMpOworCiAgICAgU3RyaW5nIGlkZW50aWZpZXIgPSBpbnNwZWN0b3JDYW52YXMuaWRl bnRpZmllcigpOwogICAgIG1faWRlbnRpZmllclRvSW5zcGVjdG9yQ2FudmFzLnJlbW92ZShpZGVu dGlmaWVyKTsKIAo=