[webkit-reviews] review denied: [Bug 179504] [PAL] Move WebCoreNSStringExtras and WebCoreObjCExtras into PAL : [Attachment 326604] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Christopher Reid
<christopher.reid at am.sony.com>'s request for review:
Bug 179504: [PAL] Move WebCoreNSStringExtras and WebCoreObjCExtras into PAL
https://bugs.webkit.org/show_bug.cgi?id=179504

Attachment 326604: Patch

https://bugs.webkit.org/attachment.cgi?id=326604&action=review




--- Comment #7 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 326604
  --> https://bugs.webkit.org/attachment.cgi?id=326604
Patch

> FileSystemMac has a dependency on WebCoreNSStringExtras.

Looking at WebCoreNSStringExtras.h, I don't see anything worth moving as is.
These files seem like remnants of some past code move, in an effort to share
code between WebCore and WebKitLegacy that didn't quite work.

There are several groups of unrelated functions in this file:

WEBCORE_EXPORT BOOL stringIsCaseInsensitiveEqualToString(NSString *first,
NSString *second);
WEBCORE_EXPORT BOOL hasCaseInsensitiveSuffix(NSString *, NSString *suffix);
WEBCORE_EXPORT BOOL hasCaseInsensitivePrefix(NSString *, NSString *prefix);
WEBCORE_EXPORT BOOL hasCaseInsensitiveSubstring(NSString *, NSString
*substring);

These are barely used one-liners. It would likely be beneficial to rewrite the
only call site in WebCore to use String instead.

WEBCORE_EXPORT NSString *filenameByFixingIllegalCharacters(NSString *);

This is a file system specific function, not a legitimate "NSString extra".
This one is used in WebCore, WebKit and WebKitLegacy, so it may be harder to
eliminate, but it should live elsewhere.

WEBCORE_EXPORT NSString *preferredBundleLocalizationName();

Even less of an "NSString extra", only used in one place in WebKitLegacy. So it
should go there.

NSString *canonicalLocaleName(NSString *);

Only used in preferredBundleLocalizationName, so it should be merged into that
function. The concept of canonical locale name is not clear enough to have the
function safely reusable, and the implementation is super obsolete.