183051 2018-02-22 12:12:25 -0800 [Curl] Cookie Database files are wrongfully getting deleted when the database is opened 2018-02-23 18:02:59 -0800 1 1 1 Unclassified WebKit Platform WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 chris.reid webkit-unassigned achristensen annulen basuke bfulgham commit-queue don.olmstead ews-watchlist galpeter pvollan webkit-bug-importer youennf oldest_to_newest 1401498 0 chris.reid 2018-02-22 12:12:25 -0800 The file stat logic in CookieJarDB::checkDatabaseCorruptionAndRemoveIfNeeded is backwards causing persistence issues. The database is wrongfully deleted when opened because it thinks it's corrupted. 1401502 1 334465 chris.reid 2018-02-22 12:19:40 -0800 Created attachment 334465 Patch 1401789 2 334465 don.olmstead 2018-02-23 12:51:10 -0800 Comment on attachment 334465 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334465&action=review LGTM. I'm not sure how liberally we should be using auto or not. > Source/WebCore/platform/network/curl/CookieJarDB.cpp:166 > + FileSystem::PlatformFileHandle handle = FileSystem::openFile(getCorruptionMarkerPath(), FileSystem::FileOpenMode::Write); Maybe auto here? 1401805 3 chris.reid 2018-02-23 14:01:36 -0800 (In reply to Don Olmstead from comment #2) > Comment on attachment 334465 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334465&action=review > > LGTM. I'm not sure how liberally we should be using auto or not. > > > Source/WebCore/platform/network/curl/CookieJarDB.cpp:166 > > + FileSystem::PlatformFileHandle handle = FileSystem::openFile(getCorruptionMarkerPath(), FileSystem::FileOpenMode::Write); > > Maybe auto here? It looks like auto is used quite a bit with openFile in the FileSystem implementation files so it makes sense to use it here too. I'll update the patch. 1401807 4 334547 chris.reid 2018-02-23 14:25:28 -0800 Created attachment 334547 Patch 1401817 5 334547 pvollan 2018-02-23 15:15:10 -0800 Comment on attachment 334547 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334547&action=review R=me. > Source/WebCore/platform/network/curl/CookieJarDB.cpp:167 > + FileSystem::closeFile(handle); Should we check whether 'handle' is valid before closing? 1401827 6 chris.reid 2018-02-23 15:55:02 -0800 Thanks for the review! (In reply to Per Arne Vollan from comment #5) > Comment on attachment 334547 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334547&action=review > > R=me. > > > Source/WebCore/platform/network/curl/CookieJarDB.cpp:167 > > + FileSystem::closeFile(handle); > > Should we check whether 'handle' is valid before closing? Ah, right now all the closeFile implementations have a check but it seems safer to have a check here too in case more functionality is added to that file. 1401829 7 334550 chris.reid 2018-02-23 16:03:52 -0800 Created attachment 334550 Patch for landing 1401851 8 334550 commit-queue 2018-02-23 18:01:41 -0800 Comment on attachment 334550 Patch for landing Clearing flags on attachment: 334550 Committed r228971: <https://trac.webkit.org/changeset/228971> 1401852 9 commit-queue 2018-02-23 18:01:42 -0800 All reviewed patches have been landed. Closing bug. 1401853 10 webkit-bug-importer 2018-02-23 18:02:59 -0800 <rdar://problem/37846254> 334465 2018-02-22 12:19:40 -0800 2018-02-23 14:25:28 -0800 Patch 183051.diff text/plain 1906 chris.reid ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAwNjM5ZDc2ZWEyNC4uOWM1ZWIwMTE2ZTggMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNSBAQAorMjAxOC0wMi0yMiAgQ2hyaXN0b3BoZXIgUmVpZCAgPGNocmlzLnJlaWRAc29u eS5jb20+CisKKyAgICAgICAgW0N1cmxdIENvb2tpZSBEYXRhYmFzZSBmaWxlcyBhcmUgd3Jvbmdm dWxseSBnZXR0aW5nIGRlbGV0ZWQgd2hlbiB0aGUgZGF0YWJhc2UgaXMgb3BlbmVkCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODMwNTEKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgZmlsZSBzdGF0IGxv Z2ljIHdhcyBiYWNrd2FyZHMgY2F1c2luZyBhIHdyb25nZnVsIGRldGVjdGlvbiBvZiBkYXRhYmFz ZSBjb3JydXB0aW9uLgorICAgICAgICBGaXhlZCB0aGUgbG9naWMgYW5kIGFic3RyYWN0ZWQgdGhl c2UgY2FsbHMgdG8gdXNlIEZpbGVTeXN0ZW0uCisKKyAgICAgICAgKiBwbGF0Zm9ybS9uZXR3b3Jr L2N1cmwvQ29va2llSmFyREIuY3BwOgorCiAyMDE4LTAyLTIwICBOYW4gV2FuZyAgPG5fd2FuZ0Bh cHBsZS5jb20+CiAKICAgICAgICAgQVg6IEFPTTogRGlzcGF0Y2ggYWNjZXNzaWJsZWNsaWNrIGV2 ZW50CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL2N1cmwvQ29v a2llSmFyREIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9jdXJsL0Nvb2tp ZUphckRCLmNwcAppbmRleCA5NjU5NTg0MzllOC4uMzA3ODJiOTUyYTEgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvY3VybC9Db29raWVKYXJEQi5jcHAKKysrIGIv U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9jdXJsL0Nvb2tpZUphckRCLmNwcApAQCAt MTYzLDIyICsxNjMsMTggQEAgdm9pZCBDb29raWVKYXJEQjo6ZmxhZ0RhdGFiYXNlQ29ycnVwdGlv bigpCiAgICAgaWYgKGlzT25NZW1vcnkoKSkKICAgICAgICAgcmV0dXJuOwogCi0gICAgRklMRSog ZiA9IGZvcGVuKGdldENvcnJ1cHRpb25NYXJrZXJQYXRoKCkudXRmOCgpLmRhdGEoKSwgIndiIik7 Ci0gICAgZmNsb3NlKGYpOworICAgIEZpbGVTeXN0ZW06OlBsYXRmb3JtRmlsZUhhbmRsZSBoYW5k bGUgPSBGaWxlU3lzdGVtOjpvcGVuRmlsZShnZXRDb3JydXB0aW9uTWFya2VyUGF0aCgpLCBGaWxl U3lzdGVtOjpGaWxlT3Blbk1vZGU6OldyaXRlKTsKKyAgICBGaWxlU3lzdGVtOjpjbG9zZUZpbGUo aGFuZGxlKTsKIH0KIAogYm9vbCBDb29raWVKYXJEQjo6Y2hlY2tEYXRhYmFzZUNvcnJ1cHRpb25B bmRSZW1vdmVJZk5lZWRlZCgpCiB7Ci0gICAgaWYgKGlzT25NZW1vcnkoKSkKLSAgICAgICAgcmV0 dXJuIGZhbHNlOwotCi0gICAgc3RydWN0IHN0YXQgc3Q7Ci0gICAgaW50IHJldCA9IHN0YXQoZ2V0 Q29ycnVwdGlvbk1hcmtlclBhdGgoKS51dGY4KCkuZGF0YSgpLCAmc3QpOwotICAgIGlmICghcmV0 KQotICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgaWYgKCFpc09uTWVtb3J5KCkgJiYgRmlsZVN5 c3RlbTo6ZmlsZUV4aXN0cyhnZXRDb3JydXB0aW9uTWFya2VyUGF0aCgpKSkgeworICAgICAgICBk ZWxldGVBbGxEYXRhYmFzZUZpbGVzKCk7CisgICAgICAgIHJldHVybiB0cnVlOworICAgIH0KIAot ICAgIGRlbGV0ZUFsbERhdGFiYXNlRmlsZXMoKTsKLSAgICByZXR1cm4gdHJ1ZTsKKyAgICByZXR1 cm4gZmFsc2U7CiB9CiAKIGJvb2wgQ29va2llSmFyREI6OmNoZWNrU1FMaXRlUmV0dXJuQ29kZShp bnQgYWN0dWFsLCBpbnQgZXhwZWN0ZWQpCg== 334547 2018-02-23 14:25:28 -0800 2018-02-23 16:03:48 -0800 Patch 183051.diff text/plain 1880 chris.reid ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAwNjM5ZDc2ZWEyNC4uOWM1ZWIwMTE2ZTggMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNSBAQAorMjAxOC0wMi0yMiAgQ2hyaXN0b3BoZXIgUmVpZCAgPGNocmlzLnJlaWRAc29u eS5jb20+CisKKyAgICAgICAgW0N1cmxdIENvb2tpZSBEYXRhYmFzZSBmaWxlcyBhcmUgd3Jvbmdm dWxseSBnZXR0aW5nIGRlbGV0ZWQgd2hlbiB0aGUgZGF0YWJhc2UgaXMgb3BlbmVkCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODMwNTEKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUaGUgZmlsZSBzdGF0IGxv Z2ljIHdhcyBiYWNrd2FyZHMgY2F1c2luZyBhIHdyb25nZnVsIGRldGVjdGlvbiBvZiBkYXRhYmFz ZSBjb3JydXB0aW9uLgorICAgICAgICBGaXhlZCB0aGUgbG9naWMgYW5kIGFic3RyYWN0ZWQgdGhl c2UgY2FsbHMgdG8gdXNlIEZpbGVTeXN0ZW0uCisKKyAgICAgICAgKiBwbGF0Zm9ybS9uZXR3b3Jr L2N1cmwvQ29va2llSmFyREIuY3BwOgorCiAyMDE4LTAyLTIwICBOYW4gV2FuZyAgPG5fd2FuZ0Bh cHBsZS5jb20+CiAKICAgICAgICAgQVg6IEFPTTogRGlzcGF0Y2ggYWNjZXNzaWJsZWNsaWNrIGV2 ZW50CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3b3JrL2N1cmwvQ29v a2llSmFyREIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9jdXJsL0Nvb2tp ZUphckRCLmNwcAppbmRleCA5NjU5NTg0MzllOC4uMzA3ODJiOTUyYTEgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL25ldHdvcmsvY3VybC9Db29raWVKYXJEQi5jcHAKKysrIGIv U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbmV0d29yay9jdXJsL0Nvb2tpZUphckRCLmNwcApAQCAt MTYzLDIyICsxNjMsMTggQEAgdm9pZCBDb29raWVKYXJEQjo6ZmxhZ0RhdGFiYXNlQ29ycnVwdGlv bigpCiAgICAgaWYgKGlzT25NZW1vcnkoKSkKICAgICAgICAgcmV0dXJuOwogCi0gICAgRklMRSog ZiA9IGZvcGVuKGdldENvcnJ1cHRpb25NYXJrZXJQYXRoKCkudXRmOCgpLmRhdGEoKSwgIndiIik7 Ci0gICAgZmNsb3NlKGYpOworICAgIGF1dG8gaGFuZGxlID0gRmlsZVN5c3RlbTo6b3BlbkZpbGUo Z2V0Q29ycnVwdGlvbk1hcmtlclBhdGgoKSwgRmlsZVN5c3RlbTo6RmlsZU9wZW5Nb2RlOjpXcml0 ZSk7CisgICAgRmlsZVN5c3RlbTo6Y2xvc2VGaWxlKGhhbmRsZSk7CiB9CiAKIGJvb2wgQ29va2ll SmFyREI6OmNoZWNrRGF0YWJhc2VDb3JydXB0aW9uQW5kUmVtb3ZlSWZOZWVkZWQoKQogewotICAg IGlmIChpc09uTWVtb3J5KCkpCi0gICAgICAgIHJldHVybiBmYWxzZTsKLQotICAgIHN0cnVjdCBz dGF0IHN0OwotICAgIGludCByZXQgPSBzdGF0KGdldENvcnJ1cHRpb25NYXJrZXJQYXRoKCkudXRm OCgpLmRhdGEoKSwgJnN0KTsKLSAgICBpZiAoIXJldCkKLSAgICAgICAgcmV0dXJuIGZhbHNlOwor ICAgIGlmICghaXNPbk1lbW9yeSgpICYmIEZpbGVTeXN0ZW06OmZpbGVFeGlzdHMoZ2V0Q29ycnVw dGlvbk1hcmtlclBhdGgoKSkpIHsKKyAgICAgICAgZGVsZXRlQWxsRGF0YWJhc2VGaWxlcygpOwor ICAgICAgICByZXR1cm4gdHJ1ZTsKKyAgICB9CiAKLSAgICBkZWxldGVBbGxEYXRhYmFzZUZpbGVz KCk7Ci0gICAgcmV0dXJuIHRydWU7CisgICAgcmV0dXJuIGZhbHNlOwogfQogCiBib29sIENvb2tp ZUphckRCOjpjaGVja1NRTGl0ZVJldHVybkNvZGUoaW50IGFjdHVhbCwgaW50IGV4cGVjdGVkKQo= 334550 2018-02-23 16:03:52 -0800 2018-02-23 18:01:41 -0800 Patch for landing bug-183051-20180223160351.patch text/plain 2073 chris.reid U3VidmVyc2lvbiBSZXZpc2lvbjogMjI4ODI3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMDYzOWQ3NmVhMjRmNzgy OTM3NTM1NGZiMzVkZGQwNDFlY2YwZjQ0ZC4uZjhhZDIyNDQ2Zjg5NjcxMjYyOGViN2I0Y2ZiYTI0 ODk1OTc1NDFkYyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE4LTAyLTIyICBDaHJp c3RvcGhlciBSZWlkICA8Y2hyaXMucmVpZEBzb255LmNvbT4KKworICAgICAgICBbQ3VybF0gQ29v a2llIERhdGFiYXNlIGZpbGVzIGFyZSB3cm9uZ2Z1bGx5IGdldHRpbmcgZGVsZXRlZCB3aGVuIHRo ZSBkYXRhYmFzZSBpcyBvcGVuZWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTE4MzA1MQorCisgICAgICAgIFJldmlld2VkIGJ5IFBlciBBcm5lIFZvbGxh bi4KKworICAgICAgICBUaGUgZmlsZSBzdGF0IGxvZ2ljIHdhcyBiYWNrd2FyZHMgY2F1c2luZyBh IHdyb25nZnVsIGRldGVjdGlvbiBvZiBkYXRhYmFzZSBjb3JydXB0aW9uLgorICAgICAgICBGaXhl ZCB0aGUgbG9naWMgYW5kIGFic3RyYWN0ZWQgdGhlc2UgY2FsbHMgdG8gdXNlIEZpbGVTeXN0ZW0u CisKKyAgICAgICAgKiBwbGF0Zm9ybS9uZXR3b3JrL2N1cmwvQ29va2llSmFyREIuY3BwOgorCiAy MDE4LTAyLTIwICBOYW4gV2FuZyAgPG5fd2FuZ0BhcHBsZS5jb20+CiAKICAgICAgICAgQVg6IEFP TTogRGlzcGF0Y2ggYWNjZXNzaWJsZWNsaWNrIGV2ZW50CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi Q29yZS9wbGF0Zm9ybS9uZXR3b3JrL2N1cmwvQ29va2llSmFyREIuY3BwIGIvU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vbmV0d29yay9jdXJsL0Nvb2tpZUphckRCLmNwcAppbmRleCA5NjU5NTg0Mzll ODM2MTYwYTBlMzVmZTc5OTM3OGUxNzAyNjRlYjFkLi42M2FjMDA4NjI0MDljZGQ2ZTIyYWVkMmE0 ZjI1ZDk2ZDc4NmJhYjgwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9uZXR3 b3JrL2N1cmwvQ29va2llSmFyREIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL25l dHdvcmsvY3VybC9Db29raWVKYXJEQi5jcHAKQEAgLTE2MywyMiArMTYzLDE5IEBAIHZvaWQgQ29v a2llSmFyREI6OmZsYWdEYXRhYmFzZUNvcnJ1cHRpb24oKQogICAgIGlmIChpc09uTWVtb3J5KCkp CiAgICAgICAgIHJldHVybjsKIAotICAgIEZJTEUqIGYgPSBmb3BlbihnZXRDb3JydXB0aW9uTWFy a2VyUGF0aCgpLnV0ZjgoKS5kYXRhKCksICJ3YiIpOwotICAgIGZjbG9zZShmKTsKKyAgICBhdXRv IGhhbmRsZSA9IEZpbGVTeXN0ZW06Om9wZW5GaWxlKGdldENvcnJ1cHRpb25NYXJrZXJQYXRoKCks IEZpbGVTeXN0ZW06OkZpbGVPcGVuTW9kZTo6V3JpdGUpOworICAgIGlmIChGaWxlU3lzdGVtOjpp c0hhbmRsZVZhbGlkKGhhbmRsZSkpCisgICAgICAgIEZpbGVTeXN0ZW06OmNsb3NlRmlsZShoYW5k bGUpOwogfQogCiBib29sIENvb2tpZUphckRCOjpjaGVja0RhdGFiYXNlQ29ycnVwdGlvbkFuZFJl bW92ZUlmTmVlZGVkKCkKIHsKLSAgICBpZiAoaXNPbk1lbW9yeSgpKQotICAgICAgICByZXR1cm4g ZmFsc2U7Ci0KLSAgICBzdHJ1Y3Qgc3RhdCBzdDsKLSAgICBpbnQgcmV0ID0gc3RhdChnZXRDb3Jy dXB0aW9uTWFya2VyUGF0aCgpLnV0ZjgoKS5kYXRhKCksICZzdCk7Ci0gICAgaWYgKCFyZXQpCi0g ICAgICAgIHJldHVybiBmYWxzZTsKKyAgICBpZiAoIWlzT25NZW1vcnkoKSAmJiBGaWxlU3lzdGVt OjpmaWxlRXhpc3RzKGdldENvcnJ1cHRpb25NYXJrZXJQYXRoKCkpKSB7CisgICAgICAgIGRlbGV0 ZUFsbERhdGFiYXNlRmlsZXMoKTsKKyAgICAgICAgcmV0dXJuIHRydWU7CisgICAgfQogCi0gICAg ZGVsZXRlQWxsRGF0YWJhc2VGaWxlcygpOwotICAgIHJldHVybiB0cnVlOworICAgIHJldHVybiBm YWxzZTsKIH0KIAogYm9vbCBDb29raWVKYXJEQjo6Y2hlY2tTUUxpdGVSZXR1cm5Db2RlKGludCBh Y3R1YWwsIGludCBleHBlY3RlZCkK