Summary: | [Chromium] Memory cache client calls should be disabled | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexander Pavlov (apavlov) <apavlov> | ||||||||
Component: | Page Loading | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED WONTFIX | ||||||||||
Severity: | Normal | CC: | commit-queue, eric, fishd, jparent, levin, ojan, pfeldman, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Alexander Pavlov (apavlov)
2009-12-16 10:42:09 PST
Created attachment 44994 [details]
patch
style-queue ran check-webkit-style on attachment 44994 [details] without any errors.
Comment on attachment 44994 [details]
patch
What is a memoryCacheClientCall? Seems the ChangeLog might want to mention why we're disabling this. In general the change seems fine.
Seems this was added in https://bugs.webkit.org/show_bug.cgi?id=23160 (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.) (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." Comment on attachment 44994 [details]
patch
r- per comments in the bug.
Created attachment 45087 [details]
patch (ChangeLog message augmented)
Created attachment 45089 [details]
patch (refined ChangeLog message as suggested)
style-queue ran check-webkit-style on attachment 45089 [details] without any errors.
Comment on attachment 45089 [details]
patch (refined ChangeLog message as suggested)
OK.
Comment on attachment 45089 [details]
patch (refined ChangeLog message as suggested)
Alexander does not have a commit bit yet.
Comment on attachment 45089 [details] patch (refined ChangeLog message as suggested) Clearing flags on attachment: 45089 Committed r52291: <http://trac.webkit.org/changeset/52291> All reviewed patches have been landed. Closing bug. 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. I suspect IN_PROC_BROWSER_TEST_F(SSLUITest, TestCachedMixedContents) will start failing. That's a test in chrome/browser/ssl/ssl_browser_tests.cc. Started the rollout. Committed r52293: <http://trac.webkit.org/changeset/52293> The change described is not needed anymore. |