| Summary: | Page Cache should support pages with frames | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||||||||||||||
| Component: | Page Loading | Assignee: | Brady Eidson <beidson> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | ap, emacemac7, koivisto, michael | ||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
| Version: | 523.x (Safari 3) | ||||||||||||||||||
| Hardware: | All | ||||||||||||||||||
| OS: | All | ||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||
| Bug Blocks: | 4123, 20806 | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Brady Eidson
2007-05-08 19:50:38 PDT
Back to working on this lately. It will be coming in baby steps, trying to make the code a little more sane as we get closer to flipping the switch. Created attachment 35002 [details]
Refactor related code out of DocumentLoader and enhance logging
Comment on attachment 35002 [details]
Refactor related code out of DocumentLoader and enhance logging
It's now possible to test the page cache (with the setting override stuff). Dumi (I think?) added the first page-cache test the other day. Seems we should add one for this change too.
If he added page cache layout tests, that's great. In this situation, I wish you could've told me *where* he added these, because I personally don't have time to watch all the commits go by on a day to day basis and don't know where to look. Regardless, since this patch is a pure refactoring and has zero change in behavior, testing the change in behavior for regressions is a non-starter. I did run the layout tests and there were no new failures. Putting back up for review. Comment on attachment 35002 [details]
Refactor related code out of DocumentLoader and enhance logging
Layout test not possible, zero behavior change. Putting back up for review.
Sam Weinig reviewed this over my shoulder. Updated Radar URL: <rdar://problem/3541409> Created attachment 38871 [details]
Cleanup and scaffolding - no change in behavior
Landed in http://trac.webkit.org/changeset/47943 Created attachment 38928 [details]
A next step - more of the handling of the "CachedFrame tree"
Again, this doesn't change current behavior, and is just another step in landing my work piece-by-piece.
This morning's patch landed in http://trac.webkit.org/changeset/47985 More coming... http://trac.webkit.org/changeset/47989 is related to this work, as well. Created attachment 38944 [details]
Another step - getting close!
Created attachment 38951 [details]
Almost there...
Comment on attachment 38951 [details]
Almost there...
Should use an enum instead of adding a new bool argument.
It's true that in one place the argument is a boolean expression, not a constant, but I still think that these confusing functions can be made less so with enums.
In fact, I think you could combine the sendUnload and sendPagehide bool arguments into a single enum if you name the values carefully.
Created attachment 38955 [details]
Address Darin's comments
Landed in http://trac.webkit.org/changeset/48001 I'll flip the switch sometime tomorrow, after I get a chance to update and build on Windows. The patch was reviewed over my shoulder, so it's not in the list, but: http://trac.webkit.org/changeset/48034 Created attachment 39019 [details]
Enable back/forward cache for pages with frames (along with some layout tests)
Landed in http://trac.webkit.org/changeset/48036 |