Bug 633266 - nsINavHistoryObserver: also pass in GUID whenever we pass in a URI. r=mak

Part 1: Tests

toolkit/components/places/tests/head_common.js

@@ -609,6 +609,34 @@ function do_check_valid_places_guid(aGuid,
   do_check_true(/^[a-zA-Z0-9\-_]{12}$/.test(aGuid), aStack);
 }
 
+/**
+ * Retrieves the guid for a given uri.
+ *
+ * @param aURI
+ *        The uri to check.
+ * @param [optional] aStack
+ *        The stack frame used to report the error.
+ * @return the associated the guid.
+ */
+function do_get_guid_for_uri(aURI,
+                             aStack)
+{
+  if (!aStack) {
+    aStack = Components.stack.caller;
+  }
+  let stmt = DBConn().createStatement(
+    "SELECT guid "
+  + "FROM moz_places "
+  + "WHERE url = :url "
+  );
+  stmt.params.url = aURI.spec;
+  do_check_true(stmt.executeStep(), aStack);
+  let guid = stmt.row.guid;
+  stmt.finalize();
+  do_check_valid_places_guid(guid, aStack);
+  return guid;
+}
+
 /**
  * Tests that a guid was set in moz_places for a given uri.
  *
@@ -621,19 +649,11 @@ function do_check_guid_for_uri(aURI,
                                aGUID)
 {
   let caller = Components.stack.caller;
-  let stmt = DBConn().createStatement(
-    "SELECT guid "
-  + "FROM moz_places "
-  + "WHERE url = :url "
-  );
-  stmt.params.url = aURI.spec;
-  do_check_true(stmt.executeStep(), caller);
-  do_check_valid_places_guid(stmt.row.guid, caller);
+  let guid = do_get_guid_for_uri(aURI, caller);
   if (aGUID) {
     do_check_valid_places_guid(aGUID, caller);
-    do_check_eq(stmt.row.guid, aGUID, caller);
+    do_check_eq(guid, aGUID, caller);
   }
-  stmt.finalize();
 }
 
 /**

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

@@ -107,9 +107,11 @@ TitleChangedObserver.prototype = {
  *        being visited.
  */
 function VisitObserver(aURI,
+                       aGUID,
                        aCallback)
 {
   this.uri = aURI;
+  this.guid = aGUID;
   this.callback = aCallback;
 }
 VisitObserver.prototype = {
@@ -119,12 +121,13 @@ VisitObserver.prototype = {
                     aTime,
                     aSessionId,
                     aReferringId,
-                    aTransitionType)
+                    aTransitionType,
+                    aGUID)
   {
     do_log_info("onVisit(" + aURI.spec + ", " + aVisitId + ", " + aTime +
                 ", " + aSessionId + ", " + aReferringId + ", " +
-                aTransitionType + ")");
-    if (!this.uri.equals(aURI)) {
+                aTransitionType + ", " + aGUID + ")"); 
+    if (!this.uri.equals(aURI) || this.guid != aGUID) {
       return;
     }
     this.callback(aTime, aTransitionType);
@@ -1173,6 +1176,7 @@ function test_visit_notifies()
   // There are two observers we need to see for each visit.  One is an
   // nsINavHistoryObserver and the other is the uri-visit-saved observer topic.
   let place = {
+    guid: "abcdefghijkl",
     uri: NetUtil.newURI(TEST_DOMAIN + "test_visit_notifies"),
     visits: [
       new VisitInfo(),
@@ -1186,8 +1190,9 @@ function test_visit_notifies()
       waitForAsyncUpdates(run_next_test);
     }
   }
-  let visitObserver = new VisitObserver(place.uri, function(aVisitDate,
-                                                            aTransitionType) {
+  let visitObserver = new VisitObserver(place.uri, place.guid,
+                                        function(aVisitDate,
+                                                 aTransitionType) {
     let visit = place.visits[0];
     do_check_eq(visit.visitDate, aVisitDate);
     do_check_eq(visit.transitionType, aTransitionType);

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

@@ -0,0 +1,91 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Generic nsINavHistoryObserver that doesn't implement anything, but provides
+ * dummy methods to prevent errors about an object not having a certain method.
+ */
+function NavHistoryObserver() {
+}
+NavHistoryObserver.prototype = {
+  onBeginUpdateBatch: function() { },
+  onEndUpdateBatch: function() { },
+  onVisit: function() { },
+  onTitleChanged: function() { },
+  onBeforeDeleteURI: function() { },
+  onDeleteURI: function() { },
+  onClearHistory: function() { },
+  onPageChanged: function() { },
+  onDeleteVisits: function() { },
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver])
+};
+
+/**
+ * Registers a one-time history observer for and calls the callback
+ * when the specified nsINavHistoryObserver method is called.
+ */
+function onNotify(callback) {
+  let obs = new NavHistoryObserver();
+  obs[callback.name] = function () {
+    PlacesUtils.history.removeObserver(this);
+    callback.apply(this, arguments);
+  };
+  PlacesUtils.history.addObserver(obs, false);
+}
+
+/**
+ * Adds a TRANSITION_TYPED visit to the history database.
+ */
+function add_visit(uri, timestamp) {
+  uri = uri || NetUtil.newURI("http://firefox.com/");
+  timestamp = timestamp || Date.now() * 1000;
+  PlacesUtils.history.addVisit(
+    uri, timestamp, null, Ci.nsINavHistoryService.TRANSITION_TYPED, false, 0);
+  return [uri, timestamp];
+}
+
+function run_test() {
+  run_next_test();
+}
+
+add_test(function test_onVisit() {
+  onNotify(function onVisit(aURI, aVisitID, aTime, aSessionID, aReferringID,
+                            aTransitionType, aGUID) {
+    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, Ci.nsINavHistoryService.TRANSITION_TYPED);
+    do_check_guid_for_uri(aURI, aGUID);
+
+    run_next_test();
+  });
+  let testuri = NetUtil.newURI("http://firefox.com/");
+  let testtime = Date.now() * 1000;
+  add_visit(testuri, testtime);
+});
+
+add_test(function test_onBeforeDeleteURI() {
+  onNotify(function onBeforeDeleteURI(aURI, aGUID) {
+    do_check_true(aURI.equals(testuri));
+    do_check_guid_for_uri(aURI, aGUID);
+
+    run_next_test();
+  });
+  let [testuri] = add_visit();
+  PlacesUtils.bhistory.removePage(testuri);
+});
+
+add_test(function test_onDeleteURI() {
+  onNotify(function onDeleteURI(aURI, aGUID) {
+    do_check_true(aURI.equals(testuri));
+    // Can't use do_check_guid_for_uri() here because the visit is already gone.
+    do_check_eq(aGUID, testguid);
+
+    run_next_test();
+  });
+  let [testuri] = add_visit();
+  let testguid = do_get_guid_for_uri(testuri);
+  PlacesUtils.bhistory.removePage(testuri);
+});

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

@@ -68,14 +68,17 @@ Observer.prototype =
   onTitleChanged: function(aURI, aPageTable)
   {
   },
-  onBeforeDeleteURI: function(aURI)
+  onBeforeDeleteURI: function(aURI, aGUID)
   {
     this.removedURI = aURI;
+    this.removedGUID = aGUID;
+    do_check_guid_for_uri(aURI, aGUID);
   },
-  onDeleteURI: function(aURI)
+  onDeleteURI: function(aURI, aGUID)
   {
     do_check_false(this.checked);
     do_check_true(this.removedURI.equals(aURI));
+    do_check_eq(this.removedGUID, aGUID);
     this.checked = true;
   },
   onPageChanged: function(aURI, aWhat, aValue)

toolkit/components/places/tests/unit/xpcshell.ini

@@ -64,6 +64,7 @@ tail =
 [test_history_catobs.js]
 [test_history_import.js]
 [test_history_notifications.js]
+[test_history_observer.js]
 [test_history_removeAllPages.js]
 [test_history_sidebar.js]
 [test_isvisited.js]