Summary: | Use std::unique_ptr | std::make_unique in FontCacheFoo | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||
Component: | WebCore Misc. | Assignee: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, mmaxfield, thorton | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 128007 | ||||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2014-10-20 18:19:38 PDT
Created attachment 240171 [details]
Patch
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.
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. Created attachment 240181 [details]
Patch
(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. Created attachment 240183 [details]
Patch
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. Created attachment 240229 [details]
Patch for landing
(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. Comment on attachment 240229 [details] Patch for landing Clearing flags on attachment: 240229 Committed r175013: <http://trac.webkit.org/changeset/175013> All reviewed patches have been landed. Closing bug. Hopeful iOS build fix in http://trac.webkit.org/changeset/175017. (In reply to comment #12) > Hopeful iOS build fix in http://trac.webkit.org/changeset/175017. Oops, thank you for fixing it. |