Bug 13631 – Page Cache should support pages with frames

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Brady Eidson]

The back/forward cache needs to work with pages that have frames

<rdar://problem/4886592>

[Brady Eidson]

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.

[Brady Eidson]

Created attachment 35002 [details]
Refactor related code out of DocumentLoader and enhance logging

[Eric Seidel (no email)]

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.

[Brady Eidson]

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.

[Brady Eidson]

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.

[Brady Eidson]

Sam Weinig reviewed this over my shoulder.

[Brady Eidson]

This cleanup landed in r47407.

[Brady Eidson]

Updated Radar URL: <rdar://problem/3541409>

[Brady Eidson]

Created attachment 38871 [details]
Cleanup and scaffolding - no change in behavior

[Brady Eidson]

Landed in http://trac.webkit.org/changeset/47943

[Brady Eidson]

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.

[Brady Eidson]

This morning's patch landed in http://trac.webkit.org/changeset/47985
More coming...

[Brady Eidson]

http://trac.webkit.org/changeset/47989 is related to this work, as well.

[Brady Eidson]

Created attachment 38944 [details]
Another step - getting close!

[Brady Eidson]

http://trac.webkit.org/changeset/47999

[Brady Eidson]

Created attachment 38951 [details]
Almost there...

[Darin Adler]

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.

[Brady Eidson]

Created attachment 38955 [details]
Address Darin's comments

[Brady Eidson]

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.

[Brady Eidson]

The patch was reviewed over my shoulder, so it's not in the list, but:
http://trac.webkit.org/changeset/48034

[Brady Eidson]

Created attachment 39019 [details]
Enable back/forward cache for pages with frames (along with some layout tests)

[Brady Eidson]

Landed in http://trac.webkit.org/changeset/48036