The serialization actor for remote documents, implemented at https://searchfox.org/mozilla-central/source/dom/webbrowserpersist/WebBrowserPersistSerializeParent.cpp , writes directly to a file stream from the actor, whose messages are received on the main thread. I'm torn on how to fix this. The surrounding process is already asynchronous as the data to write comes from the child, and each instance notifies its creator (the [caller of `WriteContent` on `WebBrowserPersistRemoteDocument`](https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/dom/webbrowserpersist/nsWebBrowserPersist.cpp#704-707) when it's done. The two obvious routes I see to take the file writing off the main thread are: 1. move the serialization actor to be managed by PBackground so that the messages from the child come in away from the main thread, and just write from there. Then post a runnable back to the main thread when the actor is finished. 2. keep the actor how it is but just have it queue the actual writing tasks by moving the buffers onto the a taskqueue (so they execute serially) on the background IO thread. The first seems the more idiomatic, but I don't actually know how the IPC background stuff works, and if doing mainthread IO on that thread is going to have other bad effects (seems we also use it for vsync? That seems scary...). The relevant code that currently instantiates the serializer actor also does so directly, and the alloc helper at https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/dom/webbrowserpersist/WebBrowserPersistDocumentParent.cpp#86-89 `MOZ_CRASH`es. I don't know why this arrangement was chosen and if it means moving to PBackground is going to cause other issues - looks like :jld implemented this, so I'm hoping you know! The second is less idiomatic / more fussy, but otherwise straightforward - I'm adding a queue on the webbrowserpersist instance in bug 1658202 anyway, so AFAICT it'd just be a question of `std::move` ing the data to a runnable that gets dispatched to the other thread and writes there, and doing the same when finishing - though we may have to dispatch twice, and the actual data will first come in on the main thread, so if we want to avoid that and have a little neater code, perhaps the first option is better. :jld, :mattwoodrow, can you point the way? :-)
Bug 1661941 Comment 0 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
The serialization actor for remote documents, implemented at https://searchfox.org/mozilla-central/source/dom/webbrowserpersist/WebBrowserPersistSerializeParent.cpp , writes directly to a file stream from the actor, whose messages are received on the main thread. I'm torn on how to fix this. The surrounding process is already asynchronous as the data to write comes from the child, and each instance notifies its creator (the [caller of `WriteContent` on `WebBrowserPersistRemoteDocument`](https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/dom/webbrowserpersist/nsWebBrowserPersist.cpp#704-707)) when it's done. The two obvious routes I see to take the file writing off the main thread are: 1. move the serialization actor to be managed by PBackground so that the messages from the child come in away from the main thread, and just write from there. Then post a runnable back to the main thread when the actor is finished. 2. keep the actor how it is but just have it queue the actual writing tasks by moving the buffers onto the a taskqueue (so they execute serially) on the background IO thread. The first seems the more idiomatic, but I don't actually know how the IPC background stuff works, and if doing IO on that thread is going to have other bad effects (seems we also use it for vsync? That seems scary...). The relevant code that currently instantiates the serializer actor also does so directly, and the alloc helper at https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/dom/webbrowserpersist/WebBrowserPersistDocumentParent.cpp#86-89 `MOZ_CRASH`es. I don't know why this arrangement was chosen and if it means moving to PBackground is going to cause other issues - looks like :jld implemented this, so I'm hoping you know! The second is less idiomatic / more fussy, but otherwise straightforward - I'm adding a queue on the webbrowserpersist instance in bug 1658202 anyway, so AFAICT it'd just be a question of `std::move` ing the data to a runnable that gets dispatched to the other thread and writes there, and doing the same when finishing - though we may have to dispatch twice, and the actual data will first come in on the main thread, so if we want to avoid that and have a little neater code, perhaps the first option is better. :jld, :mattwoodrow, can you point the way? :-)