190795 2018-10-22 08:59:17 -0700 Regression (r232410): StorageTracker.db file gets unlinked while in use 2018-10-23 09:58:55 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 sihui_liu sihui_liu cdumez commit-queue ggaren webkit-bug-importer oldest_to_newest 1471226 0 sihui_liu 2018-10-22 08:59:17 -0700 In r232410, WebKit stopped to use StorageTracker.db because of performance, and it would delete this file for safety. r232410 didn't change the behavior of WebKitLegacy, as to keep the old things working. It turned out WebKitLegacy could be using the same file as WebKit, so app built on WebKit could delete the db file while apps with WebKitLegacy is using it. 1471227 1 sihui_liu 2018-10-22 09:00:19 -0700 <rdar://problem/45409880> 1471232 2 352893 sihui_liu 2018-10-22 09:07:37 -0700 Created attachment 352893 Patch 1471240 3 352893 cdumez 2018-10-22 09:33:18 -0700 Comment on attachment 352893 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352893&action=review Is this an issue for apps using both WK1 and WK2? > Source/WebKitLegacy/ChangeLog:8 > + WK2 stopped to use StorageTracker.db file in r232410 and would delete stopped to use -> stopped using 1471258 4 sihui_liu 2018-10-22 10:26:06 -0700 (In reply to Chris Dumez from comment #3) > Comment on attachment 352893 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=352893&action=review > > Is this an issue for apps using both WK1 and WK2? > Yes, if the app uses WK1's Storage API. > > Source/WebKitLegacy/ChangeLog:8 > > + WK2 stopped to use StorageTracker.db file in r232410 and would delete > > stopped to use -> stopped using Okay. 1471261 5 352896 sihui_liu 2018-10-22 10:29:05 -0700 Created attachment 352896 Patch 1471265 6 352896 cdumez 2018-10-22 10:50:36 -0700 Comment on attachment 352896 Patch One concern: Does this mean that WK1-only apps will now have the historical StorageTracker.db and the new LegacyStorageTracker.db when updating WebKit? If so, we'd probably need to remove the historical StorageTracker.db file. 1471286 7 ggaren 2018-10-22 12:00:53 -0700 When we make this file name change, won't LegacyWebKit apps "forget" all the LocalStorage entries that were recorded in StorageTracker.db? 1471324 8 sihui_liu 2018-10-22 13:36:55 -0700 (In reply to Geoffrey Garen from comment #7) > When we make this file name change, won't LegacyWebKit apps "forget" all the > LocalStorage entries that were recorded in StorageTracker.db? It would. But during the initialization of StorageTracker, the origins get re-imported by syncFileSystemAndTrackerDatabase(). 1471325 9 352896 sihui_liu 2018-10-22 13:40:53 -0700 Comment on attachment 352896 Patch Seems like disk access error on wincairo. 1471326 10 cdumez 2018-10-22 13:41:37 -0700 (In reply to Chris Dumez from comment #6) > Comment on attachment 352896 [details] > Patch > > One concern: Does this mean that WK1-only apps will now have the historical > StorageTracker.db and the new LegacyStorageTracker.db when updating WebKit? > If so, we'd probably need to remove the historical StorageTracker.db file. You did not address my concern. 1471327 11 cdumez 2018-10-22 13:42:14 -0700 (In reply to Chris Dumez from comment #10) > (In reply to Chris Dumez from comment #6) > > Comment on attachment 352896 [details] > > Patch > > > > One concern: Does this mean that WK1-only apps will now have the historical > > StorageTracker.db and the new LegacyStorageTracker.db when updating WebKit? > > If so, we'd probably need to remove the historical StorageTracker.db file. > > You did not address my concern. i.e. we do not want legacy files to use up space on customers' devices. 1471335 12 352896 commit-queue 2018-10-22 14:05:29 -0700 Comment on attachment 352896 Patch Clearing flags on attachment: 352896 Committed r237330: <https://trac.webkit.org/changeset/237330> 1471336 13 commit-queue 2018-10-22 14:05:31 -0700 All reviewed patches have been landed. Closing bug. 1471594 14 sihui_liu 2018-10-23 09:58:55 -0700 (In reply to Chris Dumez from comment #11) > (In reply to Chris Dumez from comment #10) > > (In reply to Chris Dumez from comment #6) > > > Comment on attachment 352896 [details] > > > Patch > > > > > > One concern: Does this mean that WK1-only apps will now have the historical > > > StorageTracker.db and the new LegacyStorageTracker.db when updating WebKit? > > > If so, we'd probably need to remove the historical StorageTracker.db file. > > > > You did not address my concern. > > i.e. we do not want legacy files to use up space on customers' devices. Sorry I missed your comment... Yes, I assumed that the users always have some WK2 app to do the cleanup, but there is a chance the user only use WK1 apps so the historical file is left. It won't pose a threat like using up the space, but I should do the cleanup. 352893 2018-10-22 09:07:37 -0700 2018-10-22 10:29:02 -0700 Patch bug-190795-20181022090736.patch text/plain 1719 sihui_liu U3VidmVyc2lvbiBSZXZpc2lvbjogMjM3MzA4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0TGVn YWN5L0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXRMZWdhY3kvQ2hhbmdlTG9nCmluZGV4IDhiZWFh OTQxMDA0YTE0YjU2NDRhMzVhOTg1NjExNTg2MGMxYzFhZWYuLjIzZDZiOWYxMGY4ZjNmMTQ2ZDAy M2JjNWRlYjY4ZWY2MjFkNTIzYzQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXRMZWdhY3kvQ2hh bmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXRMZWdhY3kvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTgtMTAtMjIgIFNpaHVpIExpdSAgPHNpaHVpX2xpdUBhcHBsZS5jb20+CisKKyAgICAg ICAgUmVncmVzc2lvbiAocjIzMjQxMCk6IFN0b3JhZ2VUcmFja2VyLmRiIGZpbGUgZ2V0cyB1bmxp bmtlZCB3aGlsZSBpbiB1c2UKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE5MDc5NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIFdLMiBzdG9wcGVkIHRvIHVzZSBTdG9yYWdlVHJhY2tlci5kYiBmaWxlIGluIHIy MzI0MTAgYW5kIHdvdWxkIGRlbGV0ZSAKKyAgICAgICAgdGhlIGZpbGUgZm9yIHNhZmV0eS4KKyAg ICAgICAgSXQgdHVybmVkIG91dCBXSzEgY291bGQgdXNlIHRoZSBzYW1lIGZpbGUgcGF0aCwgc28g V0syIG1heSBkZWxldGUgdGhlIAorICAgICAgICBmaWxlIHdoaWxlIFdLMSBpcyB1c2luZyBpdC4K KworICAgICAgICAqIFN0b3JhZ2UvU3RvcmFnZVRyYWNrZXIuY3BwOgorICAgICAgICAoV2ViS2l0 OjpTdG9yYWdlVHJhY2tlcjo6dHJhY2tlckRhdGFiYXNlUGF0aCk6CisKIDIwMTgtMTAtMTggIEFs ZXhleSBQcm9za3VyeWFrb3YgIDxhcEBhcHBsZS5jb20+CiAKICAgICAgICAgU3dpdGNoIGZyb20g UExBVEZPUk0oSU9TKSB0byBQTEFURk9STShJT1NfRkFNSUxZKQpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYktpdExlZ2FjeS9TdG9yYWdlL1N0b3JhZ2VUcmFja2VyLmNwcCBiL1NvdXJjZS9XZWJLaXRM ZWdhY3kvU3RvcmFnZS9TdG9yYWdlVHJhY2tlci5jcHAKaW5kZXggODM0OTA0NzAxNWM3ODRlZDAy NGFlMGMwMzBiYmRlOTcxMTBhMTJhNS4uODFkMmE0MzU1MmI1N2EzMjBmN2NkMTgzMDU4NmE5ZDRh ZDliZjM0OCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdExlZ2FjeS9TdG9yYWdlL1N0b3JhZ2VU cmFja2VyLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0TGVnYWN5L1N0b3JhZ2UvU3RvcmFnZVRyYWNr ZXIuY3BwCkBAIC0xMDQsNyArMTA0LDcgQEAgU3RvcmFnZVRyYWNrZXI6OlN0b3JhZ2VUcmFja2Vy KGNvbnN0IFN0cmluZyYgc3RvcmFnZVBhdGgpCiBTdHJpbmcgU3RvcmFnZVRyYWNrZXI6OnRyYWNr ZXJEYXRhYmFzZVBhdGgoKQogewogICAgIEFTU0VSVCghbV9kYXRhYmFzZU11dGV4LnRyeUxvY2so KSk7Ci0gICAgcmV0dXJuIEZpbGVTeXN0ZW06OnBhdGhCeUFwcGVuZGluZ0NvbXBvbmVudChtX3N0 b3JhZ2VEaXJlY3RvcnlQYXRoLCAiU3RvcmFnZVRyYWNrZXIuZGIiKTsKKyAgICByZXR1cm4gRmls ZVN5c3RlbTo6cGF0aEJ5QXBwZW5kaW5nQ29tcG9uZW50KG1fc3RvcmFnZURpcmVjdG9yeVBhdGgs ICJMZWdhY3lTdG9yYWdlVHJhY2tlci5kYiIpOwogfQogCiBzdGF0aWMgYm9vbCBlbnN1cmVEYXRh YmFzZUZpbGVFeGlzdHMoY29uc3QgU3RyaW5nJiBmaWxlTmFtZSwgYm9vbCBjcmVhdGVJZkRvZXNO b3RFeGlzdCkK 352896 2018-10-22 10:29:05 -0700 2018-10-22 14:05:29 -0700 Patch bug-190795-20181022102904.patch text/plain 1718 sihui_liu U3VidmVyc2lvbiBSZXZpc2lvbjogMjM3MzA4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0TGVn YWN5L0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXRMZWdhY3kvQ2hhbmdlTG9nCmluZGV4IDhiZWFh OTQxMDA0YTE0YjU2NDRhMzVhOTg1NjExNTg2MGMxYzFhZWYuLjkzYjQzYTFjNTk4ODBiZTEzZjdj N2U1Y2FlNDBhNTM0M2MyNTQwOTggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXRMZWdhY3kvQ2hh bmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXRMZWdhY3kvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTgg QEAKKzIwMTgtMTAtMjIgIFNpaHVpIExpdSAgPHNpaHVpX2xpdUBhcHBsZS5jb20+CisKKyAgICAg ICAgUmVncmVzc2lvbiAocjIzMjQxMCk6IFN0b3JhZ2VUcmFja2VyLmRiIGZpbGUgZ2V0cyB1bmxp bmtlZCB3aGlsZSBpbiB1c2UKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE5MDc5NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIFdLMiBzdG9wcGVkIHVzaW5nIFN0b3JhZ2VUcmFja2VyLmRiIGZpbGUgaW4gcjIz MjQxMCBhbmQgd291bGQgZGVsZXRlIAorICAgICAgICB0aGUgZmlsZSBmb3Igc2FmZXR5LgorICAg ICAgICBJdCB0dXJuZWQgb3V0IFdLMSBjb3VsZCB1c2UgdGhlIHNhbWUgZmlsZSBwYXRoLCBzbyBX SzIgbWF5IGRlbGV0ZSB0aGUgCisgICAgICAgIGZpbGUgd2hpbGUgV0sxIGlzIHVzaW5nIGl0Lgor CisgICAgICAgICogU3RvcmFnZS9TdG9yYWdlVHJhY2tlci5jcHA6CisgICAgICAgIChXZWJLaXQ6 OlN0b3JhZ2VUcmFja2VyOjp0cmFja2VyRGF0YWJhc2VQYXRoKToKKwogMjAxOC0xMC0xOCAgQWxl eGV5IFByb3NrdXJ5YWtvdiAgPGFwQGFwcGxlLmNvbT4KIAogICAgICAgICBTd2l0Y2ggZnJvbSBQ TEFURk9STShJT1MpIHRvIFBMQVRGT1JNKElPU19GQU1JTFkpCmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViS2l0TGVnYWN5L1N0b3JhZ2UvU3RvcmFnZVRyYWNrZXIuY3BwIGIvU291cmNlL1dlYktpdExl Z2FjeS9TdG9yYWdlL1N0b3JhZ2VUcmFja2VyLmNwcAppbmRleCA4MzQ5MDQ3MDE1Yzc4NGVkMDI0 YWUwYzAzMGJiZGU5NzExMGExMmE1Li44MWQyYTQzNTUyYjU3YTMyMGY3Y2QxODMwNTg2YTlkNGFk OWJmMzQ4IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0TGVnYWN5L1N0b3JhZ2UvU3RvcmFnZVRy YWNrZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXRMZWdhY3kvU3RvcmFnZS9TdG9yYWdlVHJhY2tl ci5jcHAKQEAgLTEwNCw3ICsxMDQsNyBAQCBTdG9yYWdlVHJhY2tlcjo6U3RvcmFnZVRyYWNrZXIo Y29uc3QgU3RyaW5nJiBzdG9yYWdlUGF0aCkKIFN0cmluZyBTdG9yYWdlVHJhY2tlcjo6dHJhY2tl ckRhdGFiYXNlUGF0aCgpCiB7CiAgICAgQVNTRVJUKCFtX2RhdGFiYXNlTXV0ZXgudHJ5TG9jaygp KTsKLSAgICByZXR1cm4gRmlsZVN5c3RlbTo6cGF0aEJ5QXBwZW5kaW5nQ29tcG9uZW50KG1fc3Rv cmFnZURpcmVjdG9yeVBhdGgsICJTdG9yYWdlVHJhY2tlci5kYiIpOworICAgIHJldHVybiBGaWxl U3lzdGVtOjpwYXRoQnlBcHBlbmRpbmdDb21wb25lbnQobV9zdG9yYWdlRGlyZWN0b3J5UGF0aCwg IkxlZ2FjeVN0b3JhZ2VUcmFja2VyLmRiIik7CiB9CiAKIHN0YXRpYyBib29sIGVuc3VyZURhdGFi YXNlRmlsZUV4aXN0cyhjb25zdCBTdHJpbmcmIGZpbGVOYW1lLCBib29sIGNyZWF0ZUlmRG9lc05v dEV4aXN0KQo=