In a pernosco trace I noticed ClientHandleOp was always calling either the sequence of [mResolveCallback, mRejectCallback] or [mRejectCallback, mRejectCallback]. This is because we never clear out mRejectCallback and always [call it during ActorDestroy](https://searchfox.org/mozilla-central/rev/5f037f0b930de92a65ced0831b0f199281ac4df3/dom/clients/manager/ClientHandleOpChild.cpp#16-17). Because the callbacks always just make simple calls into MozPromise this doesn't actually pose a problem, but is definitely slightly weird and a bit confusing/surprising inside a debugger. Ideally we could convert this to using the async return value mechanism to simplify things while eliminating the extra callback, but there are two slightly complicating factors that make this not an entirely rote mechanical change: - There is a slight conceptual plumbing change as our NS_FAILED case actually would be a resolved value from IPC, not a rejected value (which would correspond to ActorDestroy without ever having received a value). - Currently we use a discriminated union for the 4 different payloads; these probably want to be their own async return value calls to reduce obfuscation.
Bug 1900242 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.
In a pernosco trace I noticed ClientHandleOp was always calling either the sequence of [mResolveCallback, mRejectCallback] or [mRejectCallback, mRejectCallback]. This is because we never clear out mRejectCallback and always [call it during ActorDestroy](https://searchfox.org/mozilla-central/rev/5f037f0b930de92a65ced0831b0f199281ac4df3/dom/clients/manager/ClientHandleOpChild.cpp#16-17). Because the callbacks always just make simple calls into MozPromise this doesn't actually pose a problem, but is definitely slightly weird and a bit confusing/surprising inside a debugger. Ideally we could convert this to using the async return value mechanism to simplify things while eliminating the extra callback, but there are two slightly complicating factors that make this not an entirely rote mechanical change: - There is a slight conceptual plumbing change as our NS_FAILED case actually would be a resolved value from IPC, not a rejected value (which would correspond to ActorDestroy without ever having received a value). - Currently we use a discriminated union for the 4 different payloads. Ideally these would be their own async return value calls to reduce obfuscation, but there's also a bit of machinery related to the `EnsureSource()` mechanism and effectively forwarding the requests onward to the ClientSourceParent, so there would be some code duplication (but not enough to justify template machinery and its many costs).
In a pernosco trace I noticed ClientHandleOp was always calling either the sequence of [mResolveCallback, mRejectCallback] or [mRejectCallback, mRejectCallback]. This is because we never clear out mRejectCallback and always [call it during ActorDestroy](https://searchfox.org/mozilla-central/rev/5f037f0b930de92a65ced0831b0f199281ac4df3/dom/clients/manager/ClientHandleOpChild.cpp#16-17). Because the callbacks always just make simple calls into MozPromise this doesn't actually pose a problem, but is definitely slightly weird and a bit confusing/surprising inside a debugger. Ideally we could convert this to using the async return value mechanism to simplify things while eliminating the extra callback, but there are two slightly complicating factors that make this not an entirely rote mechanical change: - There is a slight conceptual plumbing change as our NS_FAILED case actually would be a resolved value from IPC, not a rejected value (which would correspond to ActorDestroy without ever having received a value). - Currently we use a discriminated union for the 4 different payloads. Ideally these would be their own async return value calls to reduce obfuscation, but there's also a bit of machinery related to the `EnsureSource()` mechanism and effectively forwarding the requests onward to the ClientSourceParent, so there would be some code duplication or template shenanigans. That said, we do already have some [template shenanigans for DoSourceOp](https://searchfox.org/mozilla-central/rev/5f037f0b930de92a65ced0831b0f199281ac4df3/dom/clients/manager/ClientSourceOpChild.h#32-33).