Bug 13631

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:

Keywords:

InRadar

Version:

523.x (Safari 3)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

4123, 20806

Attachments:

Description	Flags
Refactor related code out of DocumentLoader and enhance logging	none
Cleanup and scaffolding - no change in behavior	sam: review+
A next step - more of the handling of the "CachedFrame tree"	sam: review+
Another step - getting close!	darin: review+
Almost there...	darin: review-
Address Darin's comments	darin: review+
Enable back/forward cache for pages with frames (along with some layout tests)	sam: review+

Brady Eidson

Reported 2007-05-08 19:50:38 PDT

The back/forward cache needs to work with pages that have frames <rdar://problem/4886592>

Attachments
Refactor related code out of DocumentLoader and enhance logging (8.44 KB, patch) 2009-08-17 17:25 PDT, Brady Eidson	no flags	Details Formatted Diff Diff
Cleanup and scaffolding - no change in behavior (9.97 KB, patch) 2009-09-01 10:58 PDT, Brady Eidson	sam: review+	Details Formatted Diff Diff
A next step - more of the handling of the "CachedFrame tree" (7.36 KB, patch) 2009-09-02 09:40 PDT, Brady Eidson	sam: review+	Details Formatted Diff Diff
Another step - getting close! (9.14 KB, patch) 2009-09-02 16:09 PDT, Brady Eidson	darin: review+	Details Formatted Diff Diff
Almost there... (5.33 KB, patch) 2009-09-02 17:19 PDT, Brady Eidson	darin: review-	Details Formatted Diff Diff
Address Darin's comments (6.80 KB, patch) 2009-09-02 17:38 PDT, Brady Eidson	darin: review+	Details Formatted Diff Diff
Enable back/forward cache for pages with frames (along with some layout tests) (11.15 KB, patch) 2009-09-03 16:39 PDT, Brady Eidson	sam: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2009-08-17 17:22:36 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.

Brady Eidson

Comment 2 2009-08-17 17:25:25 PDT

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

Eric Seidel (no email)

Comment 3 2009-08-17 18:13:51 PDT

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

Comment 4 2009-08-17 18:24:32 PDT

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 5 2009-08-17 18:25:14 PDT

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

Comment 6 2009-08-17 18:28:38 PDT

Sam Weinig reviewed this over my shoulder.

Brady Eidson

Comment 7 2009-08-17 18:38:55 PDT

This cleanup landed in r47407.

Brady Eidson

Comment 8 2009-09-01 10:30:43 PDT

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

Brady Eidson

Comment 9 2009-09-01 10:58:29 PDT

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

Brady Eidson

Comment 10 2009-09-01 11:14:31 PDT

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

Brady Eidson

Comment 11 2009-09-02 09:40:10 PDT

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

Comment 12 2009-09-02 12:33:26 PDT

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

Brady Eidson

Comment 13 2009-09-02 13:17:19 PDT

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

Brady Eidson

Comment 14 2009-09-02 16:09:54 PDT

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

Brady Eidson

Comment 15 2009-09-02 17:08:24 PDT

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

Brady Eidson

Comment 16 2009-09-02 17:19:12 PDT

Created attachment 38951 [details] Almost there...

Darin Adler

Comment 17 2009-09-02 17:26:17 PDT

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

Comment 18 2009-09-02 17:38:52 PDT

Created attachment 38955 [details] Address Darin's comments

Brady Eidson

Comment 19 2009-09-02 17:56:00 PDT

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

Comment 20 2009-09-03 16:10:55 PDT

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

Brady Eidson

Comment 21 2009-09-03 16:39:13 PDT

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

Brady Eidson

Comment 22 2009-09-03 16:57:48 PDT

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

Note You need to log in before you can comment on or make changes to this bug.