Bug 237445

Summary: [iOS] Books ASSERTs upon opening a book with a debug build of WebKit
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: New BugsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, benjamin, cdumez, cgarcia, cmarcelo, ews-watchlist, sihui_liu, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
achristensen: review+, achristensen: commit-queue-
Patch for committing
none
Patch for committing
ews-feeder: commit-queue-
Patch for committing ews-feeder: commit-queue-

Myles C. Maxfield
Reported 2022-03-03 13:56:21 PST
[iOS] Books crashes upon opening a book with a debug build of WebKit
Attachments
Patch (9.16 KB, patch)
2022-03-03 13:59 PST, Myles C. Maxfield
achristensen: review+
achristensen: commit-queue-
Patch for committing (8.05 KB, patch)
2022-03-04 11:42 PST, Myles C. Maxfield
no flags
Patch for committing (10.73 KB, patch)
2022-03-04 11:45 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Patch for committing (10.82 KB, patch)
2022-03-04 15:01 PST, Myles C. Maxfield
ews-feeder: commit-queue-
Myles C. Maxfield
Comment 1 2022-03-03 13:59:57 PST
Radar WebKit Bug Importer
Comment 2 2022-03-03 14:08:05 PST
Alex Christensen
Comment 3 2022-03-03 14:53:25 PST
Comment on attachment 453784 [details] 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?
Sihui Liu
Comment 4 2022-03-03 15:41:46 PST
Comment on attachment 453784 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453784&action=review > Source/WTF/wtf/cocoa/FileSystemCocoa.mm:220 > + return !error && success; Can you add log to print error if it's not nil? What is the error Books gets? I wonder if there's other issue we should fix
Alex Christensen
Comment 5 2022-03-03 18:39:52 PST
Books gets this: {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"} Makes sense.
Myles C. Maxfield
Comment 6 2022-03-04 10:26:52 PST
Comment on attachment 453784 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453784&action=review >> Source/WTF/wtf/cocoa/FileSystemCocoa.mm:220 >> + return !error && success; > > Can you add log to print error if it's not nil? > What is the error Books gets? I wonder if there's other issue we should fix Sure, I can add logging no problem.
Myles C. Maxfield
Comment 7 2022-03-04 11:42:33 PST
Created attachment 453860 [details] Patch for committing
Myles C. Maxfield
Comment 8 2022-03-04 11:45:33 PST
Created attachment 453862 [details] Patch for committing
Myles C. Maxfield
Comment 9 2022-03-04 15:01:18 PST
Created attachment 453875 [details] Patch for committing
Myles C. Maxfield
Comment 10 2022-03-04 15:08:31 PST
Note You need to log in before you can comment on or make changes to this bug.