Bug 834541 - Remove the public History.hasHistoryEntries synchronous API. r=standard8

MozReview-Commit-ID: KJW9YNwoSZb

--HG--
extra : rebase_source : c379c145386478eb0cadfefe20c881f8652e6c2d

browser/components/extensions/test/xpcshell/test_ext_history.js

@@ -129,7 +129,6 @@ add_task(async function test_delete() {
 
   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) {

toolkit/components/places/nsINavHistoryService.idl

@@ -1272,14 +1272,6 @@ interface nsINavHistoryService : nsISupports
    */
   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

toolkit/components/places/nsNavHistory.cpp

@@ -787,12 +787,10 @@ nsNavHistory::DomainNameFromURI(nsIURI *aURI,
 }
 
 
-NS_IMETHODIMP
-nsNavHistory::GetHasHistoryEntries(bool* aHasEntries)
+bool
+nsNavHistory::hasHistoryEntries()
 {
-  NS_ENSURE_ARG_POINTER(aHasEntries);
-  *aHasEntries = GetDaysOfHistory() > 0;
-  return NS_OK;
+  return GetDaysOfHistory() > 0;
 }
 
 

toolkit/components/places/nsNavHistory.h

@@ -331,6 +331,12 @@ public:
    */
   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.
    *

toolkit/components/places/nsNavHistoryResult.cpp

@@ -2059,7 +2059,8 @@ nsNavHistoryQueryResultNode::GetHasChildren(bool* aHasChildren)
       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:

toolkit/components/places/tests/unit/test_browserhistory.js

@@ -7,14 +7,20 @@
 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() {
@@ -42,7 +48,7 @@ add_task(async function test_removePages() {
                                             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);
@@ -90,44 +96,22 @@ add_task(async function test_removePagesByTimeframe() {
     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);
-});
-
-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();
-      }
-    });
-  });
+  Assert.ok(await checkEmptyHistory(), "History is empty");
 });

toolkit/components/places/tests/unit/test_history_clear.js

@@ -55,12 +55,8 @@ add_task(async function test_history_clear() {
   // 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