Bug 776872 - Remove calls to addVisit from the "bookmarks" folder. r=mak

toolkit/components/places/tests/bookmarks/test_async_observers.js

@@ -9,11 +9,22 @@ const NOW = Date.now() * 1000;
 let observer = {
   bookmarks: [],
   observedBookmarks: 0,
-  visitId: 0,
-  reset: function ()
+  observedVisitId: 0,
+  deferred: null,
+
+  /**
+   * Returns a promise that is resolved when the observer determines that the
+   * test can continue.  This is required rather than calling run_next_test
+   * directly in the observer because there are cases where we must wait for
+   * other asynchronous events to be completed in addition to this.
+   */
+  setupCompletionPromise: function ()
   {
     this.observedBookmarks = 0;
+    this.deferred = Promise.defer();
+    return this.deferred.promise;
   },
+
   onBeginUpdateBatch: function () {},
   onEndUpdateBatch: function () {},
   onItemAdded: function () {},
@@ -42,17 +53,17 @@ let observer = {
     }
 
     if (++this.observedBookmarks == this.bookmarks.length) {
-      run_next_test();
+      this.deferred.resolve();
     }
   },
   onItemVisited: function(aItemId, aVisitId, aTime)
   {
     do_log_info("Check that we got the correct visit information.");
     do_check_neq(this.bookmarks.indexOf(aItemId), -1);
-    do_check_eq(aVisitId, this.visitId);
+    this.observedVisitId = aVisitId;
     do_check_eq(aTime, NOW);
     if (++this.observedBookmarks == this.bookmarks.length) {
-      run_next_test();
+      this.deferred.resolve();
     }
   },
 
@@ -62,33 +73,56 @@ let observer = {
 };
 PlacesUtils.bookmarks.addObserver(observer, false);
 
