Bug 139019 – Use std::unique_ptr instead of PassOwnPtr|OwnPtr for Pasteboard

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Joonghun Park]

Change from PassOwnPtr|OwnPtr to std::unique_ptr for Pasteboard in All port(Mac, IOS, Efl, Gtk, Win).

[Joonghun Park]

Created attachment 242144 [details]
Patch

[Joonghun Park]

Created attachment 242146 [details]
Patch

[Gyuyoung Kim]

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<>

[Joonghun Park]

Created attachment 242151 [details]
<WIP>

[Joonghun Park]

Created attachment 242153 [details]
<WIP>

[Joonghun Park]

Created attachment 242157 [details]
Patch

[Joonghun Park]

Created attachment 242159 [details]
Patch

[Gyuyoung Kim]

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.

[Joonghun Park]

Created attachment 242163 [details]
Patch

[Joonghun Park]

Created attachment 242164 [details]
Patch

[Gyuyoung Kim]

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.

[Darin Adler]

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.

[Darin Adler]

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.

[Joonghun Park]

Created attachment 242425 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 242425 [details]
Patch for landing

Clearing flags on attachment: 242425

Committed r176780: <http://trac.webkit.org/changeset/176780>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.