`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).
<rdar://problem/49357109>
Created attachment 366119 [details] Patch
Comment on attachment 366119 [details] 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?
Comment on attachment 366119 [details] 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!
Comment on attachment 366119 [details] Patch Clearing flags on attachment: 366119 Committed r243611: <https://trac.webkit.org/changeset/243611>
All reviewed patches have been landed. Closing bug.