1480366 - AbortFollower must traverse/unlink mAbortFollowing object

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Andrea Marchesini [:baku]]

Follow up for bug 1478101

Comment 1

[Andrea Marchesini [:baku]]

Attachment: abort_3.patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8996961 [details] [diff] [review]
abort_3.patch

I don't understand why we need the complexity here.  Why can't we traverse/unlink an AbortSignalImpl?

Is the issue that an AbortSignalImpl might live on a different thread?

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Is the issue that an AbortSignalImpl might live on a different thread?

Looks like no... those are single-threaded objects.  In that case, I really don't understand why we have all this complexity.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8996961 [details] [diff] [review]
abort_3.patch

This presumably needs updating on top of bug 1478101.

Comment 5

[Andrea Marchesini [:baku]]

Attachment: abort_3.patch

Smaug, do you mind to take a look here? bz is on PTO.
Note that there are other classes inheriting AbortSignal but they are not CCed and they should not traverse/unlink internal members. Here the list of these classes:

1. FetchBody - This is covered by this patch because FetchBody is Request/Response.

2. WebAuthManager - used only during the webauth process.
https://searchfox.org/mozilla-central/rev/e126996d9b0a3d7653de205898517f4f5b632e7f/dom/webauthn/WebAuthnManager.h#74

3. FetchObserver - covered by this patch.

4. FetchDriver - used only during the fetch operation. https://searchfox.org/mozilla-central/rev/e126996d9b0a3d7653de205898517f4f5b632e7f/dom/fetch/FetchDriver.h#99

5. FetchBodyConsumer - only used during the fetch operation. https://searchfox.org/mozilla-central/rev/e126996d9b0a3d7653de205898517f4f5b632e7f/dom/fetch/FetchConsumer.h#32

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

(in general I'm worried about the memory management around WebAuthManager, but that isn't about this bug. Pinged jcj. Perhaps it is ok, but I'm just missing something.)

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9004174 [details] [diff] [review]
abort_3.patch

How can we make this setup less error prone. Currently it is just good luck if the implementer of the inheriting class remembers to traverse/unlink the member variable.

Could we make AbortFollower template class, and the template param would somehow hint that inheriting class needs to explicitly decide whether or not traverse/unlink?

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/44e3fc29c49f
Traverse/Unlink mAbortFollowing in any object exposed to content, r=smaug

Comment 9

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/44e3fc29c49f