RESOLVED FIXED 214050
Network process crashes in WebKit::StorageManagerSet::deleteSessionStorageForOrigins
https://bugs.webkit.org/show_bug.cgi?id=214050
Summary Network process crashes in WebKit::StorageManagerSet::deleteSessionStorageFor...
Sihui Liu
Reported 2020-07-07 10:49:06 PDT
...
Attachments
Patch (2.79 KB, patch)
2020-07-07 10:52 PDT, Sihui Liu
no flags
Sihui Liu
Comment 1 2020-07-07 10:49:55 PDT Comment hidden (obsolete)
Sihui Liu
Comment 2 2020-07-07 10:52:44 PDT
John Wilander
Comment 3 2020-07-07 10:57:46 PDT
Comment on attachment 403706 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403706&action=review > Source/WebKit/NetworkProcess/NetworkProcess.cpp:1929 > + return; I don't think early return is correct here. The fact that the storage manager set doesn't contain the session ID doesn't mean there aren't other means of storage to delete. The function is for deleting all existing and future website data types. > Source/WebKit/NetworkProcess/NetworkProcess.cpp:1939 > + return; Ditto.
Sihui Liu
Comment 4 2020-07-07 11:13:17 PDT
Comment on attachment 403706 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403706&action=review >> Source/WebKit/NetworkProcess/NetworkProcess.cpp:1929 >> + return; > > I don't think early return is correct here. The fact that the storage manager set doesn't contain the session ID doesn't mean there aren't other means of storage to delete. The function is for deleting all existing and future website data types. This early return is in the callback function of getSessionStorageOrigins, so this change only affects deletion of session storage.. DiskCache below does the same thing..
Chris Dumez
Comment 5 2020-07-07 11:46:51 PDT
Comment on attachment 403706 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403706&action=review > Source/WebKit/NetworkProcess/NetworkProcess.cpp:1932 > m_storageManagerSet->deleteSessionStorageForOrigins(sessionID, originsToDelete, [callbackAggregator] { }); Why are we not doing the sessionID check inside deleteSessionStorageForOrigins() ? > Source/WebKit/NetworkProcess/NetworkProcess.cpp:1942 > m_storageManagerSet->deleteLocalStorageForOrigins(sessionID, originsToDelete, [callbackAggregator] { }); ditto.
Sihui Liu
Comment 6 2020-07-07 12:09:46 PDT
Comment on attachment 403706 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403706&action=review >> Source/WebKit/NetworkProcess/NetworkProcess.cpp:1932 >> m_storageManagerSet->deleteSessionStorageForOrigins(sessionID, originsToDelete, [callbackAggregator] { }); > > Why are we not doing the sessionID check inside deleteSessionStorageForOrigins() ? Currently all functions in StorageManagerSet asserts session exists when they are called, so it looks more consistent to not change there... And in NetworkProcess, we normally get/delete websitedata only when session exists.
Chris Dumez
Comment 7 2020-07-07 12:16:56 PDT
Comment on attachment 403706 [details] Patch Ok
EWS
Comment 8 2020-07-07 12:45:59 PDT
Committed r264036: <https://trac.webkit.org/changeset/264036> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403706 [details].
Sihui Liu
Comment 9 2020-07-07 13:13:35 PDT
Note You need to log in before you can comment on or make changes to this bug.