Bug 30274

Summary:

HistoryController should point back to FrameLoader not Frame

Product:

WebKit

Reporter:

Adam Barth <abarth>

Component:

Page Loading

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WORKSFORME

Severity:

Normal

CC:

eric

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

29947

Attachments:

Description	Flags
Patch v1	none
HistoryController back pointer	darin: review+

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.