Bug 191759

Summary: [MSE][GStreamer] Refactor AppendPipeline deinitialization
Product: WebKit Reporter: Alicia Boya García <aboya>
Component: WebKitGTKAssignee: Alicia Boya García <aboya>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, commit-queue, eocanha
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Alicia Boya García 2018-11-16 11:06:30 PST
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.
Comment 1 Alicia Boya García 2018-11-16 11:09:19 PST
Created attachment 355078 [details]
Patch
Comment 2 Xabier Rodríguez Calvar 2018-11-18 22:33:17 PST
Patch looks ok to me. Anyway, I'd like Enrique to have a quick look at this as well.
Comment 3 Enrique Ocaña 2018-11-21 00:12:48 PST
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 4 WebKit Commit Bot 2018-11-21 00:49:46 PST
Comment on attachment 355078 [details]
Patch

Clearing flags on attachment: 355078

Committed r238412: <https://trac.webkit.org/changeset/238412>
Comment 5 WebKit Commit Bot 2018-11-21 00:49:47 PST
All reviewed patches have been landed.  Closing bug.