Bug 17673 - Bring persistent history to WebCore
Summary: Bring persistent history to WebCore
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: History (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Enhancement
Assignee: Nobody
Keywords: InRadar
Depends on:
Blocks: 16977
  Show dependency treegraph
Reported: 2008-03-04 15:56 PST by Pierre-Luc Beaudoin
Modified: 2010-06-10 19:00 PDT (History)
8 users (show)

See Also:

Early preview (85.54 KB, patch)
2008-03-04 16:05 PST, Pierre-Luc Beaudoin
no flags Details | Formatted Diff | Diff
WebCore only version (54.41 KB, patch)
2008-03-05 13:19 PST, Pierre-Luc Beaudoin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Luc Beaudoin 2008-03-04 15:56:11 PST
The persistent history functionality should be brought to WebCore from WebKit.  This would allow all ports to share a common implementation.

It has been suggested to use SqLite as the engine, a dependency already existing for the IconDatabase.
Comment 1 Pierre-Luc Beaudoin 2008-03-04 16:05:30 PST
Created attachment 19532 [details]
Early preview

This is an early preview version.  It is a diff of my current workspace.  Recent changes (since the 21st feb) may have broken it but it compiles.

Main files of interest: HistoryDatabase*.* and what happens in FrameLoader.

This code is very inspired by IconDatabase, some of its behaviour have been kept in HistoryDatabase but may not have been necessary.  Please comment!
Comment 2 Brady Eidson 2008-03-04 16:12:55 PST
I will take a look at this if I get some spare time soon, or if it progresses closer to ToT/reviewable state.

Without even looking at code let me just say
A - This is a great thing to do that we have a radar for
B - There are a lot of things to get wrong.

Performance (main thread blocking) is concern #1, and thread safety is... also concern #1  :)
Comment 3 Pierre-Luc Beaudoin 2008-03-05 13:19:34 PST
Created attachment 19555 [details]
WebCore only version

I got time to test it against latest and apart from the title being never set, the url and time of visit are saved in the history.  The history pruning seems to works too.

This version should be easier to read.
Comment 4 Brady Eidson 2008-03-07 14:43:53 PST
I've started to look at this, specifically HistoryDatabase.  
I haven't focused on details yet, but I have a high level comment.

While it was logical to start with the icon database design, our understanding of "what works" for threaded WebCore code has evolved since then.  Primarily, I would recommend making the main thread loop be based on a MessageQueue instead.  Then construct a set of tasks that represent the different levels of granularity we need.  ie "Delete this one history item", "Update this one history item", "remove all history items older than this day", "remove all history items beyond the most recent N items", etc

Also, much of complexity of the IconDatabase design was me struggling with the forced notion of the per-icon retain count.  The IconDatabase had this very complex requirement of needing to keep around *only* the important icons the user agent cares about, but otherwise staying as pruned as possible.

The History Database will have no such limitation.  The pruning operations on History are much simpler - at a much higher granularity - than the Icon Database pruning.  Therefore the design has a lot of places it can be simplified.  I have a vague idea that the concept of all these different sets of items that need action taken on them can be reduced to using "the MessageQueue" for the history database.

I think this work has a great start but also has a ways to go.  Keep this bug updated as your patch progresses, and as I find more time this weekend, I'll attach much more detailed thoughts based on what we've learned in WebKit and how we'd like to make sure the move to WebCore makes it even better!  ;)

Comment 5 Pierre-Luc Beaudoin 2008-03-27 06:17:41 PDT
Until more information is available, I won't push this further.  If someone wants to finish the job, assign this bug to you.
Comment 6 Brady Eidson 2008-10-27 15:36:32 PDT
I think I'll be attacking this very soon.  I think it will make things easier by attacking it with a much more reserved plan than trying to get a cross platform database-based store in place in 1 big step.  

I think it makes more sense to put a "GlobalHistory" abstraction in place in WebCore that really is persistent-store agnostic, then worry about new ways to get the bits on and off disk later.
Comment 7 Brady Eidson 2008-10-27 15:38:17 PDT
Comment 8 Adam Barth 2008-10-27 15:43:32 PDT
I think Chromium wants to be able to serialize history items for persistent storage as well.  I'm CC'ing Darin Fisher to he can contribute his two cents.
Comment 9 Brady Eidson 2008-10-27 16:09:52 PDT
Does Chrome not use the FrameLoaderClient methods related to history that already exist...?  Right now, I don't intend to change the design that is in place at all, only move it from a WebKit to WebCore.
Comment 10 Adam Barth 2008-10-27 19:34:17 PDT
> Does Chrome not use the FrameLoaderClient methods related to history that
> already exist...?

Darin is more qualified to answer this question than me (hopefully he'll chime in), but Chromium manages the back-forward list in the master browser process so that the back button can take the tab to another process, etc.  That means it wants to serialize history items to and from the rendering process.

> Right now, I don't intend to change the design that is in
> place at all, only move it from a WebKit to WebCore.

Ok.  I just wanted to loop Darin in case he could contribute to this discussion.