Bug 834541 - Remove the public History.hasHistoryEntries synchronous API. r=standard8
MozReview-Commit-ID: KJW9YNwoSZb
--- a/browser/components/extensions/test/xpcshell/test_ext_history.js
+++ b/browser/components/extensions/test/xpcshell/test_ext_history.js
@@ -124,17 +124,16 @@ add_task(async function test_delete() {
equal((await PlacesTestUtils.visitsInDB(visits[0].url)), 0, "0 visits for uri found in history database");
equal((await PlacesTestUtils.isPageInDB(visits[1].url)), false, "expected uri not found in history database");
equal((await PlacesTestUtils.visitsInDB(visits[1].url)), 0, "0 visits for uri found in history database");
ok((await PlacesTestUtils.isPageInDB(visits[3].url)), "expected uri found in history database");
extension.sendMessage("delete-all");
[historyClearedCount, removedUrls] = await extension.awaitMessage("history-cleared");
- equal(PlacesUtils.history.hasHistoryEntries, false, "history is empty");
equal(historyClearedCount, 2, "onVisitRemoved called for each clearing of history");
equal(removedUrls.length, 3, "onVisitRemoved called the expected number of times");
for (let i = 1; i < 3; ++i) {
let url = visits[i].url;
ok(removedUrls.includes(url), `${url} received by onVisitRemoved`);
}
await extension.unload();
});
--- a/toolkit/components/places/nsINavHistoryService.idl
+++ b/toolkit/components/places/nsINavHistoryService.idl
@@ -1268,24 +1268,16 @@ interface nsINavHistoryService : nsISupp
const unsigned short DATABASE_STATUS_LOCKED = 4;
/**
* Returns the current database status
*/
readonly attribute unsigned short databaseStatus;
/**
- * True if there is any history. This can be used in UI to determine whether
- * the "clear history" button should be enabled or not. This is much better
- * than using BrowserHistory.count since that can be very slow if there is
- * a lot of history (it must enumerate each item). This is pretty fast.
- */
- readonly attribute boolean hasHistoryEntries;
-
- /**
* This is just like markPageAsTyped (in nsIBrowserHistory, also implemented
* by the history service), but for bookmarks. It declares that the given URI
* is being opened as a result of following a bookmark. If this URI is loaded
* soon after this message has been received, that transition will be marked
* as following a bookmark.
*/
void markPageAsFollowedBookmark(in nsIURI aURI);
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -782,22 +782,20 @@ nsNavHistory::DomainNameFromURI(nsIURI *
}
// just return the original hostname
// (it's also possible the host is an IP address)
aURI->GetAsciiHost(aDomainName);
}
-NS_IMETHODIMP
-nsNavHistory::GetHasHistoryEntries(bool* aHasEntries)
+bool
+nsNavHistory::hasHistoryEntries()
{
- NS_ENSURE_ARG_POINTER(aHasEntries);
- *aHasEntries = GetDaysOfHistory() > 0;
- return NS_OK;
+ return GetDaysOfHistory() > 0;
}
namespace {
class InvalidateAllFrecenciesCallback : public AsyncStatementCallback
{
public:
--- a/toolkit/components/places/nsNavHistory.h
+++ b/toolkit/components/places/nsNavHistory.h
@@ -327,16 +327,22 @@ public:
/**
* Returns any recent activity done with a URL.
* @return Any recent events associated with this URI. Each bit is set
* according to RecentEventFlags enum values.
*/
uint32_t GetRecentFlags(nsIURI *aURI);
/**
+ * Whether there are visits.
+ * Note: This may cause synchronous I/O.
+ */
+ bool hasHistoryEntries();
+
+ /**
* Registers a TRANSITION_EMBED visit for the session.
*
* @param aURI
* URI of the page.
* @param aTime
* Visit time. Only the last registered visit time is retained.
*/
void registerEmbedVisit(nsIURI* aURI, int64_t aTime);
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -2054,17 +2054,18 @@ nsNavHistoryQueryResultNode::GetHasChild
}
// For history containers query we must check if we have any history
if (resultType == nsINavHistoryQueryOptions::RESULTS_AS_DATE_QUERY ||
resultType == nsINavHistoryQueryOptions::RESULTS_AS_DATE_SITE_QUERY ||
resultType == nsINavHistoryQueryOptions::RESULTS_AS_SITE_QUERY) {
nsNavHistory* history = nsNavHistory::GetHistoryService();
NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
- return history->GetHasHistoryEntries(aHasChildren);
+ *aHasChildren = history->hasHistoryEntries();
+ return NS_OK;
}
//XXX: For other containers queries we must:
// 1. If it's open, just check mChildren for containers
// 2. Else null the view (keep it in a var), open container, check mChildren
// for containers, close container, reset the view
if (mContentsValid) {
--- a/toolkit/components/places/tests/unit/test_browserhistory.js
+++ b/toolkit/components/places/tests/unit/test_browserhistory.js
@@ -2,24 +2,30 @@
/* vim:set ts=2 sw=2 sts=2 et: */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
const TEST_URI = NetUtil.newURI("http://mozilla.com/");
const TEST_SUBDOMAIN_URI = NetUtil.newURI("http://foobar.mozilla.com/");
+async function checkEmptyHistory() {
+ let db = await PlacesUtils.promiseDBConnection();
+ let rows = await db.executeCached("SELECT count(*) FROM moz_historyvisits");
+ return !rows[0].getResultByIndex(0);
+}
+
add_task(async function test_addPage() {
await PlacesTestUtils.addVisits(TEST_URI);
- Assert.equal(1, PlacesUtils.history.hasHistoryEntries);
+ Assert.ok(!await checkEmptyHistory(), "History has entries");
});
add_task(async function test_removePage() {
await PlacesUtils.history.remove(TEST_URI);
- Assert.equal(0, PlacesUtils.history.hasHistoryEntries);
+ Assert.ok(await checkEmptyHistory(), "History is empty");
});
add_task(async function test_removePages() {
let pages = [];
for (let i = 0; i < 8; i++) {
pages.push(NetUtil.newURI(TEST_URI.spec + i));
}
@@ -37,17 +43,17 @@ add_task(async function test_removePages
url: pages[BOOKMARK_INDEX],
title: "test bookmark"
});
PlacesUtils.annotations.setPageAnnotation(pages[BOOKMARK_INDEX],
ANNO_NAME, ANNO_VALUE, 0,
Ci.nsIAnnotationService.EXPIRE_NEVER);
await PlacesUtils.history.remove(pages);
- Assert.equal(0, PlacesUtils.history.hasHistoryEntries);
+ Assert.ok(await checkEmptyHistory(), "History is empty");
// Check that the bookmark and its annotation still exist.
let folder = await PlacesUtils.getFolderContents(PlacesUtils.unfiledBookmarksFolderId);
Assert.equal(folder.root.childCount, 1);
Assert.equal(PlacesUtils.annotations.getPageAnnotation(pages[BOOKMARK_INDEX], ANNO_NAME),
ANNO_VALUE);
// Check the annotation on the non-bookmarked page does not exist anymore.
@@ -85,49 +91,27 @@ add_task(async function test_removePages
i > 0 && i < 9);
}
// Clear remaining items and check that all pages have been removed.
await PlacesUtils.history.removeByFilter({
beginDate: PlacesUtils.toDate(startDate),
endDate: PlacesUtils.toDate(startDate + 9000)
});
- Assert.equal(0, PlacesUtils.history.hasHistoryEntries);
+ Assert.ok(await checkEmptyHistory(), "History is empty");
});
add_task(async function test_removePagesFromHost() {
await PlacesTestUtils.addVisits(TEST_URI);
await PlacesUtils.history.removeByFilter({ host: ".mozilla.com" });
- Assert.equal(0, PlacesUtils.history.hasHistoryEntries);
+ Assert.ok(await checkEmptyHistory(), "History is empty");
});
add_task(async function test_removePagesFromHost_keepSubdomains() {
await PlacesTestUtils.addVisits([{ uri: TEST_URI }, { uri: TEST_SUBDOMAIN_URI }]);
await PlacesUtils.history.removeByFilter({ host: "mozilla.com" });
- Assert.equal(1, PlacesUtils.history.hasHistoryEntries);
+ Assert.ok(!await checkEmptyHistory(), "History has entries");
});
add_task(async function test_history_clear() {
await PlacesUtils.history.clear();
- Assert.equal(0, PlacesUtils.history.hasHistoryEntries);
+ Assert.ok(await checkEmptyHistory(), "History is empty");
});
-
-add_task(async function test_getObservers() {
- // Ensure that getObservers() invalidates the hasHistoryEntries cache.
- await PlacesTestUtils.addVisits(TEST_URI);
- Assert.equal(1, PlacesUtils.history.hasHistoryEntries);
- // This is just for testing purposes, never do it.
- return new Promise((resolve, reject) => {
- DBConn().executeSimpleSQLAsync("DELETE FROM moz_historyvisits", {
- handleError(error) {
- reject(error);
- },
- handleResult(result) {
- },
- handleCompletion(result) {
- // Just invoking getObservers should be enough to invalidate the cache.
- PlacesUtils.history.getObservers();
- Assert.equal(0, PlacesUtils.history.hasHistoryEntries);
- resolve();
- }
- });
- });
-});
--- a/toolkit/components/places/tests/unit/test_history_clear.js
+++ b/toolkit/components/places/tests/unit/test_history_clear.js
@@ -50,22 +50,18 @@ add_task(async function test_history_cle
{ uri: uri("http://frecency.mozilla.org/"),
transition: TRANSITION_LINK },
]);
await PlacesTestUtils.promiseAsyncUpdates();
// Clear history and wait for the onClearHistory notification.
let promiseClearHistory =
PlacesTestUtils.waitForNotification("onClearHistory", () => true, "history");
- PlacesUtils.history.clear();
+ await PlacesUtils.history.clear();
await promiseClearHistory;
-
- // check browserHistory returns no entries
- Assert.equal(0, PlacesUtils.history.hasHistoryEntries);
-
await PlacesTestUtils.promiseAsyncUpdates();
// Check that frecency for not cleared items (bookmarks) has been converted
// to -1.
let stmt = mDBConn.createStatement(
"SELECT h.id FROM moz_places h WHERE h.frecency > 0 ");
Assert.ok(!stmt.executeStep());
stmt.finalize();