148205 2015-08-19 17:48:11 -0700 Regression(r188698): http/tests/cache/disk-cache/disk-cache-revalidation-new-expire-header.html is very flaky 2015-08-21 11:33:58 -0700 1 1 1 Unclassified WebKit WebKit Misc. Other Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=148137 P2 Normal --- 1 ap cdumez cdumez cgarcia commit-queue koivisto thorton oldest_to_newest 1119125 0 ap 2015-08-19 17:48:11 -0700 The test started to fail a lot this afternoon: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fcache%2Fdisk-cache%2Fdisk-cache-revalidation-new-expire-header.html 1119324 1 cdumez 2015-08-20 12:56:58 -0700 I can reproduce the flakiness locally. Looking at this. 1119333 2 cdumez 2015-08-20 13:53:34 -0700 Committed r188698: <http://trac.webkit.org/changeset/188698> 1119405 3 cdumez 2015-08-20 15:35:28 -0700 Looks like this became flaky due to <http://trac.webkit.org/changeset/188640>. I am still working to understand why. 1119412 4 cdumez 2015-08-20 15:44:26 -0700 Reopening to investigate the real cause of the regression. 1119439 5 cdumez 2015-08-20 16:34:21 -0700 So it looks like what is happening is that we get a 304 response in NetworkResourceLoader::didReceiveResponseAsync() / NetworkResourceLoader::didFinishLoading() but m_cacheEntryForValidation is null. As a result, we will call store() with that 304 response instead of updating an existing cache entry. Since 304 is not cacheable, makeCacheDecision() will refuse to cache and we will delete the existing entry in the cache (because we think it is no longer cacheable). My assumption is that those are 304 responses to revalidation's triggered by our MemoryCache instead of the disk cache. This is why m_cacheEntryForValidation is null. I think that even though the resource was revalidated by the memory cache, we should probably still update it in the disk cache if it exists. 1119629 6 259580 cdumez 2015-08-20 20:31:56 -0700 Created attachment 259580 Patch 1119704 7 259580 koivisto 2015-08-21 03:33:00 -0700 Comment on attachment 259580 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259580&action=review It is encouraging that tests caught this subtle bug. > Source/WebKit2/ChangeLog:14 > + Longer term, we should probably update the entry in the disk cache (if > + it exists) when it is revalidated by the memory cache. Currently, Yeah. 1119758 8 259580 cdumez 2015-08-21 09:19:00 -0700 Comment on attachment 259580 Patch Clearing flags on attachment: 259580 Committed r188755: <http://trac.webkit.org/changeset/188755> 1119759 9 cdumez 2015-08-21 09:19:04 -0700 All reviewed patches have been landed. Closing bug. 1119823 10 thorton 2015-08-21 11:33:58 -0700 (In reply to comment #7) > Comment on attachment 259580 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=259580&action=review > > It is encouraging that tests caught this subtle bug. Yes! That is awesome. 259580 2015-08-20 20:31:56 -0700 2015-08-21 09:19:00 -0700 Patch bug-148205-20150820203150.patch text/plain 4401 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTg4Njk4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggNmJmODc1YTAwZDllNmFi NjNhZjExNGNlYzI2YjU5MDQ2YjcxMjZjYi4uOTg2MzgyODRjZmZmNWMwYjk0ZTZiMGExNzc1NWE0 NjJlM2VhNjAxYiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDE1LTA4LTIwICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgUmVncmVzc2lvbihyMTg4Njk4 KTogaHR0cC90ZXN0cy9jYWNoZS9kaXNrLWNhY2hlL2Rpc2stY2FjaGUtcmV2YWxpZGF0aW9uLW5l dy1leHBpcmUtaGVhZGVyLmh0bWwgaXMgdmVyeSBmbGFreQorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ4MjA1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQWZ0ZXIgcjE4ODY0MCwgc3VjY2Vzc2Z1bCByZXZh bGlkYXRpb24gb2YgcmVzb3VyY2VzIGluIHRoZSBtZW1vcnkgY2FjaGUKKyAgICAgICAgd291bGQg Y2F1c2UgdXMgdG8gZHJvcCB0aGUgY29ycmVzcG9uZGluZyByZXNvdXJjZSBpbiB0aGUgZGlzayBj YWNoZS4KKyAgICAgICAgVGhpcyBwYXRjaCBhZGRyZXNzZXMgdGhlIGlzc3VlIGJ5IG5vdCByZW1v dmluZyB0aGUgY2FjaGUgZW50cnkgaWYgdGhlCisgICAgICAgIHJlc3BvbnNlIGlzIGEgc3VjY2Vz c2Z1bCByZXZhbGlkYXRpb24gKGkuZS4gc3RhdHVzIGNvZGUgPT0gMzA0KS4KKworICAgICAgICBM b25nZXIgdGVybSwgd2Ugc2hvdWxkIHByb2JhYmx5IHVwZGF0ZSB0aGUgZW50cnkgaW4gdGhlIGRp c2sgY2FjaGUgKGlmCisgICAgICAgIGl0IGV4aXN0cykgd2hlbiBpdCBpcyByZXZhbGlkYXRlZCBi eSB0aGUgbWVtb3J5IGNhY2hlLiBDdXJyZW50bHksCisgICAgICAgIHJldmFsaWRhdGlvbiBieSB0 aGUgbWVtb3J5IGNhY2hlIGJ5cGFzc2VzIHRoZSBkaXNrIGNhY2hlIGFuZCBnb2VzCisgICAgICAg IHN0cmFpZ2h0IHRvIHRoZSBuZXR3b3JrLiBUaGVuLCB3aGVuIHRoZSByZXNwb25zZSBjb21lcyBi YWNrIGFzIGEgMzA0LAorICAgICAgICB3ZSB0cnkgYW5kIHN0b3JlIHRoZSByZXNwb25zZSBpbiB0 aGUgY2FjaGUuIEhvd2V2ZXIsIGEgMzA0IHN0YXR1cyBjb2RlCisgICAgICAgIGlzIG5vdCBjYWNo ZWFibGUgc28gdGhlIGNhY2hlIHJlamVjdHMgaXQuCisKKyAgICAgICAgKiBOZXR3b3JrUHJvY2Vz cy9jYWNoZS9OZXR3b3JrQ2FjaGUuY3BwOgorICAgICAgICAoV2ViS2l0OjpOZXR3b3JrQ2FjaGU6 OkNhY2hlOjpzdG9yZSk6CisKIDIwMTUtMDgtMjAgIEJldGggRGFraW4gIDxiZGFraW5AYXBwbGUu Y29tPgogCiAgICAgICAgIFN0YW5kYWxvbmUgaW1hZ2UgZG9jdW1lbnRzIHNob3VsZCBzZW5kIHRo ZWlyIHNpemUgdG8gdGhlIFVJQ2xpZW50IGp1c3QgbGlrZSAKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJLaXQyL05ldHdvcmtQcm9jZXNzL2NhY2hlL05ldHdvcmtDYWNoZS5jcHAgYi9Tb3VyY2UvV2Vi S2l0Mi9OZXR3b3JrUHJvY2Vzcy9jYWNoZS9OZXR3b3JrQ2FjaGUuY3BwCmluZGV4IDE2M2Y2YjBm OWM0MmI5ZDI5NGE2M2E0MGZmMjcyYjRiYTliZDhkOWYuLmU3ODgxMTU0ZTc5ZDFlMmVkOTBmYjRl NGM0YmE4MDJlOTM0ZmQxMjQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL05ldHdvcmtQcm9j ZXNzL2NhY2hlL05ldHdvcmtDYWNoZS5jcHAKKysrIGIvU291cmNlL1dlYktpdDIvTmV0d29ya1By b2Nlc3MvY2FjaGUvTmV0d29ya0NhY2hlLmNwcApAQCAtNDA3LDggKzQwNywxMSBAQCB2b2lkIENh Y2hlOjpzdG9yZShjb25zdCBXZWJDb3JlOjpSZXNvdXJjZVJlcXVlc3QmIG9yaWdpbmFsUmVxdWVz dCwgY29uc3QgV2ViQ29yZQogICAgICAgICBMT0coTmV0d29ya0NhY2hlLCAiKE5ldHdvcmtQcm9j ZXNzKSBkaWRuJ3Qgc3RvcmUsIHN0b3JlRGVjaXNpb249JWQiLCBzdG9yZURlY2lzaW9uKTsKICAg ICAgICAgYXV0byBrZXkgPSBtYWtlQ2FjaGVLZXkob3JpZ2luYWxSZXF1ZXN0KTsKIAotICAgICAg ICAvLyBNYWtlIHN1cmUgd2UgZG9uJ3Qga2VlcCBhIHN0YWxlIGVudHJ5IGluIHRoZSBjYWNoZS4K LSAgICAgICAgcmVtb3ZlKGtleSk7CisgICAgICAgIGF1dG8gaXNTdWNjZXNzZnVsUmV2YWxpZGF0 aW9uID0gcmVzcG9uc2UuaHR0cFN0YXR1c0NvZGUoKSA9PSAzMDQ7CisgICAgICAgIGlmICghaXNT dWNjZXNzZnVsUmV2YWxpZGF0aW9uKSB7CisgICAgICAgICAgICAvLyBNYWtlIHN1cmUgd2UgZG9u J3Qga2VlcCBhIHN0YWxlIGVudHJ5IGluIHRoZSBjYWNoZS4KKyAgICAgICAgICAgIHJlbW92ZShr ZXkpOworICAgICAgICB9CiAKICAgICAgICAgaWYgKG1fc3RhdGlzdGljcykKICAgICAgICAgICAg IG1fc3RhdGlzdGljcy0+cmVjb3JkTm90Q2FjaGluZ1Jlc3BvbnNlKGtleSwgc3RvcmVEZWNpc2lv bik7CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cKaW5kZXggOTA0YTUyNzVkOTc2MDc3OTc1NzU5NTE1MTUwZmJmYmUxYjA4ZjIyOS4uZjdk MDgxYzYxOTNiY2FkYTg4NWZhMTFhODY4MGEyYmYyYjE4ODU1ZSAxMDA2NDQKLS0tIGEvTGF5b3V0 VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3RzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE1 IEBACiAyMDE1LTA4LTIwICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKKyAgICAg ICAgUmVncmVzc2lvbihyMTg4Njk4KTogaHR0cC90ZXN0cy9jYWNoZS9kaXNrLWNhY2hlL2Rpc2st Y2FjaGUtcmV2YWxpZGF0aW9uLW5ldy1leHBpcmUtaGVhZGVyLmh0bWwgaXMgdmVyeSBmbGFreQor ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ4MjA1CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBodHRwL3Rl c3RzL2NhY2hlL2Rpc2stY2FjaGUvZGlzay1jYWNoZS1yZXZhbGlkYXRpb24tbmV3LWV4cGlyZS1o ZWFkZXIuaHRtbDoKKyAgICAgICAgRHJvcCB0ZW1wb3JhcnkgZml4IGxhbmRlZCBpbiByMTg4Njk4 IHRvIG1ha2UgdGhlIHRlc3QgbGVzcyBmbGFreS4KKworMjAxNS0wOC0yMCAgQ2hyaXMgRHVtZXog IDxjZHVtZXpAYXBwbGUuY29tPgorCiAgICAgICAgIFJFR1JFU1NJT046IGh0dHAvdGVzdHMvY2Fj aGUvZGlzay1jYWNoZS9kaXNrLWNhY2hlLXJldmFsaWRhdGlvbi1uZXctZXhwaXJlLWhlYWRlci5o dG1sIGlzIHZlcnkgZmxha3kKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE0ODIwNQogCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9odHRwL3Rlc3RzL2Nh Y2hlL2Rpc2stY2FjaGUvZGlzay1jYWNoZS1yZXZhbGlkYXRpb24tbmV3LWV4cGlyZS1oZWFkZXIu aHRtbCBiL0xheW91dFRlc3RzL2h0dHAvdGVzdHMvY2FjaGUvZGlzay1jYWNoZS9kaXNrLWNhY2hl LXJldmFsaWRhdGlvbi1uZXctZXhwaXJlLWhlYWRlci5odG1sCmluZGV4IDZiNDIwNDE2NmYxODRm ZTM4OWRjYzQ4OWU4OGNhNjdjYTIxZGY2ZDEuLjAzM2NhMGZlYTA0MzhhMzQ1YmM2ZTAwMWNkMGYz NWZmOWY1ZjM3NDkgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL2h0dHAvdGVzdHMvY2FjaGUvZGlz ay1jYWNoZS9kaXNrLWNhY2hlLXJldmFsaWRhdGlvbi1uZXctZXhwaXJlLWhlYWRlci5odG1sCisr KyBiL0xheW91dFRlc3RzL2h0dHAvdGVzdHMvY2FjaGUvZGlzay1jYWNoZS9kaXNrLWNhY2hlLXJl dmFsaWRhdGlvbi1uZXctZXhwaXJlLWhlYWRlci5odG1sCkBAIC0xMywxMSArMTMsOCBAQCBkZXNj cmlwdGlvbigiVGVzdCB0aGF0IHRoZSAnRXhwaXJlcycgaGVhZGVyIGlzIHVwZGF0ZWQgdXBvbiBz dWNjZXNzZnVsIHZhbGlkYXRpbwogZGVidWcoIiIpOwogCiBydW5UZXN0cyh0ZXN0cywgZnVuY3Rp b24oKSB7Ci0gICAgLy8gV2FpdCBmb3IgdGhpbmdzIHRvIHNldHRsZSBkb3duIGluIHRoZSBjYWNo ZS4KLSAgICBzZXRUaW1lb3V0KGZ1bmN0aW9uKCkgewotICAgICAgICBkZWJ1ZygiMzA0IHJlc3Bv bnNlIGluY2x1ZGVkIGFuICdFeHBpcmVzJyBoZWFkZXIgaW4gdGhlIGZ1dHVyZSwgc28gd2Ugc2hv dWxkIG5vdCBuZWVkIHRvIHJldmFsaWRhdGUgdGhpcyB0aW1lLiIpOwotICAgICAgICBydW5UZXN0 cyh0ZXN0cyk7Ci0gICAgfSwgMjAwKTsKKyAgICBkZWJ1ZygiMzA0IHJlc3BvbnNlIGluY2x1ZGVk IGFuICdFeHBpcmVzJyBoZWFkZXIgaW4gdGhlIGZ1dHVyZSwgc28gd2Ugc2hvdWxkIG5vdCBuZWVk IHRvIHJldmFsaWRhdGUgdGhpcyB0aW1lLiIpOworICAgIHJ1blRlc3RzKHRlc3RzKTsKIH0pOwog CiA8L3NjcmlwdD4K