Bug 127431 – [GLIB] Use GUniquePtr instead of GOwnPtr

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

[Carlos Garcia Campos]

GUniquePtr is just a template alias of std::unique_ptr with a custo deleter that replaces GOwnPtr.

[Carlos Garcia Campos]

Created attachment 221873 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[WebKit Commit Bot]

Attachment 221873 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:446:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:446:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:447:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 3 in 121 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Created attachment 221874 [details]
Updated patch

Sorry, I forgot to git add a file.

[WebKit Commit Bot]

Attachment 221874 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:446:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:446:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:447:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 3 in 122 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 221874 [details]
Updated patch

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

> Source/WebCore/platform/gtk/GOwnPtrGtk.cpp:-31
> -template <> void freeOwnedGPtr<GdkEvent>(GdkEvent* ptr)
> -{
> -    if (ptr)
> -        gdk_event_free(ptr);
> -}
> -

A specialization isn't necessary for GUniquePtr?

[Carlos Garcia Campos]

(In reply to comment #6)
> (From update of attachment 221874 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=221874&action=review
> 
> > Source/WebCore/platform/gtk/GOwnPtrGtk.cpp:-31
> > -template <> void freeOwnedGPtr<GdkEvent>(GdkEvent* ptr)
> > -{
> > -    if (ptr)
> > -        gdk_event_free(ptr);
> > -}
> > -
> 
> A specialization isn't necessary for GUniquePtr?

Yes, good catch! I forgot to git add another file :-P

[Carlos Garcia Campos]

Created attachment 221876 [details]
Updated patch

[WebKit Commit Bot]

Attachment 221876 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:446:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:446:  Comma should be at the beginning of the line in a member initialization list.  [whitespace/init] [4]
ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:447:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 3 in 123 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 221876 [details]
Updated patch

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

This is a very large patch, but appears to be largely mechanical.

> Source/WebCore/platform/network/soup/GUniquePtrSoup.h:2
> + *  Copyright (C) 2010 Igalia S.L

Probably want to use 2014 here.

> Source/WebCore/platform/network/soup/GUniquePtrSoup.h:24
> +#include <libsoup/soup.h>
> +#include <wtf/gobject/GUniquePtr.h>

One disadvantage of this is that we include third-party headers in more places, so this has the potential to impact compilation time. I suppose that isn't a *huge* issue though.

[Carlos Garcia Campos]

(In reply to comment #10)
> (From update of attachment 221876 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=221876&action=review
> 
> This is a very large patch, but appears to be largely mechanical.

Yeah, thanks for the quick review.

> > Source/WebCore/platform/network/soup/GUniquePtrSoup.h:2
> > + *  Copyright (C) 2010 Igalia S.L
> 
> Probably want to use 2014 here.

Sure.

> > Source/WebCore/platform/network/soup/GUniquePtrSoup.h:24
> > +#include <libsoup/soup.h>
> > +#include <wtf/gobject/GUniquePtr.h>
> 
> One disadvantage of this is that we include third-party headers in more places, so this has the potential to impact compilation time. I suppose that isn't a *huge* issue though.

Yes, I thought about that too, the advantage is that you can use those just including the header. I haven't noticed any regression in the build time, but I'll check if we can improve it anyway.

[Carlos Garcia Campos]

Committed r162599: <http://trac.webkit.org/changeset/162599>