97583 2012-09-25 11:27:09 -0700 [GTK][EFL] Disk cache is never dumped to disk in WebKit2 2012-09-25 12:30:09 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 97580 1 cgarcia webkit-unassigned cdumez danw gyuyoung.kim mrobinson rakuco svillar webkit.review.bot oldest_to_newest 727816 0 cgarcia 2012-09-25 11:27:09 -0700 The problem is that WebProcess::platformTerminate(), where the cache is currently dumped, is not always called for the reasons explained here https://bugs.webkit.org/show_bug.cgi?id=94783. The cache is created and loaded in the main() so it can also be flushed and dumped there, so that we also make sure it's always called when the process finishes normally when the mina loop quits. 727823 1 165647 cgarcia 2012-09-25 11:30:29 -0700 Created attachment 165647 Patch 727827 2 165647 mrobinson 2012-09-25 11:35:04 -0700 Comment on attachment 165647 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165647&action=review > Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:107 > + soup_cache_flush(soupCache); > + soup_cache_dump(soupCache); > + g_object_unref(soupCache); > + I'm surprised that g_object_unref doesn't flush and dump the cache as a side-effect. Maybe Dan or Sergio can comment on that? If that's the case, we can just call g_object_unref here. > Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:90 > + soup_cache_flush(soupCache.get()); > + soup_cache_dump(soupCache.get()); > + Assuming that the soup cache destructor takes care of this, this could just become: soupCache = nullptr; And with a comment explaining why it's important to do that. 727830 3 cgarcia 2012-09-25 11:39:48 -0700 (In reply to comment #2) > (From update of attachment 165647 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=165647&action=review > > > Source/WebKit2/WebProcess/efl/WebProcessMainEfl.cpp:107 > > + soup_cache_flush(soupCache); > > + soup_cache_dump(soupCache); > > + g_object_unref(soupCache); > > + > > I'm surprised that g_object_unref doesn't flush and dump the cache as a side-effect. Maybe Dan or Sergio can comment on that? If that's the case, we can just call g_object_unref here. No, finalize() only frees the internal hash table, but the cache needs to be flushed and dumped manually. Serio told me it's expected to be done when the process finishes and the cache is not going to be used more, because there isn't a way to block the cache while flush/dump. 727879 4 cgarcia 2012-09-25 12:30:09 -0700 Committed r129541: <http://trac.webkit.org/changeset/129541> 165647 2012-09-25 11:30:29 -0700 2012-09-25 11:51:29 -0700 Patch wk2-disk-cache-dump.diff text/plain 3489 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBjZTA3MmEzLi4zN2NmNjgwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjgg QEAKIDIwMTItMDktMjUgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgogCisgICAgICAgIFtHVEtdW0VGTF0gRGlzayBjYWNoZSBpcyBuZXZlciBkdW1wZWQgdG8gZGlz ayBpbiBXZWJLaXQyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD05NzU4MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFRoZSBwcm9ibGVtIGlzIHRoYXQgV2ViUHJvY2Vzczo6cGxhdGZvcm1UZXJtaW5hdGUoKSwg d2hlcmUgdGhlCisgICAgICAgIGNhY2hlIGlzIGN1cnJlbnRseSBkdW1wZWQsIGlzIG5vdCBjYWxs ZWQgd2hlbiB0aGUgd2ViIHByb2Nlc3MKKyAgICAgICAgZmluaXNoZXMgYmVjYXVzZSB0aGUgVUkg cHJvY2VzcyBjbG9zZXMgdGhlIGNvbm5lY3Rpb24uIFRoZSBjYWNoZQorICAgICAgICBpcyBjcmVh dGVkIGFuZCBsb2FkZWQgaW4gdGhlIG1haW4oKSBzbyBpdCBjYW4gYWxzbyBiZSBmbHVzaGVkIGFu ZAorICAgICAgICBkdW1wZWQgdGhlcmUsIHNvIHRoYXQgd2UgYWxzbyBtYWtlIHN1cmUgaXQncyBh bHdheXMgY2FsbGVkIHdoZW4KKyAgICAgICAgdGhlIHByb2Nlc3MgZmluaXNoZXMgbm9ybWFsbHkg d2hlbiB0aGUgbWFpbiBsb29wIHF1aXRzLgorCisgICAgICAgICogV2ViUHJvY2Vzcy9lZmwvV2Vi UHJvY2Vzc01haW5FZmwuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJQcm9jZXNzTWFpbkVmbCk6 IEZsdXNoIGFuZCBkdW1wIHRoZSBkaXNrIGNhY2hlIHdoZW4KKyAgICAgICAgdGhlIG1haW4gbG9v cCByZXR1cm5zLgorICAgICAgICAqIFdlYlByb2Nlc3MvZ3RrL1dlYlByb2Nlc3NNYWluR3RrLmNw cDoKKyAgICAgICAgKFdlYktpdDo6V2ViUHJvY2Vzc01haW5HdGspOiBEaXR0by4KKyAgICAgICAg KiBXZWJQcm9jZXNzL3NvdXAvV2ViUHJvY2Vzc1NvdXAuY3BwOgorICAgICAgICAoV2ViS2l0OjpX ZWJQcm9jZXNzOjpwbGF0Zm9ybVRlcm1pbmF0ZSk6IERvIG5vdCBmbHVzaCBhbmQgZHVtcAorICAg ICAgICB0aGUgZGlzayBjYWNoZS4KKworMjAxMi0wOS0yNSAgQ2FybG9zIEdhcmNpYSBDYW1wb3Mg IDxjZ2FyY2lhQGlnYWxpYS5jb20+CisKICAgICAgICAgW0dUS10gRG9uJ3QgdXNlIHRoZSBDIEFQ SSBpbnRlcm5hbGx5IGluIFdlYktpdEZpbmRDb250cm9sbGVyCiAgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD05Njc3NQogCmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViS2l0Mi9XZWJQcm9jZXNzL2VmbC9XZWJQcm9jZXNzTWFpbkVmbC5jcHAgYi9Tb3VyY2UvV2Vi S2l0Mi9XZWJQcm9jZXNzL2VmbC9XZWJQcm9jZXNzTWFpbkVmbC5jcHAKaW5kZXggOTMxMjQ0Yy4u NTg5YWE3MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9lZmwvV2ViUHJv Y2Vzc01haW5FZmwuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1dlYlByb2Nlc3MvZWZsL1dlYlBy b2Nlc3NNYWluRWZsLmNwcApAQCAtOTAsNyArOTAsNiBAQCBXS19FWFBPUlQgaW50IFdlYlByb2Nl c3NNYWluRWZsKGludCBhcmdjLCBjaGFyKiBhcmd2W10pCiAgICAgU291cENhY2hlKiBzb3VwQ2Fj aGUgPSBzb3VwX2NhY2hlX25ldyhzb3VwQ2FjaGVEaXJlY3RvcnkudXRmOCgpLmRhdGEoKSwgU09V UF9DQUNIRV9TSU5HTEVfVVNFUik7CiAgICAgc291cF9zZXNzaW9uX2FkZF9mZWF0dXJlKHNlc3Np b24sIFNPVVBfU0VTU0lPTl9GRUFUVVJFKHNvdXBDYWNoZSkpOwogICAgIHNvdXBfY2FjaGVfbG9h ZChzb3VwQ2FjaGUpOwotICAgIGdfb2JqZWN0X3VucmVmKHNvdXBDYWNoZSk7CiAKICNpZiBVU0Uo Q09PUkRJTkFURURfR1JBUEhJQ1MpCiAgICAgQ29vcmRpbmF0ZWRHcmFwaGljc0xheWVyOjppbml0 RmFjdG9yeSgpOwpAQCAtMTAyLDYgKzEwMSwxMCBAQCBXS19FWFBPUlQgaW50IFdlYlByb2Nlc3NN YWluRWZsKGludCBhcmdjLCBjaGFyKiBhcmd2W10pCiAgICAgV2ViUHJvY2Vzczo6c2hhcmVkKCku aW5pdGlhbGl6ZShzb2NrZXQsIFJ1bkxvb3A6Om1haW4oKSk7CiAgICAgUnVuTG9vcDo6cnVuKCk7 CiAKKyAgICBzb3VwX2NhY2hlX2ZsdXNoKHNvdXBDYWNoZSk7CisgICAgc291cF9jYWNoZV9kdW1w KHNvdXBDYWNoZSk7CisgICAgZ19vYmplY3RfdW5yZWYoc291cENhY2hlKTsKKwogICAgIHJldHVy biAwOwogCiB9CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL2d0ay9XZWJQ cm9jZXNzTWFpbkd0ay5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL2d0ay9XZWJQcm9j ZXNzTWFpbkd0ay5jcHAKaW5kZXggZGVmZDQxOC4uN2RhY2MyMiAxMDA2NDQKLS0tIGEvU291cmNl L1dlYktpdDIvV2ViUHJvY2Vzcy9ndGsvV2ViUHJvY2Vzc01haW5HdGsuY3BwCisrKyBiL1NvdXJj ZS9XZWJLaXQyL1dlYlByb2Nlc3MvZ3RrL1dlYlByb2Nlc3NNYWluR3RrLmNwcApAQCAtODUsNiAr ODUsOSBAQCBXS19FWFBPUlQgaW50IFdlYlByb2Nlc3NNYWluR3RrKGludCBhcmdjLCBjaGFyKiBh cmd2W10pCiAKICAgICBSdW5Mb29wOjpydW4oKTsKIAorICAgIHNvdXBfY2FjaGVfZmx1c2goc291 cENhY2hlLmdldCgpKTsKKyAgICBzb3VwX2NhY2hlX2R1bXAoc291cENhY2hlLmdldCgpKTsKKwog ICAgIHJldHVybiAwOwogfQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNz L3NvdXAvV2ViUHJvY2Vzc1NvdXAuY3BwIGIvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9zb3Vw L1dlYlByb2Nlc3NTb3VwLmNwcAppbmRleCAzNjUyMzZiLi41MWVmM2E3IDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL3NvdXAvV2ViUHJvY2Vzc1NvdXAuY3BwCisrKyBiL1Nv dXJjZS9XZWJLaXQyL1dlYlByb2Nlc3Mvc291cC9XZWJQcm9jZXNzU291cC5jcHAKQEAgLTE3MSwx MSArMTcxLDYgQEAgdm9pZCBXZWJQcm9jZXNzOjpwbGF0Zm9ybUluaXRpYWxpemVXZWJQcm9jZXNz KGNvbnN0IFdlYlByb2Nlc3NDcmVhdGlvblBhcmFtZXRlcnMKIAogdm9pZCBXZWJQcm9jZXNzOjpw bGF0Zm9ybVRlcm1pbmF0ZSgpCiB7Ci0gICAgU291cFNlc3Npb24qIHNlc3Npb24gPSBXZWJDb3Jl OjpSZXNvdXJjZUhhbmRsZTo6ZGVmYXVsdFNlc3Npb24oKTsKLSAgICBTb3VwQ2FjaGUqIGNhY2hl ID0gU09VUF9DQUNIRShzb3VwX3Nlc3Npb25fZ2V0X2ZlYXR1cmUoc2Vzc2lvbiwgU09VUF9UWVBF X0NBQ0hFKSk7Ci0gICAgc291cF9jYWNoZV9mbHVzaChjYWNoZSk7Ci0gICAgc291cF9jYWNoZV9k dW1wKGNhY2hlKTsKLQogICAgIFdlYkNvcmU6OnJlbW92ZUxhbmd1YWdlQ2hhbmdlT2JzZXJ2ZXIo dGhpcyk7CiB9CiAK