834541 - remove hasHistoryEntries

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P3)

Description

[Marco Bonardo [:mak]]

hasHistoryEntries is cached most of the time, though sometimes we have to requery, and that query is synchronous.

Comment 1

[Marco Bonardo [:mak]]

a couple instances in WindowsJumpLists.jsm could be easily converted to be async.
The one on shutdown could be converted using AsyncShutdown...

but actually, if we could completely remove the API, it would be even better.
WindowsJumpLists.jsm is the only _external_ consumer in our code, the instances in queries are there just as micro optimizations (don't even query if we know there's no history), we could just ignore those optimizations since they only help someone that doesn't have any history (very small minority of users).
The check on shutdown is basically a "did we run clear history on shutdown?" question, and we can figure a better way to reply that question that doesn't involve disk access or even knowing if there's history (it could listen to a clear on shutdown topic or directly to history, unfortunately "shutdown-cleanse" has gone).

Comment 3

[Marco Bonardo [:mak]]

Attachment: Bug 834541 - Remove the public History.hasHistoryEntries synchronous API.

Review commit: https://reviewboard.mozilla.org/r/242280/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242280/

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 8973949 [details]
Bug 834541 - Remove the public History.hasHistoryEntries synchronous API.

https://reviewboard.mozilla.org/r/242280/#review248504

Comment 5

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 8f16311fb00e415258833ff313a9982bf5e32c4b -d 2d5c99b70e34: rebasing 462825:8f16311fb00e "Bug 834541 - Remove the public History.hasHistoryEntries synchronous API. r=standard8" (tip)
merging toolkit/components/places/nsNavHistory.cpp
merging toolkit/components/places/nsNavHistory.h
merging toolkit/components/places/nsNavHistoryResult.cpp
merging toolkit/components/places/tests/unit/test_browserhistory.js
warning: conflicts while merging toolkit/components/places/nsNavHistory.h! (edit, then use 'hg resolve --mark')
warning: conflicts while merging toolkit/components/places/tests/unit/test_browserhistory.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 8973949 [details]
Bug 834541 - Remove the public History.hasHistoryEntries synchronous API.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242280/diff/1-2/

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 8973949 [details]
Bug 834541 - Remove the public History.hasHistoryEntries synchronous API.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242280/diff/2-3/

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 8973949 [details]
Bug 834541 - Remove the public History.hasHistoryEntries synchronous API.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242280/diff/3-4/

Comment 9

[Marco Bonardo [:mak]]

Comment on attachment 8973949 [details]
Bug 834541 - Remove the public History.hasHistoryEntries synchronous API.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242280/diff/4-5/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80cabc246b5f
Remove the public History.hasHistoryEntries synchronous API. r=standard8

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/80cabc246b5f