Bug 30274 - HistoryController should point back to FrameLoader not Frame
Summary: HistoryController should point back to FrameLoader not Frame
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 29947
  Show dependency tree / graph
 
Reported: 2009-10-10 02:27 PDT by Adam Barth
Modified: 2009-10-14 21:26 PDT (History)
1 user (show)

See Also:

Attachments
Patch v1 (29.56 KB, patch)
2009-10-10 02:28 PDT, Adam Barth 		no flags Details | Formatted Diff | Diff
HistoryController back pointer (29.59 KB, patch)
2009-10-10 02:42 PDT, Adam Barth 		darin: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2009-10-10 02:27:09 PDT 
Eric thinks HistoryController should point back to FrameLoader instead of Frame.  I'm not entirely convinced, but I made a patch to see what it would be like.  If he thinks we should do this, I'll defer to his experience.
Comment 1 Adam Barth 2009-10-10 02:28:54 PDT 
Created attachment 40987 [details]
Patch v1
Comment 2 Adam Barth 2009-10-10 02:42:08 PDT 
Created attachment 40988 [details]
HistoryController back pointer
Comment 3 Darin Adler 2009-10-11 20:28:40 PDT 
Comment on attachment 40988 [details]
HistoryController back pointer

I think this is OK either way.

Pointing back to the frame is OK because everybody points back to the frame. It's all one big frame-lifetime-associated family of objects.

Pointing back to the frame loader is OK because the frame loader is the immediate owner of the history controller, and it's the thing the creates and destroys the history controller.

I have a hard time getting strong feelings either way.

The part I don't like is functions that are only needed once per page on the main frame that exist here on a pre-frame object.

r=me, and you can decide whether or not to land this. I really have no preference.
Comment 4 Adam Barth 2009-10-14 21:26:51 PDT 
I think I'm going to leave this as-is.  Thanks for your comments.