-let gTests = [
-  function add_visit_test()
-  {
-    observer.reset();
-    // Add a visit to the bookmark and wait for the observer.
-    observer.visitId =
-      PlacesUtils.history.addVisit(NetUtil.newURI("http://book.ma.rk/"), NOW, null,
-                                   PlacesUtils.history.TRANSITION_TYPED, false, 0);
-  },
-  function add_icon_test()
-  {
-    observer.reset();
-    PlacesUtils.favicons.setAndFetchFaviconForPage(NetUtil.newURI("http://book.ma.rk/"),
+add_task(function test_add_visit()
+{
+  let observerPromise = observer.setupCompletionPromise();
+
+  // Add a visit to the bookmark and wait for the observer.
+  let visitId;
+  let deferUpdatePlaces = Promise.defer();
+  PlacesUtils.asyncHistory.updatePlaces({
+    uri: NetUtil.newURI("http://book.ma.rk/"),
+    visits: [{ transitionType: TRANSITION_TYPED, visitDate: NOW }]
+  }, {
+    handleError: function TAV_handleError() {
+      deferUpdatePlaces.reject(new Error("Unexpected error in adding visit."));
+    },
+    handleResult: function (aPlaceInfo) {
+      visitId = aPlaceInfo.visits[0].visitId;
+    },
+    handleCompletion: function TAV_handleCompletion() {
+      deferUpdatePlaces.resolve();
+    }
+  });
+
+  // Wait for both the observer and the asynchronous update, in any order.
+  yield deferUpdatePlaces.promise;
+  yield observerPromise;
+
+  // Check that both asynchronous results are consistent.
+  do_check_eq(observer.observedVisitId, visitId);
+});
+
+add_task(function test_add_icon()
+{
+  let observerPromise = observer.setupCompletionPromise();
+  PlacesUtils.favicons.setAndFetchFaviconForPage(NetUtil.newURI("http://book.ma.rk/"),
                                                    SMALLPNG_DATA_URI, true,
                                                    PlacesUtils.favicons.FAVICON_LOAD_NON_PRIVATE);
-  },
-  function remove_page_test()
-  {
-    observer.reset();
-    PlacesUtils.history.removePage(NetUtil.newURI("http://book.ma.rk/"));
-  },
-  function cleanup()
-  {
-    PlacesUtils.bookmarks.removeObserver(observer, false);
-    run_next_test();
-  },
-];
+  yield observerPromise;
+});
+
+add_task(function test_remove_page()
+{
+  let observerPromise = observer.setupCompletionPromise();
+  PlacesUtils.history.removePage(NetUtil.newURI("http://book.ma.rk/"));
+  yield observerPromise;
+});
+
+add_task(function cleanup()
+{
+  PlacesUtils.bookmarks.removeObserver(observer, false);
+});
 
 function run_test()
 {

toolkit/components/places/tests/bookmarks/test_bookmarks.js

@@ -76,6 +76,10 @@ let bmStartIndex = 0;
 
 
 function run_test() {
+  run_next_test();
+}
+
+add_task(function test_bookmarks() {
   bs.addObserver(bookmarksObserver, false);
 
   // test special folders
@@ -612,7 +616,7 @@ function run_test() {
   // bug 378820
   let uri1 = uri("http://foo.tld/a");
   bs.insertBookmark(testRoot, uri1, bs.DEFAULT_INDEX, "");
-  hs.addVisit(uri1, Date.now() * 1000, null, hs.TRANSITION_TYPED, false, 0);
+  yield promiseAddVisits(uri1);
 
   // bug 646993 - test bookmark titles longer than the maximum allowed length
   let title15 = Array(TITLE_LENGTH_MAX + 5).join("X");
@@ -632,7 +636,7 @@ function run_test() {
   do_check_eq(bookmarksObserver._itemChangedValue, title15expected);
 
   testSimpleFolderResult();
-}
+});
 
 function testSimpleFolderResult() {
   // the time before we create a folder, in microseconds

toolkit/components/places/tests/bookmarks/test_getBookmarkedURIFor.js

@@ -11,35 +11,15 @@
 let hs = PlacesUtils.history;
 let bs = PlacesUtils.bookmarks;
 
-/**
- * Adds a fake redirect between two visits.
- */
-function addFakeRedirect(aSourceVisitId, aDestVisitId, aRedirectType) {
-  let stmt = DBConn().createStatement(
-    "UPDATE moz_historyvisits " +
-    "SET from_visit = :source, visit_type = :type " +
-    "WHERE id = :dest");
-  stmt.params.source = aSourceVisitId;
-  stmt.params.dest = aDestVisitId;
-  stmt.params.type = aRedirectType;
-  try {
-    stmt.executeStep();
-  }
-  finally {
-    stmt.finalize();
-  }
+function run_test() {
+  run_next_test();
 }
 
-function run_test() {
+add_task(function test_getBookmarkedURIFor() {
   let now = Date.now() * 1000;
   const sourceURI = uri("http://test.mozilla.org/");
   // Add a visit and a bookmark.
-  let sourceVisitId = hs.addVisit(sourceURI,
-                                  now,
-                                  null,
-                                  hs.TRANSITION_TYPED,
-                                  false,
-                                  0);
+  yield promiseAddVisits({ uri: sourceURI, visitDate: now });
   do_check_eq(bs.getBookmarkedURIFor(sourceURI), null);
 
   let sourceItemId = bs.insertBookmark(bs.unfiledBookmarksFolder,
@@ -50,12 +30,12 @@ function run_test() {
 
   // Add a redirected visit.
   const permaURI = uri("http://perma.mozilla.org/");
-  hs.addVisit(permaURI,
-              now++,
-              sourceURI,
-              hs.TRANSITION_REDIRECT_PERMANENT,
-              true,
-              0);
+  yield promiseAddVisits({
+    uri: permaURI,
+    transition: TRANSITION_REDIRECT_PERMANENT,
+    visitDate: now++,
+    referrer: sourceURI
+  });
   do_check_true(bs.getBookmarkedURIFor(sourceURI).equals(sourceURI));
   do_check_true(bs.getBookmarkedURIFor(permaURI).equals(sourceURI));
   // Add a bookmark to the destination.
@@ -72,12 +52,13 @@ function run_test() {
 
   // Add another redirected visit.
   const tempURI = uri("http://perma.mozilla.org/");
-  hs.addVisit(tempURI,
-              now++,
-              permaURI,
-              hs.TRANSITION_REDIRECT_TEMPORARY,
-              true,
-              0);
+  yield promiseAddVisits({
+    uri: tempURI,
+    transition: TRANSITION_REDIRECT_TEMPORARY,
+    visitDate: now++,
+    referrer: permaURI
+  });
+
   do_check_true(bs.getBookmarkedURIFor(sourceURI).equals(sourceURI));
   do_check_true(bs.getBookmarkedURIFor(tempURI).equals(sourceURI));
   // Add a bookmark to the destination.
@@ -100,4 +81,4 @@ function run_test() {
   // not be added to the database.
   do_check_eq(bs.getBookmarkedURIFor(uri("http://does.not.exist/")), null);
   do_check_false(page_in_database("http://does.not.exist/"));
-}
+});

toolkit/components/places/tests/bookmarks/test_nsINavBookmarkObserver.js

@@ -291,8 +291,7 @@ add_test(function onItemMoved_bookmark() {
         { name: "parentGuid", check: function (v) typeof(v) == "string" && /^[a-zA-Z0-9\-_]{12}$/.test(v) },
       ] },
   ];
-  PlacesUtils.history.addVisit(uri, Date.now() * 1000, null,
-                               PlacesUtils.history.TRANSITION_TYPED, false, 0);
+  promiseAddVisits({ uri: uri, transition: TRANSITION_TYPED });
 });
 
 add_test(function onItemRemoved_bookmark() {

toolkit/components/places/tests/bookmarks/test_savedsearches.js

@@ -144,92 +144,85 @@ add_test(function test_savedsearches_bookmarks() {
   run_next_test();
 });
 
-add_test(function test_savedsearches_history() {
+add_task(function test_savedsearches_history() {
   // add a visit that matches the search term
   var testURI = uri("http://" + searchTerm + ".com");
-  addVisits({ uri: testURI, title: searchTerm }, function afterAddVisits() {
+  yield promiseAddVisits({ uri: testURI, title: searchTerm });
 
-    // create a saved-search that matches the visit we added
-    var searchId = bmsvc.insertBookmark(testRoot,
-                                        uri("place:terms=" + searchTerm + "&excludeQueries=1&expandQueries=1&queryType=0"),
-                                        bmsvc.DEFAULT_INDEX, searchTerm);
+  // create a saved-search that matches the visit we added
+  var searchId = bmsvc.insertBookmark(testRoot,
+                                      uri("place:terms=" + searchTerm + "&excludeQueries=1&expandQueries=1&queryType=0"),
+                                      bmsvc.DEFAULT_INDEX, searchTerm);
 
-    // query for the test root, expandQueries=1
-    // the query should show up as a query container, with 1 child
+  // query for the test root, expandQueries=1
+  // the query should show up as a query container, with 1 child
+  try {
+    var options = histsvc.getNewQueryOptions();
+    options.expandQueries = 1;
+    var query = histsvc.getNewQuery();
+    query.setFolders([testRoot], 1);
+    var result = histsvc.executeQuery(query, options);
+    var rootNode = result.root;
+    rootNode.containerOpen = true;
+    var cc = rootNode.childCount;
+    do_check_eq(cc, 1);
+    for (var i = 0; i < cc; i++) {
+      var node = rootNode.getChild(i);
+      // test that query node type is container when expandQueries=1
+      do_check_eq(node.type, node.RESULT_TYPE_QUERY);
+      // test that queries (as containers) have valid itemId
+      do_check_eq(node.itemId, searchId);
+      node.QueryInterface(Ci.nsINavHistoryContainerResultNode);
+      node.containerOpen = true;
+
+      // test that queries have children when excludeItems=1
+      // test that query nodes don't show containers (shouldn't have our folder that matches)
+      // test that queries don't show themselves in query results (shouldn't have our saved search)
+      do_check_eq(node.childCount, 1);
+
+      // test that history visit shows in query results
+      var item = node.getChild(0);
+      do_check_eq(item.type, item.RESULT_TYPE_URI);
+      do_check_eq(item.itemId, -1); // history visit
+      do_check_eq(item.uri, testURI.spec); // history visit
+
+      // test live-update of query results - add a history visit that matches the query
+      yield promiseAddVisits({
+        uri: uri("http://foo.com"),
+        title: searchTerm + "blah"
+      });
+      do_check_eq(node.childCount, 2);
+
+      // test live-update of query results - delete a history visit that matches the query
+      bhist.removePage(uri("http://foo.com"));
+      do_check_eq(node.childCount, 1);
+      node.containerOpen = false;
+    }
+
+    // test live-update of moved queries
+    var tmpFolderId = bmsvc.createFolder(testRoot, "foo", bmsvc.DEFAULT_INDEX);
+    bmsvc.moveItem(searchId, tmpFolderId, bmsvc.DEFAULT_INDEX);
+    var tmpFolderNode = rootNode.getChild(0);
+    do_check_eq(tmpFolderNode.itemId, tmpFolderId);
+    tmpFolderNode.QueryInterface(Ci.nsINavHistoryContainerResultNode);
+    tmpFolderNode.containerOpen = true;
+    do_check_eq(tmpFolderNode.childCount, 1);
+
+    // test live-update of renamed queries
+    bmsvc.setItemTitle(searchId, "foo");
+    do_check_eq(tmpFolderNode.title, "foo");
+
+    // test live-update of deleted queries
+    bmsvc.removeItem(searchId);
     try {
-      var options = histsvc.getNewQueryOptions();
-      options.expandQueries = 1;
-      var query = histsvc.getNewQuery();
-      query.setFolders([testRoot], 1);
-      var result = histsvc.executeQuery(query, options);
-      var rootNode = result.root;
-      rootNode.containerOpen = true;
-      var cc = rootNode.childCount;
-      do_check_eq(cc, 1);
-      for (var i = 0; i < cc; i++) {
-        var node = rootNode.getChild(i);
-        // test that query node type is container when expandQueries=1
-        do_check_eq(node.type, node.RESULT_TYPE_QUERY);
-        // test that queries (as containers) have valid itemId
-        do_check_eq(node.itemId, searchId);
-        node.QueryInterface(Ci.nsINavHistoryContainerResultNode);
-        node.containerOpen = true;
+      var tmpFolderNode = root.getChild(1);
+      do_throw("query was not removed");
+    } catch(ex) {}
 
-        // test that queries have children when excludeItems=1
-        // test that query nodes don't show containers (shouldn't have our folder that matches)
-        // test that queries don't show themselves in query results (shouldn't have our saved search)
-        do_check_eq(node.childCount, 1);
-
-        // test that history visit shows in query results
-        var item = node.getChild(0);
-        do_check_eq(item.type, item.RESULT_TYPE_URI);
-        do_check_eq(item.itemId, -1); // history visit
-        do_check_eq(item.uri, testURI.spec); // history visit
-
-        // test live-update of query results - add a history visit that matches the query
-        PlacesUtils.history.addVisit(uri("http://foo.com"),
-                                     Date.now() * 1000,
-                                     null,
-                                     Ci.nsINavHistoryService.TRANSITION_LINK,
-                                     false,
-                                     0);
-        PlacesUtils.ghistory2.setPageTitle(uri("http://foo.com"),
-                                           searchTerm + "blah");
-        do_check_eq(node.childCount, 2);
-
-        // test live-update of query results - delete a history visit that matches the query
-        bhist.removePage(uri("http://foo.com"));
-        do_check_eq(node.childCount, 1);
-        node.containerOpen = false;
-      }
-
-      // test live-update of moved queries
-      var tmpFolderId = bmsvc.createFolder(testRoot, "foo", bmsvc.DEFAULT_INDEX);
-      bmsvc.moveItem(searchId, tmpFolderId, bmsvc.DEFAULT_INDEX);
-      var tmpFolderNode = rootNode.getChild(0);
-      do_check_eq(tmpFolderNode.itemId, tmpFolderId);
-      tmpFolderNode.QueryInterface(Ci.nsINavHistoryContainerResultNode);
-      tmpFolderNode.containerOpen = true;
-      do_check_eq(tmpFolderNode.childCount, 1);
-
-      // test live-update of renamed queries
-      bmsvc.setItemTitle(searchId, "foo");
-      do_check_eq(tmpFolderNode.title, "foo");
-
-      // test live-update of deleted queries
-      bmsvc.removeItem(searchId);
-      try {
-        var tmpFolderNode = root.getChild(1);
-        do_throw("query was not removed");
-      } catch(ex) {}
-
-      tmpFolderNode.containerOpen = false;
-      rootNode.containerOpen = false;
-    }
-    catch(ex) {
-      do_throw("expandQueries=1 bookmarks query: " + ex);
-    }
-
-    run_next_test();
-  });
-});
+    tmpFolderNode.containerOpen = false;
+    rootNode.containerOpen = false;
+  }
+  catch(ex) {
+    do_throw("expandQueries=1 bookmarks query: " + ex);
+  }
+});