...
Created attachment 452539 [details] Patch
Created attachment 452586 [details] Patch
Comment on attachment 452586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452586&action=review r=me > Source/WebKit/ChangeLog:3 > + Add assertion that no two network session share the same storage path sessions (plural) > Source/WebKit/ChangeLog:8 > + Each NetworkStorageManager has its own queue and access the storage files on that queue, so we can't have two accesses > Source/WebKit/NetworkProcess/storage/NetworkStorageManager.cpp:115 > + static NeverDestroyed<HashSet<String>> paths; I just learned today that you can use MainThreadNeverDestroyed (and drop your main thread assertion above).
Created attachment 452609 [details] Patch for landing
Committed r290196 (247524@main): <https://commits.webkit.org/247524@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452609 [details].
<rdar://problem/89178566>
Reverted r290196 for reason: Caused Network Process crashes <rdar://89190571> Committed r290214 (247542@trunk): <https://commits.webkit.org/247542@trunk>
Created attachment 453443 [details] Patch
Comment on attachment 453443 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453443&action=review > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:211 > + HashMap<String, PAL::SessionID> directoriesBySession; This name seems wrong. If something says "BySession", then I'd expect the session to be the HashMap key, not the value. Maybe this could be named something like: "directorySessions" or "sessionForDirectory" > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:216 > + RELEASE_ASSERT_WITH_MESSAGE(directoriesBySession.add(directory, sessionID).isNewEntry, "GeneralStorageDirectory for session '%llu' is already in use by session '%llu'", directoriesBySession.get(directory).toUInt64(), sessionID.toUInt64()); We're supposed to use PRIu64, not %llu to print 64bit integers to be friendly to Linux ports. Looks like this code may not build on Linux but I think it I still good practice.
Created attachment 453446 [details] Patch
(In reply to Chris Dumez from comment #9) > Comment on attachment 453443 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=453443&action=review > > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:211 > > + HashMap<String, PAL::SessionID> directoriesBySession; > > This name seems wrong. If something says "BySession", then I'd expect the > session to be the HashMap key, not the value. > Maybe this could be named something like: "directorySessions" or > "sessionForDirectory" Changed to sessionForDirectory. > > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:216 > > + RELEASE_ASSERT_WITH_MESSAGE(directoriesBySession.add(directory, sessionID).isNewEntry, "GeneralStorageDirectory for session '%llu' is already in use by session '%llu'", directoriesBySession.get(directory).toUInt64(), sessionID.toUInt64()); > > We're supposed to use PRIu64, not %llu to print 64bit integers to be > friendly to Linux ports. Looks like this code may not build on Linux but I > think it I still good practice. Sure, changed to PRIu64.
Created attachment 453563 [details] Patch
Created attachment 453573 [details] Patch
ChangeLog entry in Tools/ChangeLog contains OOPS!.
Created attachment 453627 [details] Patch for landing
Committed r290739 (247985@main): <https://commits.webkit.org/247985@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453627 [details].