WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
137738
Move TextCodec classes to std::unique_ptr
https://bugs.webkit.org/show_bug.cgi?id=137738
Summary
Move TextCodec classes to std::unique_ptr
Gyuyoung Kim
Reported
2014-10-15 03:17:17 PDT
SSIA
Attachments
Patch
(12.29 KB, patch)
2014-10-15 03:18 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(13.25 KB, patch)
2014-10-15 04:18 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch for landing
(13.78 KB, patch)
2014-10-15 18:06 PDT
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch for landing
(13.86 KB, patch)
2014-10-15 18:35 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-15 03:18:04 PDT
Created
attachment 239861
[details]
Patch
Gyuyoung Kim
Comment 2
2014-10-15 04:18:32 PDT
Created
attachment 239864
[details]
Patch
Darin Adler
Comment 3
2014-10-15 09:39:31 PDT
Comment on
attachment 239864
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=239864&action=review
OK to do this, but please use make_unique, not unique_ptr/new.
> Source/WebCore/platform/text/TextCodecICU.cpp:72 > + return std::unique_ptr<TextCodec>(new TextCodecICU(encoding.name(), static_cast<const char*>(additionalData)));
This should use std::make_unique, not new combined with std::unique_ptr.
> Source/WebCore/platform/text/TextCodecLatin1.cpp:109 > + return std::unique_ptr<TextCodec>(new TextCodecLatin1);
Ditto.
> Source/WebCore/platform/text/TextCodecUTF16.cpp:53 > + return std::unique_ptr<TextCodec>(new TextCodecUTF16(true));
Ditto.
> Source/WebCore/platform/text/TextCodecUTF16.cpp:58 > + return std::unique_ptr<TextCodec>(new TextCodecUTF16(false));
Ditto.
> Source/WebCore/platform/text/TextCodecUTF8.cpp:43 > + return std::unique_ptr<TextCodec>(new TextCodecUTF8);
Ditto.
> Source/WebCore/platform/text/TextCodecUserDefined.cpp:44 > + return std::unique_ptr<TextCodec>(new TextCodecUserDefined);
Ditto.
> Source/WebCore/platform/text/mac/TextCodecMac.cpp:71 > + return std::unique_ptr<TextCodec>(new TextCodecMac(*static_cast<const TECTextEncodingID*>(additionalData)));
Ditto.
Gyuyoung Kim
Comment 4
2014-10-15 18:06:48 PDT
Created
attachment 239912
[details]
Patch for landing
Gyuyoung Kim
Comment 5
2014-10-15 18:23:09 PDT
(In reply to
comment #3
)
> (From update of
attachment 239864
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=239864&action=review
> > OK to do this, but please use make_unique, not unique_ptr/new. > > > Source/WebCore/platform/text/TextCodecICU.cpp:72 > > + return std::unique_ptr<TextCodec>(new TextCodecICU(encoding.name(), static_cast<const char*>(additionalData))); > > This should use std::make_unique, not new combined with std::unique_ptr. > > > Source/WebCore/platform/text/TextCodecLatin1.cpp:109 > > + return std::unique_ptr<TextCodec>(new TextCodecLatin1); > > Ditto. > > > Source/WebCore/platform/text/TextCodecUTF16.cpp:53 > > + return std::unique_ptr<TextCodec>(new TextCodecUTF16(true)); > > Ditto. > > > Source/WebCore/platform/text/TextCodecUTF16.cpp:58 > > + return std::unique_ptr<TextCodec>(new TextCodecUTF16(false)); > > Ditto. > > > Source/WebCore/platform/text/TextCodecUTF8.cpp:43 > > + return std::unique_ptr<TextCodec>(new TextCodecUTF8); > > Ditto. > > > Source/WebCore/platform/text/TextCodecUserDefined.cpp:44 > > + return std::unique_ptr<TextCodec>(new TextCodecUserDefined); > > Ditto. > > > Source/WebCore/platform/text/mac/TextCodecMac.cpp:71 > > + return std::unique_ptr<TextCodec>(new TextCodecMac(*static_cast<const TECTextEncodingID*>(additionalData))); > > Ditto.
Thank you for pointing those things out. Fixed all.
Gyuyoung Kim
Comment 6
2014-10-15 18:35:07 PDT
Created
attachment 239914
[details]
Patch for landing
WebKit Commit Bot
Comment 7
2014-10-15 19:51:00 PDT
Comment on
attachment 239914
[details]
Patch for landing Clearing flags on attachment: 239914 Committed
r174757
: <
http://trac.webkit.org/changeset/174757
>
WebKit Commit Bot
Comment 8
2014-10-15 19:51:04 PDT
All reviewed patches have been landed. Closing bug.
Chris Dumez
Comment 9
2014-10-16 21:32:31 PDT
Comment on
attachment 239914
[details]
Patch for landing View in context:
https://bugs.webkit.org/attachment.cgi?id=239914&action=review
> Source/WebCore/platform/text/TextCodecICU.h:39 > + static std::unique_ptr<TextCodec> create(const TextEncoding&, const void* additionalData);
It is a bit sad that we had to make all these public :(
> Source/WebCore/platform/text/TextCodecUTF8.h:36 > + TextCodecUTF8()
ditto.
Gyuyoung Kim
Comment 10
2014-10-16 22:48:37 PDT
(In reply to
comment #9
)
> Comment on
attachment 239914
[details]
> Patch for landing > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=239914&action=review
> > > Source/WebCore/platform/text/TextCodecICU.h:39 > > + static std::unique_ptr<TextCodec> create(const TextEncoding&, const void* additionalData); > > It is a bit sad that we had to make all these public :( > > > Source/WebCore/platform/text/TextCodecUTF8.h:36 > > + TextCodecUTF8() > > ditto.
I will also fix it after finishing to land the patch on
Bug 137765
.
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