610658 - Split HistoryDataSource into two parts

Status: RESOLVED FIXED

(Camino Graveyard :: History, defect)

Description

[Stuart Morgan]

Right now HistoryDataSource has two major responsibilities:
1) Listen for Gecko history changes and convert them all into the Cocoa representation
2) Build various tree structures from history, including search and sort support, to use as the data source for an outline view (or similar).

Part 2 varies by client, but part 1 is the same each time. Currently we have one persistent HDS for HistoryMenu (once it's been opened), one that's built on the fly for each history manager that's opened, and one that's built on the fly for each location bar autocomplete. In order to make autocomplete usable for people with large history, that copy will need to become persistent too. That means we do a lot of pointless extra work and waste memory, building extra copies of all the history items.

I'm going to split HDS into two parts, and turn part 1, which doesn't vary by client, into a singleton. That way new clients just have to build their own tree, not a whole history system. In addition to supporting the autocomplete work, this should make opening the history manager faster for everyone (and as an added bonus, split a big class into parts).

Comment 1

[Stuart Morgan]

Attachment: split HDS into two parts; full patch

This is the full patch, splitting out a HistoryTree object to do all of part 2, and making the remaining HDS a shared object.

Because this is a monster, I'm going to post a bunch of checkpoints I did at various stages, so the review can be of meaningful pieces rather than one giant blob. I'll check in the combined patch though, since the checkpoints have some hacky intermediate states (and occasionally a bug I found and fixed later) that don't really make sense to check in.

Comment 2

[Stuart Morgan]

Attachment: part 1: Split out core tree-building logic

This creates HistoryTree as a helper object for HDS, and moves the core logic for building and maintaing the tree into that helper. HDS has passthroughs to HistoryTree so clients don't change.

You can ignore all the NSOutlineViewDataSource and HistoryTreeBuilder stuff removed from HDS and in the new HistoryTree file, since it was all moved verbatim (with only a few whitespace style tweaks)

Comment 3

[Stuart Morgan]

Attachment: part 2: Update clients to use HistoryTree

This removes the passthroughs in HDS, and changes clients to use the HistoryTree directly for most things.

Comment 4

[Stuart Morgan]

Attachment: part 3: Split the notifications across the two classes

This splits up the notification responsibility; HDS now only sends changes about individual items changing and changes to the whole data source at once (clearing, rebuilding), and HistoryTree sends information about when something in the tree changes.

The latter doesn't contain details about what changed exactly, since all clients of those care about in practice is what part of the outline or menu structure to refresh.

Comment 5

[Stuart Morgan]

Attachment: part 4: make HistoryTree notification-driven

In preparation for breaking HistoryTree out entirely, this removes almost all of the direct ties from HDS to HistoryTree, by moving the day change watcher (which only applies to the tree) into HistoryTree and making HistoryTree get most of its information from HDS notifications instead of direct calls from HDS.

Comment 6

[Stuart Morgan]

Attachment: part 5: Split HistoryTree out completely

This pulls HistoryTree out completely, and adds the shared instance management to HDS, updating client appropriately.

Comment 7

[Stuart Morgan]

Attachment: part 6: Final cleanup

This is the last bit, just going through and hitting some things I missed along the way:
- Clean up whitespace in all the moved code in HistoryTree
- Simplify the HDS notifications by making the item the object (since now there's only the one source, and there's always an item in the notification).

Comment 8

[Stuart Morgan]

Attachment: split HDS into two parts; full patch (v2)

New version of the full patch, since I added a bit to the cleanup in part 6 as I was making it.

Comment 10

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 489162 [details] [diff] [review]
split HDS into two parts; full patch (v2)

i scanned through this, but really, who am i kidding? rs=pink

Comment 11

[Stuart Morgan]

Landed as http://hg.mozilla.org/camino/rev/58aec95bb367