[webkit-reviews] review granted: [Bug 176236] Add a ResourceBundle abstraction : [Attachment 449606] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Don Olmstead
<don.olmstead at sony.com>'s request for review:
Bug 176236: Add a ResourceBundle abstraction
https://bugs.webkit.org/show_bug.cgi?id=176236

Attachment 449606: Patch

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




--- Comment #8 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 449606
  --> https://bugs.webkit.org/attachment.cgi?id=449606
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=449606&action=review

r=me with comment.

> Source/WTF/wtf/ResourceBundle.cpp:63
> +    String fileName = name + "."_s + type;

Let's use makeString here, which is more efficient.

makeString(name, "."_s, type)

> Source/WTF/wtf/ResourceBundle.h:45
> +    WTF_EXPORT_PRIVATE static std::unique_ptr<ResourceBundle>
create(String);

Should we use `const String&`?
(We cannot use StringView here since we need to convert it to CFString via
String's method).

> Source/WTF/wtf/ResourceBundle.h:55
> +    WTF_EXPORT_PRIVATE String resourcePath(String name, String type, String
directory) const;
> +    WTF_EXPORT_PRIVATE URL resourceURL(String name, String type, String
directory) const;

Should we use `const String&`?
(We cannot use StringView here since we need to convert it to CFString via
String's method).