WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Sihui Liu
Comment 1
2020-07-07 10:49:55 PDT
Comment hidden (obsolete)
rdar://problem/64853929
Sihui Liu
Comment 2
2020-07-07 10:52:44 PDT
Created
attachment 403706
[details]
Patch
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
<
rdar://problem/64750825
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug