[Webkit-unassigned] [Bug 30246] Factor HistoryController out of FrameLoader

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30246


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #40952|review?                     |review+
               Flag|                            |




--- Comment #13 from Darin Adler <darin at apple.com>  2009-10-09 10:24:10 PDT ---
(From update of attachment 40952)
> -PassRefPtr<HistoryItem> FrameLoader::createHistoryItem(bool useOriginal)
> +PassRefPtr<HistoryItem> HistoryController::createHistoryItem(bool useOriginal)

Elsewhere you removed the word "history" from the phrase "history item" since
this is the history controller. I think you should do that here too. Except
maybe there's ambiguity with back/forward item?

> -PassRefPtr<HistoryItem> FrameLoader::createHistoryItemTree(Frame* targetFrame, bool clipAtTarget)
> +PassRefPtr<HistoryItem> HistoryController::createHistoryItemTree(Frame* targetFrame, bool clipAtTarget)

Ditto.

You can consider in some cases including the entire class as a member of the
owner class. The reason we don't do that in Frame itself to avoid header
dependencies -- it's probably OK to have a few more memory blocks allocated to
avoid recompiling every files that includes FrameLoader.h if
HistoryController.h changes. But if we plan to have FrameLoader.h include
HistoryController.h then I think there's no reason to use a separate memory
block.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.