[webkit-reviews] review granted: [Bug 225368] Streamline codec parsing, replacing uses of HashMap with SortedArrayMap : [Attachment 427727] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Darin Adler <darin at apple.com>'s request
for review:
Bug 225368: Streamline codec parsing, replacing uses of HashMap with
SortedArrayMap
https://bugs.webkit.org/show_bug.cgi?id=225368

Attachment 427727: Patch

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




--- Comment #5 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 427727
  --> https://bugs.webkit.org/attachment.cgi?id=427727
Patch

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

> Source/WebCore/platform/graphics/HEVCUtilities.cpp:128
> +    return makeOptionalFromPointer(typesMap.tryGet(string));

Should we just make tryGet() return an Optional? Or, perhaps have a version of
tryGet that does? This seems like it could be a pretty common idiom? (though,
as always, this kind of change can and should be left until there is actual
evidence that we need to have duplicate copies of makeOptionalFromPointer()).

> Source/WebCore/platform/graphics/cocoa/HEVCUtilitiesCocoa.mm:150
> +    Vector<uint16_t> vector;
> +    vector.reserveInitialCapacity(array.count);
> +    for (id value in array) {

I think you could use compactMap() here. If you don't want to (or it doesn't
work), a shinkToFit() after the loop is probably a good idea here.