1846361 - Consider to remove DB access in nsNavHistoryResultNode::GetTags()

Status: RESOLVED FIXED

(Toolkit :: Places, enhancement)

Description

[Daisuke Akatsuka (:daisuke)]

This is a follow-up bug of bug 1829581.
As mentioned in phabricator, we shoud consdier to remove nsNavHistoryResultNode::GetTags().

Comment 1

[Marco Bonardo [:mak]]

A little bit off-topic, but trying to keep together some thoughts.
This should be the last main-thread SQL in nsNavHistoryResult apart from the GetQueryResults call in FillChildren, that is used by Refresh(), OpenContainer(), OpenContainerAsync... and GetHasChildren(). The scope is to make only open and refresh operations execute queries, to simplify making those async.
For nsNavHistoryFolderResultNode::GetHasChildren asynchronous may be feasible, we would show no twisties until the querying is done, then invalidateRow in the tree. Though there's a lot of usage of .hasChildren to verify in the treeview, as it may break features.
Or alternatively we may keep a map of bookmark folders in memory, loaded at startup and updated through triggers.

Comment 2

[Marco Bonardo [:mak]]

I started filing Bug 1846461 to start moving some stuff out of the "bookmarks" service (that in reality is pretty much a "SynchronousTagsAsBookmarks" service).

Comment 3

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1846361: Avoid accessing DB in GetTags()

Comment 4

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1846361: Unify NotifyIfTagsChanged into OnItemTimeChanged

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99ae6c3bb49d
Unify NotifyIfTagsChanged into OnItemTimeChanged r=places-reviewers,mak
https://hg.mozilla.org/integration/autoland/rev/2ed041c3a569
Avoid accessing DB in GetTags() r=mak

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/99ae6c3bb49d
https://hg.mozilla.org/mozilla-central/rev/2ed041c3a569