[webkit-reviews] review granted: [Bug 237445] [iOS] Books crashes upon opening a book with a debug build of WebKit : [Attachment 453784] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 237445: [iOS] Books crashes upon opening a book with a debug build of
WebKit
https://bugs.webkit.org/show_bug.cgi?id=237445

Attachment 453784: Patch

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




--- Comment #3 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 453784
  --> https://bugs.webkit.org/attachment.cgi?id=453784
Patch

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

Please add a test.  Just call lookUpContentRuleListForIdentifier with
@"should-not-exist" and wait for it to return an error.  That used to hit this
assertion, right?

> Source/WTF/ChangeLog:3
> +	   [iOS] Books crashes upon opening a book with a debug build of WebKit

I would say "asserts" instead of "crashes"

> Source/WTF/wtf/FileSystem.cpp:446
> +    std::optional<FileSystem::MappedFileData> mappedFileOptional;

This seems unnecessary.  Why not just add an early return if
makeSafeToUseMemoryMapForPath returns false?