Summary: | [GLIB] Use GUniquePtr instead of GOwnPtr | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, bunhere, cdumez, cfleizach, commit-queue, danw, dmazzoni, eric.carlson, glenn, gustavo, gyuyoung.kim, jcraig, jdiggs, jer.noble, mario, menard, mrobinson, pnormand, rakuco, samuel_white | ||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2014-01-22 10:27:32 PST
Created attachment 221873 [details]
Patch
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 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.
Created attachment 221874 [details]
Updated patch
Sorry, I forgot to git add a file.
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.
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? (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 Created attachment 221876 [details]
Updated patch
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.
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. (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. Committed r162599: <http://trac.webkit.org/changeset/162599> |