32618 2009-12-16 10:42:09 -0800 [Chromium] Memory cache client calls should be disabled 2010-07-22 08:51:55 -0700 1 1 1 Unclassified WebKit Page Loading 528+ (Nightly build) PC All RESOLVED WONTFIX P2 Normal --- 1 apavlov webkit-unassigned commit-queue eric fishd jparent levin ojan pfeldman webkit.review.bot oldest_to_newest 172388 0 apavlov 2009-12-16 10:42:09 -0800 A Chromium issue http://code.google.com/p/chromium/issues/detail?id=29076 is caused by Frame::loadedResourceFromMemoryCache() calling requestFromDelegate(). It creates a new identifier for the same resource on reload, unless page->areMemoryCacheClientCallsEnabled() == false. It is always the case in Safari (which sets the flag to false from the outset), so we should just emulate the behaviour. 172402 1 44994 apavlov 2009-12-16 10:52:30 -0800 Created attachment 44994 patch 172406 2 webkit.review.bot 2009-12-16 10:54:37 -0800 style-queue ran check-webkit-style on attachment 44994 without any errors. 172492 3 44994 eric 2009-12-16 13:12:23 -0800 Comment on attachment 44994 patch What is a memoryCacheClientCall? Seems the ChangeLog might want to mention why we're disabling this. In general the change seems fine. 172493 4 eric 2009-12-16 13:14:23 -0800 Seems this was added in https://bugs.webkit.org/show_bug.cgi?id=23160 172569 5 apavlov 2009-12-16 16:36:39 -0800 (In reply to comment #3) > (From update of attachment 44994 [details]) > What is a memoryCacheClientCall? Seems the ChangeLog might want to mention > why we're disabling this. In general the change seems fine. Eric, does mentioning the reasons similar to those in my original description for this bug seem fine with you? Basically, the active code path with memoryCacheClientCallsEnabled == true creates duplicate resource IDs for cached resources that are loaded in a way similar to "(new Image()).src = 'foo.png'" (I'm afraid to touch the solid ground of this behavior as it stems from the JavaScript VM handling DOM, but it seems Safari will exhibit the same bug as Chromium, without disabling the AforeMentionedCalls.) 172857 6 levin 2009-12-17 10:36:10 -0800 (In reply to comment #5) > (In reply to comment #3) > > (From update of attachment 44994 [details] [details]) > > What is a memoryCacheClientCall? Seems the ChangeLog might want to mention > > why we're disabling this. In general the change seems fine. > > Eric, does mentioning the reasons similar to those in my original description > for this bug seem fine with you? Yes or something more terse even like this: "Disable memory cache client calls so that a new identifier isn't created for the same resource on reload." 172858 7 44994 levin 2009-12-17 10:36:30 -0800 Comment on attachment 44994 patch r- per comments in the bug. 172864 8 45087 apavlov 2009-12-17 10:40:45 -0800 Created attachment 45087 patch (ChangeLog message augmented) 172866 9 45089 apavlov 2009-12-17 10:44:58 -0800 Created attachment 45089 patch (refined ChangeLog message as suggested) 172870 10 webkit.review.bot 2009-12-17 10:46:12 -0800 style-queue ran check-webkit-style on attachment 45089 without any errors. 172933 11 45089 eric 2009-12-17 13:01:01 -0800 Comment on attachment 45089 patch (refined ChangeLog message as suggested) OK. 172946 12 45089 pfeldman 2009-12-17 14:05:56 -0800 Comment on attachment 45089 patch (refined ChangeLog message as suggested) Alexander does not have a commit bit yet. 172947 13 45089 commit-queue 2009-12-17 14:14:46 -0800 Comment on attachment 45089 patch (refined ChangeLog message as suggested) Clearing flags on attachment: 45089 Committed r52291: <http://trac.webkit.org/changeset/52291> 172948 14 commit-queue 2009-12-17 14:14:55 -0800 All reviewed patches have been landed. Closing bug. 172954 15 fishd 2009-12-17 14:35:32 -0800 Does this change suppress calls to FrameLoaderClient::dispatchDidLoadResourceFromMemoryCache? If so, then it just broke updating the SSL lock indicator. We use this very important signal to know when a resource is loaded from the memory cache in case that resource was originally fetched over broken SSL. 172957 16 fishd 2009-12-17 14:41:11 -0800 I suspect IN_PROC_BROWSER_TEST_F(SSLUITest, TestCachedMixedContents) will start failing. That's a test in chrome/browser/ssl/ssl_browser_tests.cc. 172964 17 eric 2009-12-17 15:09:38 -0800 Started the rollout. 172968 18 eric 2009-12-17 15:22:47 -0800 Committed r52293: <http://trac.webkit.org/changeset/52293> 254832 19 apavlov 2010-07-22 08:51:55 -0700 The change described is not needed anymore. 44994 2009-12-16 10:52:30 -0800 2009-12-17 10:40:45 -0800 patch memocache.patch text/plain 1024 apavlov ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDVjODZlOGYuLjZiNDdiNWIgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDEzIEBACisyMDA5LTEyLTE2ICBBbGV4YW5kZXIgUGF2bG92ICA8YXBhdmxvdkBjaHJvbWl1bS5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRGlz YWJsZSBtZW1vcnkgY2FjaGUgY2xpZW50IGNhbGxzIChsaWtlIFNhZmFyaSkKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMyNjE4CisKKyAgICAgICAgKiBz cmMvV2ViVmlld0ltcGwuY3BwOgorICAgICAgICAoV2ViS2l0OjpXZWJWaWV3SW1wbDo6V2ViVmll d0ltcGwpOgorCiAyMDA5LTEyLTE1ICBLZW5uZXRoIFJ1c3NlbGwgIDxrYnJAZ29vZ2xlLmNvbT4K IAogICAgICAgICBSZXZpZXdlZCBieSBEYXJpbiBGaXNoZXIuCmRpZmYgLS1naXQgYS9XZWJLaXQv Y2hyb21pdW0vc3JjL1dlYlZpZXdJbXBsLmNwcCBiL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViVmll d0ltcGwuY3BwCmluZGV4IDg3YTE1MGEuLjZlODNlOGMgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9zcmMvV2ViVmlld0ltcGwuY3BwCisrKyBiL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViVmll d0ltcGwuY3BwCkBAIC0yMzQsNiArMjM0LDcgQEAgV2ViVmlld0ltcGw6OldlYlZpZXdJbXBsKFdl YlZpZXdDbGllbnQqIGNsaWVudCkKIAogICAgIG1fcGFnZS0+YmFja0ZvcndhcmRMaXN0KCktPnNl dENsaWVudCgmbV9iYWNrRm9yd2FyZExpc3RDbGllbnRJbXBsKTsKICAgICBtX3BhZ2UtPnNldEdy b3VwTmFtZShwYWdlR3JvdXBOYW1lKTsKKyAgICBtX3BhZ2UtPnNldE1lbW9yeUNhY2hlQ2xpZW50 Q2FsbHNFbmFibGVkKGZhbHNlKTsKIH0KIAogV2ViVmlld0ltcGw6On5XZWJWaWV3SW1wbCgpCg== 45087 2009-12-17 10:40:45 -0800 2009-12-17 10:44:58 -0800 patch (ChangeLog message augmented) memocache.patch text/plain 1273 apavlov ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDVjODZlOGYuLmZkNzY2OGYgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDE3IEBACisyMDA5LTEyLTE2ICBBbGV4YW5kZXIgUGF2bG92ICA8YXBhdmxvdkBjaHJvbWl1bS5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRGlz YWJsZSBtZW1vcnkgY2FjaGUgY2xpZW50IGNhbGxzIChsaWtlIFNhZmFyaSkKKworICAgICAgICBX aXRoIHRoZSBjYWxscyBlbmFibGVkLCBkdXBsaWNhdGUgZW50cmllcyBmb3IgY2FjaGVkIHJlc291 cmNlcyBhcmUgY3JlYXRlZAorICAgICAgICBpbiB0aGUgV2ViIEluc3BlY3Rvci4gU2FmYXJpIGZv ciBXaW5kb3dzIGRpc2FibGVzIHRoZSBjYWxscyBuZWFyIHRoZSBwb2ludCBvZiB0aGUKKyAgICAg ICAgRnJhbWUgaW5zdGFuY2UgY3JlYXRpb24sIGFuZCB3ZSBqdXN0IGZvbGxvdyB0aGlzIGZvciBD aHJvbWl1bS4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTMyNjE4CisKKyAgICAgICAgKiBzcmMvV2ViVmlld0ltcGwuY3BwOgorICAgICAgICAoV2ViS2l0 OjpXZWJWaWV3SW1wbDo6V2ViVmlld0ltcGwpOgorCiAyMDA5LTEyLTE1ICBLZW5uZXRoIFJ1c3Nl bGwgIDxrYnJAZ29vZ2xlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXJpbiBGaXNoZXIu CmRpZmYgLS1naXQgYS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYlZpZXdJbXBsLmNwcCBiL1dlYktp dC9jaHJvbWl1bS9zcmMvV2ViVmlld0ltcGwuY3BwCmluZGV4IDg3YTE1MGEuLjZlODNlOGMgMTAw NjQ0Ci0tLSBhL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViVmlld0ltcGwuY3BwCisrKyBiL1dlYktp dC9jaHJvbWl1bS9zcmMvV2ViVmlld0ltcGwuY3BwCkBAIC0yMzQsNiArMjM0LDcgQEAgV2ViVmll d0ltcGw6OldlYlZpZXdJbXBsKFdlYlZpZXdDbGllbnQqIGNsaWVudCkKIAogICAgIG1fcGFnZS0+ YmFja0ZvcndhcmRMaXN0KCktPnNldENsaWVudCgmbV9iYWNrRm9yd2FyZExpc3RDbGllbnRJbXBs KTsKICAgICBtX3BhZ2UtPnNldEdyb3VwTmFtZShwYWdlR3JvdXBOYW1lKTsKKyAgICBtX3BhZ2Ut PnNldE1lbW9yeUNhY2hlQ2xpZW50Q2FsbHNFbmFibGVkKGZhbHNlKTsKIH0KIAogV2ViVmlld0lt cGw6On5XZWJWaWV3SW1wbCgpCg== 45089 2009-12-17 10:44:58 -0800 2010-06-10 19:51:38 -0700 patch (refined ChangeLog message as suggested) memocache.patch text/plain 1340 apavlov ZGlmZiAtLWdpdCBhL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cgYi9XZWJLaXQvY2hyb21pdW0v Q2hhbmdlTG9nCmluZGV4IDVjODZlOGYuLjY1ZGNjMDcgMTAwNjQ0Ci0tLSBhL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvV2ViS2l0L2Nocm9taXVtL0NoYW5nZUxvZwpAQCAtMSwzICsx LDE4IEBACisyMDA5LTEyLTE2ICBBbGV4YW5kZXIgUGF2bG92ICA8YXBhdmxvdkBjaHJvbWl1bS5v cmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRGlz YWJsZSBtZW1vcnkgY2FjaGUgY2xpZW50IGNhbGxzIHNvIHRoYXQgYSBuZXcgaWRlbnRpZmllciBp c24ndCBjcmVhdGVkCisgICAgICAgIGZvciB0aGUgc2FtZSByZXNvdXJjZSBvbiByZWxvYWQuCisK KyAgICAgICAgV2l0aCB0aGUgY2FsbHMgZW5hYmxlZCwgZHVwbGljYXRlIGVudHJpZXMgZm9yIGNh Y2hlZCByZXNvdXJjZXMgYXJlIGNyZWF0ZWQKKyAgICAgICAgaW4gdGhlIFdlYiBJbnNwZWN0b3Iu IFNhZmFyaSBmb3IgV2luZG93cyBkaXNhYmxlcyB0aGUgY2FsbHMgbmVhciB0aGUgcG9pbnQgb2Yg dGhlCisgICAgICAgIEZyYW1lIGluc3RhbmNlIGNyZWF0aW9uLCBhbmQgd2UganVzdCBmb2xsb3cg dGhpcyBmb3IgQ2hyb21pdW0uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0zMjYxOAorCisgICAgICAgICogc3JjL1dlYlZpZXdJbXBsLmNwcDoKKyAgICAg ICAgKFdlYktpdDo6V2ViVmlld0ltcGw6OldlYlZpZXdJbXBsKToKKwogMjAwOS0xMi0xNSAgS2Vu bmV0aCBSdXNzZWxsICA8a2JyQGdvb2dsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFy aW4gRmlzaGVyLgpkaWZmIC0tZ2l0IGEvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJWaWV3SW1wbC5j cHAgYi9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYlZpZXdJbXBsLmNwcAppbmRleCA4N2ExNTBhLi42 ZTgzZThjIDEwMDY0NAotLS0gYS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYlZpZXdJbXBsLmNwcAor KysgYi9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYlZpZXdJbXBsLmNwcApAQCAtMjM0LDYgKzIzNCw3 IEBAIFdlYlZpZXdJbXBsOjpXZWJWaWV3SW1wbChXZWJWaWV3Q2xpZW50KiBjbGllbnQpCiAKICAg ICBtX3BhZ2UtPmJhY2tGb3J3YXJkTGlzdCgpLT5zZXRDbGllbnQoJm1fYmFja0ZvcndhcmRMaXN0 Q2xpZW50SW1wbCk7CiAgICAgbV9wYWdlLT5zZXRHcm91cE5hbWUocGFnZUdyb3VwTmFtZSk7Cisg ICAgbV9wYWdlLT5zZXRNZW1vcnlDYWNoZUNsaWVudENhbGxzRW5hYmxlZChmYWxzZSk7CiB9CiAK IFdlYlZpZXdJbXBsOjp+V2ViVmlld0ltcGwoKQo=