[webkit-reviews] review granted: [Bug 78990] Add an empty skeleton of KURL for WTFURL : [Attachment 127729] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 78990: Add an empty skeleton of KURL for WTFURL
https://bugs.webkit.org/show_bug.cgi?id=78990

Attachment 127729: Patch
https://bugs.webkit.org/attachment.cgi?id=127729&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=127729&action=review


This looks fine except for the RefPtr versus plain data member issue.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:-22323
> -				BAB22AD414B7A02D00D8ABA6 /*
FractionalLayoutUnit.h in Headers */,

Is this change spurious?

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:24288
>				144FCE5D14EC79E7000D17A3 /*
FractionalLayoutSize.h in Headers */,
> +				26A5034E14F0983600AA730D /* KURLWTFURLImpl.h in
Headers */,

Please run sort-xcodeproj before landing.

> Source/WebCore/platform/KURL.h:249
> +    RefPtr<KURLWTFURLImpl> m_urlImpl;

I'm not sure we want to use a RefPtr here.  That will be an extra allocation
for each KURL, which seems like it would affect performance.  Notice that
KURLGooglePrivate is just a plain data member.

> Source/WebCore/platform/KURLWTFURLImpl.h:37
> +class KURLWTFURLImpl : public RefCounted<KURLWTFURLImpl> {

KURLWTFURLImpl ...  Oh man.  What a name.