Summary: | Use std::unique_ptr instead of PassOwnPtr|OwnPtr for Pasteboard | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joonghun Park <jh718.park> | ||||||||||||||||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | commit-queue, darin, gyuyoung.kim | ||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||
Bug Blocks: | 128007 | ||||||||||||||||||||||
Attachments: |
|
Description
Joonghun Park
2014-11-23 20:19:48 PST
Created attachment 242144 [details]
Patch
Created attachment 242146 [details]
Patch
Comment on attachment 242146 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242146&action=review r- because of create() factory function and wrong std::unique_ptr<> use. > Source/WebCore/platform/mac/PasteboardMac.mm:128 > + return std::unique_ptr<Pasteboard>(new Pasteboard(pasteboardName)); We decided not to use create() factory function. Instead we have to use std::make_unique<> except for specific cases. > Source/WebCore/platform/win/PasteboardWin.cpp:114 > + return std::unique_ptr<Pasteboard>(new Pasteboard(dataObject.get())); Use std::make_unique<> instead of std::unique_ptr<> Created attachment 242151 [details]
<WIP>
Created attachment 242153 [details]
<WIP>
Created attachment 242157 [details]
Patch
Created attachment 242159 [details]
Patch
Comment on attachment 242159 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242159&action=review > Source/WebCore/editing/Editor.cpp:860 > + std::unique_ptr<Pasteboard> pasteboard = Pasteboard::createForCopyAndPaste(); Why not using auto ? > Source/WebCore/page/mac/EventHandlerMac.mm:684 > + std::unique_ptr<Pasteboard> pasteboard = return std::make_unique<Pasteboard>(NSDragPboard); return ?????????? > Source/WebCore/platform/win/PasteboardWin.cpp:95 > + std::unique_ptr<Pasteboard> pasteboard = std::make_unique<Pasteboard>(); ditto. Created attachment 242163 [details]
Patch
Created attachment 242164 [details]
Patch
Comment on attachment 242164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242164&action=review > Source/WebCore/platform/Pasteboard.h:183 > + static std::unique_ptr<Pasteboard> createForDragAndDrop(); There is a concern we support both public constructor and createFoo() factory function. Some reviewers think this is wrong behavior. So, we need to try to remove createFoo() factory function. Comment on attachment 242164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242164&action=review >> Source/WebCore/platform/Pasteboard.h:183 >> + static std::unique_ptr<Pasteboard> createForDragAndDrop(); > > There is a concern we support both public constructor and createFoo() factory function. Some reviewers think this is wrong behavior. So, we need to try to remove createFoo() factory function. That’s not quite right. We don’t to have create functions that simply wrap std::make_unique and add nothing further. But if the create function adds some additional value, then it’s fine to have it. Comment on attachment 242164 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=242164&action=review > Source/WebCore/platform/Pasteboard.h:147 > + Pasteboard(PassRefPtr<DataObjectGtk>); > + Pasteboard(GtkClipboard*); These should be marked explicit. Not sure why they weren’t already. Created attachment 242425 [details]
Patch for landing
Comment on attachment 242425 [details] Patch for landing Clearing flags on attachment: 242425 Committed r176780: <http://trac.webkit.org/changeset/176780> All reviewed patches have been landed. Closing bug. |