Bug 191759 – [MSE][GStreamer] Refactor AppendPipeline deinitialization

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alicia Boya García]

AppendPipeline currently has a method, clearPlayerPrivate(), that the
client code uses to deinitialize most of the AppendPipeline... just
before actually destructing it in the next line of code.

Since at that point there should not be alive RefPtr's pointing to the
AppendPipeline there is no need for this kind of pre-deinitialization
in this usage pattern. Instead, we can just rely on C++ destructors,
cleaning the code a bit and removing the potential for the question
"what if `clearPlayerPrivate() has been called before?`": it has not.

Assertions have been added to ensure that there is only one alive
RefPtr pointing to AppendPipeline, therefore guaranteeing its immediate
destruction.

[Alicia Boya García]

Created attachment 355078 [details]
Patch

[Xabier Rodríguez Calvar]

Patch looks ok to me. Anyway, I'd like Enrique to have a quick look at this as well.

[Enrique Ocaña]

The patch looks good, but I suspected about problems coming from other threads still processing appends (it happened in the past).

Alicia's explanations about how the new AbortableTaskQueue would prevent non-main threads from accessing the append pipeline when it's being destroyed convinced me. She also tested player destruction in a 100 iterations loop while appends are still ongoing and didn't detect any problem related to the pipeline destruction.

I think the patch is good to be committed.

[WebKit Commit Bot]

Comment on attachment 355078 [details]
Patch

Clearing flags on attachment: 355078

Committed r238412: <https://trac.webkit.org/changeset/238412>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.