Bug 1277235 - add typed and visitCount to onVisit. r=adw draft
authorMarco Bonardo <mbonardo@mozilla.com>
Wed, 01 Jun 2016 16:42:15 +0200
changeset 374408 cfa2e86ca9e9465d2f4e26411d864c932c874d60
parent 373916 b3abb2fa42dc520a855fa684868d4af36140eaa0
child 522623 4b2e9e2fcc0d4838e6cd9150ff70ff07bae5bab4
push id20010
push usermak77@bonardo.net
push dateThu, 02 Jun 2016 09:50:01 +0000
reviewersadw
bugs1277235
milestone49.0a1
Bug 1277235 - add typed and visitCount to onVisit. r=adw MozReview-Commit-ID: 3XfBCiOgyAu
toolkit/components/downloads/nsDownloadManager.cpp
toolkit/components/places/History.cpp
toolkit/components/places/nsINavHistoryService.idl
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/nsNavHistory.h
toolkit/components/places/nsNavHistoryResult.cpp
toolkit/components/places/nsNavHistoryResult.h
toolkit/components/places/tests/unit/test_history_observer.js
--- a/toolkit/components/downloads/nsDownloadManager.cpp
+++ b/toolkit/components/downloads/nsDownloadManager.cpp
@@ -2323,17 +2323,17 @@ nsDownloadManager::OnEndUpdateBatch()
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDownloadManager::OnVisit(nsIURI *aURI, int64_t aVisitID, PRTime aTime,
                            int64_t aSessionID, int64_t aReferringID,
                            uint32_t aTransitionType, const nsACString& aGUID,
-                           bool aHidden)
+                           bool aHidden, uint32_t aVisitCount, uint32_t aTyped)
 {
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDownloadManager::OnTitleChanged(nsIURI *aURI,
                                   const nsAString &aPageTitle,
                                   const nsACString &aGUID)
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -74,16 +74,17 @@ struct VisitData {
   , visitId(0)
   , hidden(true)
   , typed(false)
   , transitionType(UINT32_MAX)
   , visitTime(0)
   , frecency(-1)
   , lastVisitId(0)
   , lastVisitTime(0)
+  , visitCount(0)
   , referrerVisitId(0)
   , titleChanged(false)
   , shouldUpdateFrecency(true)
   {
     guid.SetIsVoid(true);
     title.SetIsVoid(true);
   }
 
@@ -93,16 +94,17 @@ struct VisitData {
   , visitId(0)
   , hidden(true)
   , typed(false)
   , transitionType(UINT32_MAX)
   , visitTime(0)
   , frecency(-1)
   , lastVisitId(0)
   , lastVisitTime(0)
+  , visitCount(0)
   , referrerVisitId(0)
   , titleChanged(false)
   , shouldUpdateFrecency(true)
   {
     MOZ_ASSERT(aURI);
     if (aURI) {
       (void)aURI->GetSpec(spec);
       (void)GetReversedHostname(aURI, revHost);
@@ -134,16 +136,17 @@ struct VisitData {
   nsString revHost;
   bool hidden;
   bool typed;
   uint32_t transitionType;
   PRTime visitTime;
   int32_t frecency;
   int64_t lastVisitId;
   PRTime lastVisitTime;
+  uint32_t visitCount;
 
   /**
    * Stores the title.  If this is empty (IsEmpty() returns true), then the
    * title should be removed from the Place.  If the title is void (IsVoid()
    * returns true), then no title has been set on this object, and titleChanged
    * should remain false.
    */
   nsString title;
@@ -640,17 +643,19 @@ public:
       return NS_ERROR_UNEXPECTED;
     }
 
     // Notify the visit.  Note that TRANSITION_EMBED visits are never added
     // to the database, thus cannot be queried and we don't notify them.
     if (mPlace.transitionType != nsINavHistoryService::TRANSITION_EMBED) {
       navHistory->NotifyOnVisit(uri, mPlace.visitId, mPlace.visitTime,
                                 mPlace.referrerVisitId, mPlace.transitionType,
-                                mPlace.guid, mPlace.hidden);
+                                mPlace.guid, mPlace.hidden,
+                                mPlace.visitCount + 1, // Add current visit.
+                                static_cast<uint32_t>(mPlace.typed));
     }
 
     nsCOMPtr<nsIObserverService> obsService =
       mozilla::services::GetObserverService();
     if (obsService) {
       DebugOnly<nsresult> rv =
         obsService->NotifyObservers(uri, URI_VISIT_SAVED, nullptr);
       NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Could not notify observers");
@@ -936,16 +941,18 @@ public:
       } else {
         // Copy over the data from the already known place.
         place.placeId = (*lastFetchedPlace).placeId;
         place.guid = (*lastFetchedPlace).guid;
         place.lastVisitId = (*lastFetchedPlace).visitId;
         place.lastVisitTime = (*lastFetchedPlace).visitTime;
         place.titleChanged = !(*lastFetchedPlace).title.Equals(place.title);
         place.frecency = (*lastFetchedPlace).frecency;
+        // Add one visit for the previous loop.
+        place.visitCount = ++(*lastFetchedPlace).visitCount;
       }
 
       // If any transition is typed, ensure the page is marked as typed.
       if (typed != lastFetchedPlace->typed) {
         place.typed = true;
       }
 
       // If any transition is visible, ensure the page is marked as visible.
@@ -2121,30 +2128,30 @@ History::FetchPageInfo(VisitData& _place
 
   nsresult rv;
 
   // URI takes precedence.
   nsCOMPtr<mozIStorageStatement> stmt;
   bool selectByURI = !_place.spec.IsEmpty();
   if (selectByURI) {
     stmt = GetStatement(
-      "SELECT guid, id, title, hidden, typed, frecency, last_visit_date, "
+      "SELECT guid, id, title, hidden, typed, frecency, visit_count, last_visit_date, "
       "(SELECT id FROM moz_historyvisits "
        "WHERE place_id = h.id AND visit_date = h.last_visit_date) AS last_visit_id "
       "FROM moz_places h "
       "WHERE url = :page_url "
     );
     NS_ENSURE_STATE(stmt);
 
     rv = URIBinder::Bind(stmt, NS_LITERAL_CSTRING("page_url"), _place.spec);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   else {
     stmt = GetStatement(
-      "SELECT url, id, title, hidden, typed, frecency, last_visit_date, "
+      "SELECT url, id, title, hidden, typed, frecency, visit_count, last_visit_date, "
       "(SELECT id FROM moz_historyvisits "
        "WHERE place_id = h.id AND visit_date = h.last_visit_date) AS last_visit_id "
       "FROM moz_places h "
       "WHERE guid = :guid "
     );
     NS_ENSURE_STATE(stmt);
 
     rv = stmt->BindUTF8StringByName(NS_LITERAL_CSTRING("guid"), _place.guid);
@@ -2200,19 +2207,23 @@ History::FetchPageInfo(VisitData& _place
 
   int32_t typed;
   rv = stmt->GetInt32(4, &typed);
   NS_ENSURE_SUCCESS(rv, rv);
   _place.typed = !!typed;
 
   rv = stmt->GetInt32(5, &_place.frecency);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = stmt->GetInt64(6, &_place.lastVisitTime);
+  int32_t visitCount;
+  rv = stmt->GetInt32(6, &visitCount);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = stmt->GetInt64(7, &_place.lastVisitId);
+  _place.visitCount = visitCount;
+  rv = stmt->GetInt64(7, &_place.lastVisitTime);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = stmt->GetInt64(8, &_place.lastVisitId);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 MOZ_DEFINE_MALLOC_SIZE_OF(HistoryMallocSizeOf)
 
 NS_IMETHODIMP
--- a/toolkit/components/places/nsINavHistoryService.idl
+++ b/toolkit/components/places/nsINavHistoryService.idl
@@ -639,40 +639,54 @@ interface nsINavHistoryObserver : nsISup
 
   /**
    * Notifies you that we are done doing a bunch of things and you should go
    * ahead and update UI, etc.
    */
   void onEndUpdateBatch();
 
   /**
-   * Called when a resource is visited. This is called the first time a
-   * resource (page, image, etc.) is seen as well as every subsequent time.
+   * Called everytime a URI is visited.
    *
-   * Normally, transition types of TRANSITION_EMBED (corresponding to images in
-   * a page, for example) are not displayed in history results (unless
-   * includeHidden is set). Many observers can ignore _EMBED notifications
-   * (which will comprise the majority of visit notifications) to save work.
+   * @note TRANSITION_EMBED visits (corresponding to images in a page, for
+   *       example) are not displayed in history results. Most observers can
+   *       ignore TRANSITION_EMBED visit notifications (which will comprise the
+   *       majority of visit notifications) to save work.
    *
-   * @param aVisitID        ID of the visit that was just created.
-   * @param aTime           Time of the visit
-   * @param aSessionID      No longer supported (always set to 0).
-   * @param aReferringID    The ID of the visit the user came from. 0 if empty.
-   * @param aTransitionType One of nsINavHistory.TRANSITION_*
-   * @param aGUID           The unique ID associated with the page.
-   * @param aHidden         Whether the visited page is marked as hidden.
+   * @param aVisitId
+   *        Id of the visit that was just created.
+   * @param aTime
+   *        Time of the visit.
+   * @param aSessionId
+   *        No longer supported and always set to 0.
+   * @param aReferrerVisitId
+   *        The id of the visit the user came from, defaults to 0 for no referrer.
+   * @param aTransitionType
+   *        One of nsINavHistory.TRANSITION_*
+   * @param aGuid
+   *        The unique id associated with the page.
+   * @param aHidden
+   *        Whether the visited page is marked as hidden.
+   * @param aVisitCount
+   *        Number of visits (included this one) for this URI.
+   * @param aTyped
+   *        Whether the URI has been typed or not.
+   *        TODO (Bug 1271801): This will become a count, rather than a boolean.
+   *        For future compatibility, always compare it with "> 0".
    */
   void onVisit(in nsIURI aURI,
-               in long long aVisitID,
+               in long long aVisitId,
                in PRTime aTime,
-               in long long aSessionID,
-               in long long aReferringID,
+               in long long aSessionId,
+               in long long aReferrerVisitId,
                in unsigned long aTransitionType,
-               in ACString aGUID,
-               in boolean aHidden);
+               in ACString aGuid,
+               in boolean aHidden,
+               in unsigned long aVisitCount,
+               in unsigned long aTyped);
 
   /**
    * Called whenever either the "real" title or the custom title of the page
    * changed. BOTH TITLES ARE ALWAYS INCLUDED in this notification, even though
    * only one will change at a time. Often, consumers will want to display the
    * user title if it is available, and fall back to the page title (the one
    * specified in the <title> tag of the page).
    *
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -2663,17 +2663,17 @@ nsNavBookmarks::OnEndUpdateBatch()
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsNavBookmarks::OnVisit(nsIURI* aURI, int64_t aVisitId, PRTime aTime,
                         int64_t aSessionID, int64_t aReferringID,
                         uint32_t aTransitionType, const nsACString& aGUID,
-                        bool aHidden)
+                        bool aHidden, uint32_t aVisitCount, uint32_t aTyped)
 {
   NS_ENSURE_ARG(aURI);
 
   // If the page is bookmarked, notify observers for each associated bookmark.
   ItemVisitData visitData;
   nsresult rv = aURI->GetSpec(visitData.bookmark.url);
   NS_ENSURE_SUCCESS(rv, rv);
   visitData.visitId = aVisitId;
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -477,38 +477,40 @@ nsNavHistory::LoadPrefs()
   FRECENCY_PREF(mDefaultWeight,            PREF_FREC_DEFAULT_BUCKET_WEIGHT);
 
 #undef FRECENCY_PREF
 }
 
 
 void
 nsNavHistory::NotifyOnVisit(nsIURI* aURI,
-                          int64_t aVisitID,
+                          int64_t aVisitId,
                           PRTime aTime,
-                          int64_t referringVisitID,
+                          int64_t aReferrerVisitId,
                           int32_t aTransitionType,
-                          const nsACString& aGUID,
-                          bool aHidden)
+                          const nsACString& aGuid,
+                          bool aHidden,
+                          uint32_t aVisitCount,
+                          uint32_t aTyped)
 {
-  MOZ_ASSERT(!aGUID.IsEmpty());
+  MOZ_ASSERT(!aGuid.IsEmpty());
   // If there's no history, this visit will surely add a day.  If the visit is
   // added before or after the last cached day, the day count may have changed.
   // Otherwise adding multiple visits in the same day should not invalidate
   // the cache.
   if (mDaysOfHistory == 0) {
     mDaysOfHistory = 1;
   } else if (aTime > mLastCachedEndOfDay || aTime < mLastCachedStartOfDay) {
     mDaysOfHistory = -1;
   }
 
   NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
                    nsINavHistoryObserver,
-                   OnVisit(aURI, aVisitID, aTime, 0,
-                           referringVisitID, aTransitionType, aGUID, aHidden));
+                   OnVisit(aURI, aVisitId, aTime, 0, aReferrerVisitId,
+                           aTransitionType, aGuid, aHidden, aVisitCount, aTyped));
 }
 
 void
 nsNavHistory::NotifyTitleChange(nsIURI* aURI,
                                 const nsString& aTitle,
                                 const nsACString& aGUID)
 {
   MOZ_ASSERT(!aGUID.IsEmpty());
--- a/toolkit/components/places/nsNavHistory.h
+++ b/toolkit/components/places/nsNavHistory.h
@@ -424,22 +424,24 @@ public:
   {
     return mNumVisitsForFrecency;
   }
 
   /**
    * Fires onVisit event to nsINavHistoryService observers
    */
   void NotifyOnVisit(nsIURI* aURI,
-                     int64_t aVisitID,
+                     int64_t aVisitId,
                      PRTime aTime,
-                     int64_t referringVisitID,
+                     int64_t aReferrerVisitId,
                      int32_t aTransitionType,
-                     const nsACString& aGUID,
-                     bool aHidden);
+                     const nsACString& aGuid,
+                     bool aHidden,
+                     uint32_t aVisitCount,
+                     uint32_t aTyped);
 
   /**
    * Fires onTitleChanged event to nsINavHistoryService observers
    */
   void NotifyTitleChange(nsIURI* aURI,
                          const nsString& title,
                          const nsACString& aGUID);
 
--- a/toolkit/components/places/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/nsNavHistoryResult.cpp
@@ -4592,20 +4592,25 @@ nsNavHistoryResult::OnItemMoved(int64_t 
   return NS_OK;
 }
 
 
 NS_IMETHODIMP
 nsNavHistoryResult::OnVisit(nsIURI* aURI, int64_t aVisitId, PRTime aTime,
                             int64_t aSessionId, int64_t aReferringId,
                             uint32_t aTransitionType, const nsACString& aGUID,
-                            bool aHidden)
+                            bool aHidden, uint32_t aVisitCount, uint32_t aTyped)
 {
   NS_ENSURE_ARG(aURI);
 
+  // Embed visits are never shown in our views.
+  if (aTransitionType == nsINavHistoryService::TRANSITION_EMBED) {
+    return NS_OK;
+  }
+
   uint32_t added = 0;
 
   ENUMERATE_HISTORY_OBSERVERS(OnVisit(aURI, aVisitId, aTime, aSessionId,
                                       aReferringId, aTransitionType, aGUID,
                                       aHidden, &added));
 
   if (!mRootNode->mExpanded)
     return NS_OK;
--- a/toolkit/components/places/nsNavHistoryResult.h
+++ b/toolkit/components/places/nsNavHistoryResult.h
@@ -89,17 +89,17 @@ private:
                      bool aHidden, uint32_t* aAdded);
 
 // The external version is used by results.
 #define NS_DECL_BOOKMARK_HISTORY_OBSERVER_EXTERNAL(...)                 \
   NS_DECL_BOOKMARK_HISTORY_OBSERVER_BASE(__VA_ARGS__)                   \
   NS_IMETHOD OnVisit(nsIURI* aURI, int64_t aVisitId, PRTime aTime,      \
                      int64_t aSessionId, int64_t aReferringId,          \
                      uint32_t aTransitionType, const nsACString& aGUID, \
-                     bool aHidden) __VA_ARGS__;
+                     bool aHidden, uint32_t aVisitCount, uint32_t aTyped) __VA_ARGS__;
 
 // nsNavHistoryResult
 //
 //    nsNavHistory creates this object and fills in mChildren (by getting
 //    it through GetTopLevel()). Then FilledAllResults() is called to finish
 //    object initialization.
 
 #define NS_NAVHISTORYRESULT_IID \
--- a/toolkit/components/places/tests/unit/test_history_observer.js
+++ b/toolkit/components/places/tests/unit/test_history_observer.js
@@ -20,25 +20,25 @@ NavHistoryObserver.prototype = {
 };
 
 /**
  * Registers a one-time history observer for and calls the callback
  * when the specified nsINavHistoryObserver method is called.
  * Returns a promise that is resolved when the callback returns.
  */
 function onNotify(callback) {
-  let deferred = Promise.defer();
-  let obs = new NavHistoryObserver();
-  obs[callback.name] = function () {
-    PlacesUtils.history.removeObserver(this);
-    callback.apply(this, arguments);
-    deferred.resolve();
-  };
-  PlacesUtils.history.addObserver(obs, false);
-  return deferred.promise;
+  return new Promise(resolve => {
+    let obs = new NavHistoryObserver();
+    obs[callback.name] = function () {
+      PlacesUtils.history.removeObserver(this);
+      callback.apply(this, arguments);
+      resolve();
+    };
+    PlacesUtils.history.addObserver(obs, false);
+  });
 }
 
 /**
  * Asynchronous task that adds a visit to the history database.
  */
 function* task_add_visit(uri, timestamp, transition) {
   uri = uri || NetUtil.newURI("http://firefox.com/");
   timestamp = timestamp || Date.now() * 1000;
@@ -49,108 +49,160 @@ function* task_add_visit(uri, timestamp,
   });
   return [uri, timestamp];
 }
 
 add_task(function* test_onVisit() {
   let promiseNotify = onNotify(function onVisit(aURI, aVisitID, aTime,
                                                 aSessionID, aReferringID,
                                                 aTransitionType, aGUID,
-                                                aHidden) {
-    do_check_true(aURI.equals(testuri));
-    do_check_true(aVisitID > 0);
-    do_check_eq(aTime, testtime);
-    do_check_eq(aSessionID, 0);
-    do_check_eq(aReferringID, 0);
-    do_check_eq(aTransitionType, TRANSITION_TYPED);
+                                                aHidden, aVisitCount, aTyped) {
+    Assert.ok(aURI.equals(testuri));
+    Assert.ok(aVisitID > 0);
+    Assert.equal(aTime, testtime);
+    Assert.equal(aSessionID, 0);
+    Assert.equal(aReferringID, 0);
+    Assert.equal(aTransitionType, TRANSITION_TYPED);
     do_check_guid_for_uri(aURI, aGUID);
-    do_check_false(aHidden);
+    Assert.ok(!aHidden);
+    Assert.equal(aVisitCount, 1);
+    Assert.equal(aTyped, 1);
   });
   let testuri = NetUtil.newURI("http://firefox.com/");
   let testtime = Date.now() * 1000;
   yield task_add_visit(testuri, testtime);
   yield promiseNotify;
 });
 
 add_task(function* test_onVisit() {
   let promiseNotify = onNotify(function onVisit(aURI, aVisitID, aTime,
                                                 aSessionID, aReferringID,
                                                 aTransitionType, aGUID,
-                                                aHidden) {
-    do_check_true(aURI.equals(testuri));
-    do_check_true(aVisitID > 0);
-    do_check_eq(aTime, testtime);
-    do_check_eq(aSessionID, 0);
-    do_check_eq(aReferringID, 0);
-    do_check_eq(aTransitionType, TRANSITION_FRAMED_LINK);
+                                                aHidden, aVisitCount, aTyped) {
+    Assert.ok(aURI.equals(testuri));
+    Assert.ok(aVisitID > 0);
+    Assert.equal(aTime, testtime);
+    Assert.equal(aSessionID, 0);
+    Assert.equal(aReferringID, 0);
+    Assert.equal(aTransitionType, TRANSITION_FRAMED_LINK);
     do_check_guid_for_uri(aURI, aGUID);
-    do_check_true(aHidden);
+    Assert.ok(aHidden);
+    Assert.equal(aVisitCount, 1);
+    Assert.equal(aTyped, 0);
   });
   let testuri = NetUtil.newURI("http://hidden.firefox.com/");
   let testtime = Date.now() * 1000;
   yield task_add_visit(testuri, testtime, TRANSITION_FRAMED_LINK);
   yield promiseNotify;
 });
 
+add_task(function* test_multiple_onVisit() {
+  let testuri = NetUtil.newURI("http://self.firefox.com/");
+  let promiseNotifications = new Promise(resolve => {
+    let observer = {
+      _c: 0,
+      __proto__: NavHistoryObserver.prototype,
+      onVisit(uri, id, time, undefined, referrerId, transition, guid,
+              hidden, visitCount, typed) {
+        Assert.ok(testuri.equals(uri));
+        Assert.ok(id > 0);
+        Assert.ok(time > 0);
+        Assert.ok(!hidden);
+        do_check_guid_for_uri(uri, guid);
+        switch (++this._c) {
+          case 1:
+            Assert.equal(referrerId, 0);
+            Assert.equal(transition, TRANSITION_LINK);
+            Assert.equal(visitCount, 1);
+            Assert.equal(typed, 0);
+            break;
+          case 2:
+            Assert.ok(referrerId > 0);
+            Assert.equal(transition, TRANSITION_LINK);
+            Assert.equal(visitCount, 2);
+            Assert.equal(typed, 0);
+            break;
+          case 3:
+            Assert.equal(referrerId, 0);
+            Assert.equal(transition, TRANSITION_TYPED);
+            Assert.equal(visitCount, 3);
+            Assert.equal(typed, 1);
+
+            PlacesUtils.history.removeObserver(observer, false);
+            resolve();
+            break;
+        }
+      }
+    };
+    PlacesUtils.history.addObserver(observer, false);
+  });
+  yield PlacesTestUtils.addVisits([
+    { uri: testuri, transition: TRANSITION_LINK },
+    { uri: testuri, referrer: testuri, transition: TRANSITION_LINK },
+    { uri: testuri, transition: TRANSITION_TYPED },
+  ]);
+  yield promiseNotifications;
+});
+
 add_task(function* test_onDeleteURI() {
   let promiseNotify = onNotify(function onDeleteURI(aURI, aGUID, aReason) {
-    do_check_true(aURI.equals(testuri));
+    Assert.ok(aURI.equals(testuri));
     // Can't use do_check_guid_for_uri() here because the visit is already gone.
-    do_check_eq(aGUID, testguid);
-    do_check_eq(aReason, Ci.nsINavHistoryObserver.REASON_DELETED);
+    Assert.equal(aGUID, testguid);
+    Assert.equal(aReason, Ci.nsINavHistoryObserver.REASON_DELETED);
   });
   let [testuri] = yield task_add_visit();
   let testguid = do_get_guid_for_uri(testuri);
   PlacesUtils.bhistory.removePage(testuri);
   yield promiseNotify;
 });
 
 add_task(function* test_onDeleteVisits() {
   let promiseNotify = onNotify(function onDeleteVisits(aURI, aVisitTime, aGUID,
                                                        aReason) {
-    do_check_true(aURI.equals(testuri));
+    Assert.ok(aURI.equals(testuri));
     // Can't use do_check_guid_for_uri() here because the visit is already gone.
-    do_check_eq(aGUID, testguid);
-    do_check_eq(aReason, Ci.nsINavHistoryObserver.REASON_DELETED);
-    do_check_eq(aVisitTime, 0); // All visits have been removed.
+    Assert.equal(aGUID, testguid);
+    Assert.equal(aReason, Ci.nsINavHistoryObserver.REASON_DELETED);
+    Assert.equal(aVisitTime, 0); // All visits have been removed.
   });
   let msecs24hrsAgo = Date.now() - (86400 * 1000);
   let [testuri] = yield task_add_visit(undefined, msecs24hrsAgo * 1000);
   // Add a bookmark so the page is not removed.
   PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
                                        testuri,
                                        PlacesUtils.bookmarks.DEFAULT_INDEX,
                                        "test");
   let testguid = do_get_guid_for_uri(testuri);
   PlacesUtils.bhistory.removePage(testuri);
   yield promiseNotify;
 });
 
 add_task(function* test_onTitleChanged() {
   let promiseNotify = onNotify(function onTitleChanged(aURI, aTitle, aGUID) {
-    do_check_true(aURI.equals(testuri));
-    do_check_eq(aTitle, title);
+    Assert.ok(aURI.equals(testuri));
+    Assert.equal(aTitle, title);
     do_check_guid_for_uri(aURI, aGUID);
   });
 
   let [testuri] = yield task_add_visit();
   let title = "test-title";
   yield PlacesTestUtils.addVisits({
     uri: testuri,
     title: title
   });
   yield promiseNotify;
 });
 
 add_task(function* test_onPageChanged() {
   let promiseNotify = onNotify(function onPageChanged(aURI, aChangedAttribute,
                                                       aNewValue, aGUID) {
-    do_check_eq(aChangedAttribute, Ci.nsINavHistoryObserver.ATTRIBUTE_FAVICON);
-    do_check_true(aURI.equals(testuri));
-    do_check_eq(aNewValue, SMALLPNG_DATA_URI.spec);
+    Assert.equal(aChangedAttribute, Ci.nsINavHistoryObserver.ATTRIBUTE_FAVICON);
+    Assert.ok(aURI.equals(testuri));
+    Assert.equal(aNewValue, SMALLPNG_DATA_URI.spec);
     do_check_guid_for_uri(aURI, aGUID);
   });
 
   let [testuri] = yield task_add_visit();
 
   // The new favicon for the page must have data associated with it in order to
   // receive the onPageChanged notification.  To keep this test self-contained,
   // we use an URI representing the smallest possible PNG file.