| Summary: | [GTK] Cleanup PasteboardHelper | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||
| Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | berto, commit-queue, gustavo, mcatanzaro, mrobinson, zan | ||||||
| Priority: | P2 | Keywords: | Gtk | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Carlos Garcia Campos
2015-07-17 00:59:18 PDT
Created attachment 256960 [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 Comment on attachment 256960 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256960&action=review > Source/WebCore/ChangeLog:11 > + PasteboardHelper::singleton() and make it non-copiable, fast Nit: non-copyable. > Source/WebCore/platform/gtk/PasteboardHelper.cpp:70 > + gMarkupPrefix = String::fromUTF8("<meta http-equiv=\"content-type\" content=\"text/html; charset=utf-8\">"); Not terribly important since it's only called once, but this string seems ASCII-only, so you could initialize gMarkipPrefix from an ASCIILiteral. Also, can this be initialized where it's declared? > Source/WebCore/platform/gtk/PasteboardHelper.h:38 > + WTF_MAKE_NONCOPYABLE(PasteboardHelper); WTF_MAKE_FAST_ALLOCATED; No need for WTF_MAKE_FAST_ALLOCATED. There's only one object alive during the program's lifetime, and that one is constructed in memory that's reserved by the static NeverDestroyed<PasteboardHelper> object. Created attachment 256962 [details]
Updated patch
Committed r186954: <http://trac.webkit.org/changeset/186954> |