Bug 215263

Summary: [MSE][GStreamer] Remove m_sourceBufferPrivateClient checks in SourceBufferPrivateGStreamer
Product: WebKit Reporter: Alicia Boya García <aboya>
Component: WebKitGTKAssignee: Alicia Boya García <aboya>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, cgarcia, ews-watchlist, gustavo, menard, pnormand, vjaquez
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Alicia Boya García 2020-08-07 05:10:24 PDT
m_sourceBufferPrivateClient is only reset to NULL from SourceBuffer's
destructor. At this point SourceBufferPrivateGStreamer is about to
receive its last unref by SourceBuffer and therefore be destroyed.

Similarly, there is no need to check for m_mediaSource being null.
m_mediaSource is only reset when the SourceBuffer is removed, and at
that point SourceBufferPrivate shouldn't receive any calls.

This is a clean-up and doesn't introduce new behavior. Asserts have
been added checking the precondition above.
Comment 1 Alicia Boya García 2020-08-07 05:28:01 PDT
Created attachment 406164 [details]
Patch
Comment 2 Alicia Boya García 2020-08-07 05:54:22 PDT
Created attachment 406166 [details]
Patch
Comment 3 Xabier Rodríguez Calvar 2020-08-11 01:44:19 PDT
Comment on attachment 406166 [details]
Patch

I would be quieter if we ASSERT on the client before calling it.
Comment 4 Alicia Boya García 2020-08-11 01:52:37 PDT
(In reply to Xabier Rodríguez Calvar from comment #3)
> Comment on attachment 406166 [details]
> Patch
> 
> I would be quieter if we ASSERT on the client before calling it.

The way I see it, there is already an implicit assert there, built in the CPU: the pointer indirection will segfault (and crash, just like with asserts) in that same line if m_sourceBufferPrivateClient is NULL.

Since it's the only pointer being indirected in that line, or the function even, an assert doesn't give us any more specificity even when looking at the traceback.
Comment 5 EWS 2020-08-11 04:41:16 PDT
Committed r265494: <https://trac.webkit.org/changeset/265494>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406166 [details].