RESOLVED FIXED 137905
Use std::unique_ptr | std::make_unique in FontCacheFoo
https://bugs.webkit.org/show_bug.cgi?id=137905
Summary Use std::unique_ptr | std::make_unique in FontCacheFoo
Gyuyoung Kim
Reported 2014-10-20 18:19:38 PDT
SSIA
Attachments
Patch (11.30 KB, patch)
2014-10-20 18:43 PDT, Gyuyoung Kim
no flags
Patch (10.18 KB, patch)
2014-10-20 20:44 PDT, Gyuyoung Kim
no flags
Patch (10.47 KB, patch)
2014-10-20 21:30 PDT, Gyuyoung Kim
no flags
Patch for landing (10.45 KB, patch)
2014-10-21 16:54 PDT, Gyuyoung Kim
no flags
Gyuyoung Kim
Comment 1 2014-10-20 18:43:26 PDT
WebKit Commit Bot
Comment 2 2014-10-20 18:44:12 PDT
Attachment 240171 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/mac/FontCacheMac.mm:243: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Darin Adler
Comment 3 2014-10-20 18:58:13 PDT
Comment on attachment 240171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240171&action=review > Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:87 > +std::unique_ptr<TextLayout> Font::createLayout(RenderText* text, float xPos, bool collapseWhiteSpace) const Need to update this in Font.h too. > Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:91 > + return std::make_unique<TextLayout>(text, *this, xPos)); Extra end parenthesis on this line. > Source/WebCore/platform/graphics/mac/FontCacheMac.mm:243 > + auto platformData = std::make_unique<FontPlatformData>(platformFont, size, fontDescription.usePrinterFont(), syntheticBold, syntheticOblique, fontDescription.orientation(), fontDescription.widthVariant())); > + return platformData; No need for a local variable here. Just return make_unique.
Gyuyoung Kim
Comment 4 2014-10-20 20:44:12 PDT
Gyuyoung Kim
Comment 5 2014-10-20 20:47:29 PDT
(In reply to comment #3) > Comment on attachment 240171 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240171&action=review > > > Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:87 > > +std::unique_ptr<TextLayout> Font::createLayout(RenderText* text, float xPos, bool collapseWhiteSpace) const > > Need to update this in Font.h too. It is hard to touch createLayout() in this patch, because Font::createLayout() of Font.cpp generates build error as below, std::unique_ptr<TextLayout> Font::createLayout(RenderText*, float, bool) const { return nullptr; } /platform/graphics/Font.cpp.o -c ../../Source/WebCore/platform/graphics/Font.cpp In file included from /usr/include/c++/4.8/memory:81:0, from ../../Source/WTF/wtf/StdLibExtras.h:31, from ../../Source/WTF/wtf/FastMalloc.h:27, from ../../Source/WebCore/config.h:75, from ../../Source/WebCore/platform/graphics/Font.cpp:24: /usr/include/c++/4.8/bits/unique_ptr.h: In instantiation of ‘void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = WebCore::TextLayout]’: /usr/include/c++/4.8/bits/unique_ptr.h:184:16: required from ‘std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = WebCore::TextLayout; _Dp = std::default_delete<WebCore::TextLayout>]’ /usr/include/c++/4.8/bits/unique_ptr.h:157:61: required from ‘constexpr std::unique_ptr<_Tp, _Dp>::unique_ptr(std::nullptr_t) [with _Tp = WebCore::TextLayout; _Dp = std::default_delete<WebCore::TextLayout>; std::nullptr_t = std::nullptr_t]’ ../../Source/WebCore/platform/graphics/Font.cpp:420:12: required from here /usr/include/c++/4.8/bits/unique_ptr.h:65:22: error: invalid application of ‘sizeof’ to incomplete type ‘WebCore::TextLayout’ static_assert(sizeof(_Tp)>0, I would like to handle createLayout() in new bug. > > Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:91 > > + return std::make_unique<TextLayout>(text, *this, xPos)); > > Extra end parenthesis on this line. > > > Source/WebCore/platform/graphics/mac/FontCacheMac.mm:243 > > + auto platformData = std::make_unique<FontPlatformData>(platformFont, size, fontDescription.usePrinterFont(), syntheticBold, syntheticOblique, fontDescription.orientation(), fontDescription.widthVariant())); > > + return platformData; > > No need for a local variable here. Just return make_unique. I'm sorry, I missed to love mac port files.
Gyuyoung Kim
Comment 6 2014-10-20 21:30:33 PDT
Darin Adler
Comment 7 2014-10-21 08:45:30 PDT
Comment on attachment 240183 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240183&action=review > Source/WebCore/platform/graphics/win/FontCacheWin.cpp:589 > + result = nullptr; Wait, I didn’t notice. This line of code is not needed and can just be omitted.
Gyuyoung Kim
Comment 8 2014-10-21 16:54:31 PDT
Created attachment 240229 [details] Patch for landing
Gyuyoung Kim
Comment 9 2014-10-21 16:55:23 PDT
(In reply to comment #7) > Comment on attachment 240183 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=240183&action=review > > > Source/WebCore/platform/graphics/win/FontCacheWin.cpp:589 > > + result = nullptr; > > Wait, I didn’t notice. This line of code is not needed and can just be > omitted. Ok, removed it.
WebKit Commit Bot
Comment 10 2014-10-21 18:53:05 PDT
Comment on attachment 240229 [details] Patch for landing Clearing flags on attachment: 240229 Committed r175013: <http://trac.webkit.org/changeset/175013>
WebKit Commit Bot
Comment 11 2014-10-21 18:53:09 PDT
All reviewed patches have been landed. Closing bug.
Tim Horton
Comment 12 2014-10-21 19:57:24 PDT
Hopeful iOS build fix in http://trac.webkit.org/changeset/175017.
Gyuyoung Kim
Comment 13 2014-10-21 20:44:09 PDT
(In reply to comment #12) > Hopeful iOS build fix in http://trac.webkit.org/changeset/175017. Oops, thank you for fixing it.
Note You need to log in before you can comment on or make changes to this bug.