214050 2020-07-07 10:49:06 -0700 Network process crashes in WebKit::StorageManagerSet::deleteSessionStorageForOrigins 2020-07-07 13:13:35 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 sihui_liu sihui_liu cdumez webkit-bug-importer wilander oldest_to_newest 1669464 0 sihui_liu 2020-07-07 10:49:06 -0700 ... 1669465 1 sihui_liu 2020-07-07 10:49:55 -0700 rdar://problem/64853929 1669467 2 403706 sihui_liu 2020-07-07 10:52:44 -0700 Created attachment 403706 Patch 1669469 3 403706 wilander 2020-07-07 10:57:46 -0700 Comment on attachment 403706 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. 1669471 4 403706 sihui_liu 2020-07-07 11:13:17 -0700 Comment on attachment 403706 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.. 1669476 5 403706 cdumez 2020-07-07 11:46:51 -0700 Comment on attachment 403706 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. 1669482 6 403706 sihui_liu 2020-07-07 12:09:46 -0700 Comment on attachment 403706 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. 1669487 7 403706 cdumez 2020-07-07 12:16:56 -0700 Comment on attachment 403706 Patch Ok 1669495 8 ews-feeder 2020-07-07 12:45:59 -0700 Committed r264036: <https://trac.webkit.org/changeset/264036> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403706. 1669503 9 sihui_liu 2020-07-07 13:13:35 -0700 <rdar://problem/64750825> 403706 2020-07-07 10:52:44 -0700 2020-07-07 12:45:59 -0700 Patch bug-214050-20200707105244.patch text/plain 2852 sihui_liu U3VidmVyc2lvbiBSZXZpc2lvbjogMjY0MDE4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDcwNTNkZWY4ZTczZjNlYjk4 OGMyN2NjOTI2YmUxMjg2MWUxNjA4MzMuLjFjMTZiODNhNmUyZDgwOGUxNzA2MDlmNDcxYzk5OGI3 YTFkZjkxMWYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMjAtMDctMDcgIFNpaHVpIExp dSAgPHNpaHVpX2xpdUBhcHBlLmNvbT4KKworICAgICAgICBOZXR3b3JrIHByb2Nlc3MgY3Jhc2hl cyBpbiBXZWJLaXQ6OlN0b3JhZ2VNYW5hZ2VyU2V0OjpkZWxldGVTZXNzaW9uU3RvcmFnZUZvck9y aWdpbnMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIx NDA1MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIElu IE5ldHdvcmtQcm9jZXNzOjpkZWxldGVBbmRSZXN0cmljdFdlYnNpdGVEYXRhRm9yUmVnaXN0cmFi bGVEb21haW5zLCBkZWxldGVTZXNzaW9uU3RvcmFnZUZvck9yaWdpbnMgaXMgY2FsbGVkIAorICAg ICAgICBpbiBhIGNhbGxiYWNrIHdpdGhvdXQgY2hlY2tpbmcgaWYgc2Vzc2lvbiBzdGlsbCBleGlz dHMuCisKKyAgICAgICAgKiBOZXR3b3JrUHJvY2Vzcy9OZXR3b3JrUHJvY2Vzcy5jcHA6CisgICAg ICAgIChXZWJLaXQ6Ok5ldHdvcmtQcm9jZXNzOjpkZWxldGVBbmRSZXN0cmljdFdlYnNpdGVEYXRh Rm9yUmVnaXN0cmFibGVEb21haW5zKToKKwogMjAyMC0wNy0wNyAgQnJhZHkgRWlkc29uICA8YmVp ZHNvbkBhcHBsZS5jb20+CiAKICAgICAgICAgRml4IENyYXNoVHJhY2VyIHJlcG9ydGVkIGluIFBE RlBsdWdpbjo6Qnl0ZVJhbmdlUmVxdWVzdDo6bWF5YmVDb21wbGV0ZS4KZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJLaXQvTmV0d29ya1Byb2Nlc3MvTmV0d29ya1Byb2Nlc3MuY3BwIGIvU291cmNlL1dl YktpdC9OZXR3b3JrUHJvY2Vzcy9OZXR3b3JrUHJvY2Vzcy5jcHAKaW5kZXggZjEyM2IyMGFjNmMy NDUxMGY0NDlhMDI0ZWJkYzlmNTJiNjRkMmQzNS4uYjljZGI0MTRjOGUyNDkxNTVmM2NlMWE0ZDMw YjI3MjU4ZWNiNTFhMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9OZXR3b3JrUHJvY2Vzcy9O ZXR3b3JrUHJvY2Vzcy5jcHAKKysrIGIvU291cmNlL1dlYktpdC9OZXR3b3JrUHJvY2Vzcy9OZXR3 b3JrUHJvY2Vzcy5jcHAKQEAgLTE5MjUsNiArMTkyNSw5IEBAIHZvaWQgTmV0d29ya1Byb2Nlc3M6 OmRlbGV0ZUFuZFJlc3RyaWN0V2Vic2l0ZURhdGFGb3JSZWdpc3RyYWJsZURvbWFpbnMoUEFMOjpT ZXNzCiAgICAgaWYgKG1fc3RvcmFnZU1hbmFnZXJTZXQtPmNvbnRhaW5zKHNlc3Npb25JRCkpIHsK ICAgICAgICAgaWYgKHdlYnNpdGVEYXRhVHlwZXMuY29udGFpbnMoV2Vic2l0ZURhdGFUeXBlOjpT ZXNzaW9uU3RvcmFnZSkpIHsKICAgICAgICAgICAgIG1fc3RvcmFnZU1hbmFnZXJTZXQtPmdldFNl c3Npb25TdG9yYWdlT3JpZ2lucyhzZXNzaW9uSUQsIFtwcm90ZWN0ZWRUaGlzID0gbWFrZVJlZigq dGhpcyksIHRoaXMsIHNlc3Npb25JRCwgY2FsbGJhY2tBZ2dyZWdhdG9yLCBkb21haW5zVG9EZWxl dGVBbGxOb25Db29raWVXZWJzaXRlRGF0YUZvcl0oYXV0byYmIG9yaWdpbnMpIHsKKyAgICAgICAg ICAgICAgICBpZiAoIW1fc3RvcmFnZU1hbmFnZXJTZXQtPmNvbnRhaW5zKHNlc3Npb25JRCkpCisg ICAgICAgICAgICAgICAgICAgIHJldHVybjsKKwogICAgICAgICAgICAgICAgIGF1dG8gb3JpZ2lu c1RvRGVsZXRlID0gZmlsdGVyRm9yUmVnaXN0cmFibGVEb21haW5zKG9yaWdpbnMsIGRvbWFpbnNU b0RlbGV0ZUFsbE5vbkNvb2tpZVdlYnNpdGVEYXRhRm9yLCBjYWxsYmFja0FnZ3JlZ2F0b3ItPm1f ZG9tYWlucyk7CiAgICAgICAgICAgICAgICAgbV9zdG9yYWdlTWFuYWdlclNldC0+ZGVsZXRlU2Vz c2lvblN0b3JhZ2VGb3JPcmlnaW5zKHNlc3Npb25JRCwgb3JpZ2luc1RvRGVsZXRlLCBbY2FsbGJh Y2tBZ2dyZWdhdG9yXSB7IH0pOwogICAgICAgICAgICAgfSk7CkBAIC0xOTMyLDYgKzE5MzUsOSBA QCB2b2lkIE5ldHdvcmtQcm9jZXNzOjpkZWxldGVBbmRSZXN0cmljdFdlYnNpdGVEYXRhRm9yUmVn aXN0cmFibGVEb21haW5zKFBBTDo6U2VzcwogCiAgICAgICAgIGlmICh3ZWJzaXRlRGF0YVR5cGVz LmNvbnRhaW5zKFdlYnNpdGVEYXRhVHlwZTo6TG9jYWxTdG9yYWdlKSkgewogICAgICAgICAgICAg bV9zdG9yYWdlTWFuYWdlclNldC0+Z2V0TG9jYWxTdG9yYWdlT3JpZ2lucyhzZXNzaW9uSUQsIFtw cm90ZWN0ZWRUaGlzID0gbWFrZVJlZigqdGhpcyksIHRoaXMsIHNlc3Npb25JRCwgY2FsbGJhY2tB Z2dyZWdhdG9yLCBkb21haW5zVG9EZWxldGVBbGxOb25Db29raWVXZWJzaXRlRGF0YUZvcl0oYXV0 byYmIG9yaWdpbnMpIHsKKyAgICAgICAgICAgICAgICBpZiAoIW1fc3RvcmFnZU1hbmFnZXJTZXQt PmNvbnRhaW5zKHNlc3Npb25JRCkpCisgICAgICAgICAgICAgICAgICAgIHJldHVybjsKKwogICAg ICAgICAgICAgICAgIGF1dG8gb3JpZ2luc1RvRGVsZXRlID0gZmlsdGVyRm9yUmVnaXN0cmFibGVE b21haW5zKG9yaWdpbnMsIGRvbWFpbnNUb0RlbGV0ZUFsbE5vbkNvb2tpZVdlYnNpdGVEYXRhRm9y LCBjYWxsYmFja0FnZ3JlZ2F0b3ItPm1fZG9tYWlucyk7CiAgICAgICAgICAgICAgICAgbV9zdG9y YWdlTWFuYWdlclNldC0+ZGVsZXRlTG9jYWxTdG9yYWdlRm9yT3JpZ2lucyhzZXNzaW9uSUQsIG9y aWdpbnNUb0RlbGV0ZSwgW2NhbGxiYWNrQWdncmVnYXRvcl0geyB9KTsKICAgICAgICAgICAgIH0p Owo=