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
Created attachment 355078 [details]
Patch looks ok to me. Anyway, I'd like Enrique to have a quick look at this as well.
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.
Comment on attachment 355078 [details]
Clearing flags on attachment: 355078
Committed r238412: <https://trac.webkit.org/changeset/238412>
All reviewed patches have been landed. Closing bug.