Summary: | [GLIB] Add GUniqueOutPtr and use it instead of GOwnPtr | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||||||
Component: | Web Template Framework | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue | ||||||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2014-01-24 04:25:49 PST
Created attachment 222092 [details]
Patch
Attachment 222092 [details] did not pass style-queue:
ERROR: Source/WebKit2/NetworkProcess/soup/NetworkProcessSoup.cpp:40: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebKit/gtk/webkit/webkitdownload.cpp:42: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WTF/wtf/gobject/GUniquePtr.h:64: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4]
ERROR: Source/WebKit2/WebProcess/soup/WebProcessSoup.cpp:44: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:32: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 5 in 56 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 222093 [details]
Fixed coding style issues
Comment on attachment 222093 [details] Fixed coding style issues View in context: https://bugs.webkit.org/attachment.cgi?id=222093&action=review > Tools/TestWebKitAPI/Tests/WTF/gobject/GUniquePtr.cpp:188 > + a.reset(); No need for calling reset() here -- the GUniqueOutPtr will handle freeing the memory when it goes out of scope and is destroyed. > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebExtensions.cpp:146 > - result.clear(); > + result.reset(); No need for calling reset() here (since the unique pointer will be reset when outPtr() is called the next time) ... > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebExtensions.cpp:168 > - result.clear(); > + result.reset(); ... or here (since the pointer will be deleted when GUniqueOutPtr goes out of scope). I guess GUniqueOutPtr is now smart enough that the user doesn't really have to know about the reset() method, so it can be made private. Comment on attachment 222093 [details] Fixed coding style issues View in context: https://bugs.webkit.org/attachment.cgi?id=222093&action=review Thanks for the review. >> Tools/TestWebKitAPI/Tests/WTF/gobject/GUniquePtr.cpp:188 >> + a.reset(); > > No need for calling reset() here -- the GUniqueOutPtr will handle freeing the memory when it goes out of scope and is destroyed. Well, this is actually a test to check that reset works, if we make it private we don't need this test. >> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebExtensions.cpp:146 >> + result.reset(); > > No need for calling reset() here (since the unique pointer will be reset when outPtr() is called the next time) ... You are right. >> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebExtensions.cpp:168 >> + result.reset(); > > ... or here (since the pointer will be deleted when GUniqueOutPtr goes out of scope). > > I guess GUniqueOutPtr is now smart enough that the user doesn't really have to know about the reset() method, so it can be made private. Yes, I agree, and the proof is that it's only used in this test and it's not actually needed Created attachment 222435 [details]
Updated patch
Made reset private as suggested by Zan
Created attachment 222574 [details]
Rebased patch
Created attachment 223627 [details]
Rebased patch
Comment on attachment 223627 [details] Rebased patch View in context: https://bugs.webkit.org/attachment.cgi?id=223627&action=review > Source/WebCore/platform/gtk/PasteboardHelper.cpp:-272 > - // FIXME: should GOwnPtr be smarter about this and replace the existing ptr when outPtr() is used? > - key.clear(); > - value.clear(); \o/ Committed r163797: <http://trac.webkit.org/changeset/163797> |