Bug 1622445 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

For posterity here's the stacks. They seem pretty clear:

- OMTP is enabled
- A `SkPathRef` is created during `ClientLayerManager::EndTransaction` as part of `SVGGeometryFrame::Render` (inside `GetOrBuildPath`, [here](https://searchfox.org/mozilla-central/rev/3f8c67d7fd836d559491e3fe497bc739f707c1a6/dom/svg/SVGGeometryElement.cpp#105))
- This `SkPathRef` seems to be cached on the element, and a hit-testing operation ends up calling `ContainsPoint` on it [here[(https://searchfox.org/mozilla-central/rev/3f8c67d7fd836d559491e3fe497bc739f707c1a6/layout/svg/SVGGeometryFrame.cpp#324). Unfortunately, this internally triggers a lazy call to `computeBounds` [here](https://searchfox.org/mozilla-central/rev/3f8c67d7fd836d559491e3fe497bc739f707c1a6/gfx/skia/skia/include/private/SkPathRef.h#153) which is a mutating operation. So there's a write from the main thread.
- This `SkPathRef` is also passed to the paint thread in a captured drawtarget and read from in the process of rasterization.

I don't remember all the OMTP details here but I'm guessing this is a legitimate bug in the code. The `SkPathRef` is held on to via a RefPtr so in that respect it's threadsafe. But the fact that the main thread holds on to it and mutates it after handing off a copy to the paint thread seems bad.
For posterity here's the stacks. They seem pretty clear:

- OMTP is enabled
- A `SkPathRef` is created during `ClientLayerManager::EndTransaction` as part of `SVGGeometryFrame::Render` (inside `GetOrBuildPath`, [here](https://searchfox.org/mozilla-central/rev/3f8c67d7fd836d559491e3fe497bc739f707c1a6/dom/svg/SVGGeometryElement.cpp#105))
- This `SkPathRef` seems to be cached on the element, and a hit-testing operation ends up calling `ContainsPoint` on it [here](https://searchfox.org/mozilla-central/rev/3f8c67d7fd836d559491e3fe497bc739f707c1a6/layout/svg/SVGGeometryFrame.cpp#324). Unfortunately, this internally triggers a lazy call to `computeBounds` [here](https://searchfox.org/mozilla-central/rev/3f8c67d7fd836d559491e3fe497bc739f707c1a6/gfx/skia/skia/include/private/SkPathRef.h#153) which is a mutating operation. So there's a write from the main thread.
- This `SkPathRef` is also passed to the paint thread in a captured drawtarget and read from in the process of rasterization.

I don't remember all the OMTP details here but I'm guessing this is a legitimate bug in the code. The `SkPathRef` is held on to via a RefPtr so in that respect it's threadsafe. But the fact that the main thread holds on to it and mutates it after handing off a copy to the paint thread seems bad.

Back to Bug 1622445 Comment 5