Bug 137905 – Use std::unique_ptr | std::make_unique in FontCacheFoo

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Gyuyoung Kim]

SSIA

[Gyuyoung Kim]

Created attachment 240171 [details]
Patch

[WebKit Commit Bot]

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

Created attachment 240181 [details]
Patch

[Gyuyoung Kim]

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

Created attachment 240183 [details]
Patch

[Darin Adler]

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]

Created attachment 240229 [details]
Patch for landing

[Gyuyoung Kim]

(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 on attachment 240229 [details]
Patch for landing

Clearing flags on attachment: 240229

Committed r175013: <http://trac.webkit.org/changeset/175013>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Tim Horton]

Hopeful iOS build fix in http://trac.webkit.org/changeset/175017.

[Gyuyoung Kim]

(In reply to comment #12)
> Hopeful iOS build fix in http://trac.webkit.org/changeset/175017.

Oops, thank you for fixing it.