WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(10.18 KB, patch)
2014-10-20 20:44 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(10.47 KB, patch)
2014-10-20 21:30 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch for landing
(10.45 KB, patch)
2014-10-21 16:54 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Gyuyoung Kim
Comment 1
2014-10-20 18:43:26 PDT
Created
attachment 240171
[details]
Patch
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
Created
attachment 240181
[details]
Patch
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
Created
attachment 240183
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug