Bug 184533 – [GStreamer] Convert GStreamerUtilities in GStreamerCommon and include the GStreamer smart pointer traits

• Component: New Bugs
• Status: RESOLVED FIXED

[Xabier Rodríguez Calvar]

[GStreamer] Convert GStreamerUtilities in GStreamerCommon and include the GStreamer smart pointer traits

[Xabier Rodríguez Calvar]

Created attachment 337789 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 337789 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=337789&action=review

> Source/WebCore/ChangeLog:13
> +

Why the rename? I'm not sure I get the new name Common, common to what?

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:24
> +#include "GStreamerCommon.h"

The header is already guarded, so it can be moved right after config.h as suggested by the style guidelines.

[Xabier Rodríguez Calvar]

(In reply to Carlos Garcia Campos from comment #2)
> Why the rename? I'm not sure I get the new name Common, common to what?

Because I want to include the smart pointer headers there and that way is no longer a utility file, but something highly recommendable.

> > Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:24
> > +#include "GStreamerCommon.h"
> 
> The header is already guarded, so it can be moved right after config.h as
> suggested by the style guidelines.

The style checker didn't mention it :/ Could I fix this during landing if you find the rest ok?

[Carlos Garcia Campos]

(In reply to Xabier Rodríguez Calvar from comment #3)
> (In reply to Carlos Garcia Campos from comment #2)
> > Why the rename? I'm not sure I get the new name Common, common to what?
> 
> Because I want to include the smart pointer headers there and that way is no
> longer a utility file, but something highly recommendable.

And smart pointers are not utilities? I find the rename confusing and unnecessary, but I won't oppose if it makes sense for others.

> > > Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:24
> > > +#include "GStreamerCommon.h"
> > 
> > The header is already guarded, so it can be moved right after config.h as
> > suggested by the style guidelines.
> 
> The style checker didn't mention it :/ Could I fix this during landing if
> you find the rest ok?

Sure, it's just a nit, I didn't mean to r- this patch at all.

[Xabier Rodríguez Calvar]

(In reply to Carlos Garcia Campos from comment #4)
> And smart pointers are not utilities? I find the rename confusing and
> unnecessary, but I won't oppose if it makes sense for others.

Smart pointers are utilities but if you use a smart pointer without the proper deleter/ref/unref declared, you get trouble. For that you need to declare the proper deleter/ref/unref that happens to be in the G*PtrGStreamer.h files. I want to give the impression that is file is not just about utilities that you can use or not, it is something that you should include. That's why went thru the pain of changing the name.

[Xabier Rodríguez Calvar]

Created attachment 337872 [details]
Patch

Style fix

[WebKit Commit Bot]

Comment on attachment 337872 [details]
Patch

Clearing flags on attachment: 337872

Committed r230625: <https://trac.webkit.org/changeset/230625>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/39407508>