Bug 1341097 - part 3b: fix tests that expect titlechanged notifications for first visits inserted, r?kitcambridge,mak draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 21 Feb 2017 19:59:59 +0000
changeset 487581 47d671aaead80bf8fb074440d6fad0d9bd07460a
parent 487580 85842494213968ff51e55825e787b3757e6c5801
child 487582 f7fc32fe0f98ee1ee2d25606a4793e8a45c5051c
push id46253
push usergijskruitbosch@gmail.com
push dateTue, 21 Feb 2017 20:02:31 +0000
reviewerskitcambridge, mak
bugs1341097
milestone54.0a1
Bug 1341097 - part 3b: fix tests that expect titlechanged notifications for first visits inserted, r?kitcambridge,mak This came up on try. I'll unify this with the previous commit but I wanted to split it out to get someone familiar with sync to verify my test change here is OK from sync's perspective (and my changing the test won't just wallpaper the test but leave sync broken). MozReview-Commit-ID: JS8NkbpWAsi
addon-sdk/source/lib/sdk/places/events.js
browser/components/newtab/tests/xpcshell/test_PlacesProvider.js
services/sync/tests/unit/test_history_store.js
--- a/addon-sdk/source/lib/sdk/places/events.js
+++ b/addon-sdk/source/lib/sdk/places/events.js
@@ -110,16 +110,30 @@ function formatValue (type, data) {
   if (type === 'type')
     return mapBookmarkItemType(data);
   if (type === 'url' && data)
     return data.spec;
   return data;
 }
 
 var historyObserver = createObserverInstance(HISTORY_EVENTS, HISTORY_ARGS);
+// Hack alert: we sometimes need to send extra title-changed notifications
+// ourselves for backwards compat. Replace the original onVisit callback to
+// add on that logic:
+historyObserver.realOnVisit = historyObserver.onVisit;
+historyObserver.onVisit = function(url, visitId, time, sessionId,
+                                   referringId, transitionType, guid, hidden,
+                                   visitCount, typed, lastKnownTitle) {
+  // If this is the first visit we're adding, fire title-changed
+  // in case anyone cares.
+  if (visitCount <= 1) {
+    historyObserver.onTitleChanged(url, lastKnownTitle);
+  }
+  this.realOnVisit(url, visitId, time, sessionId, referringId, transitionType);
+};
 historyService.addObserver(historyObserver, false);
 
 var bookmarkObserver = createObserverInstance(BOOKMARK_EVENTS, BOOKMARK_ARGS);
 bookmarkService.addObserver(bookmarkObserver, false);
 
 when(() => {
   historyService.removeObserver(historyObserver);
   bookmarkService.removeObserver(bookmarkObserver);
--- a/browser/components/newtab/tests/xpcshell/test_PlacesProvider.js
+++ b/browser/components/newtab/tests/xpcshell/test_PlacesProvider.js
@@ -140,25 +140,24 @@ add_task(function* test_Links_getLinks_D
 add_task(function* test_Links_onLinkChanged() {
   let provider = PlacesProvider.links;
 
   let url = "https://example.com/onFrecencyChanged1";
   let linkChangedMsgCount = 0;
 
   let linkChangedPromise = new Promise(resolve => {
     let handler = (_, link) => { // jshint ignore:line
-      /* There are 3 linkChanged events:
+      /* There are 2 linkChanged events:
        * 1. visit insertion (-1 frecency by default)
        * 2. frecency score update (after transition type calculation etc)
-       * 3. title change
        */
       if (link.url === url) {
         equal(link.url, url, `expected url on linkChanged event`);
         linkChangedMsgCount += 1;
-        if (linkChangedMsgCount === 3) {
+        if (linkChangedMsgCount === 2) {
           ok(true, `all linkChanged events captured`);
           provider.off("linkChanged", this);
           resolve();
         }
       }
     };
     provider.on("linkChanged", handler);
   });
--- a/services/sync/tests/unit/test_history_store.js
+++ b/services/sync/tests/unit/test_history_store.js
@@ -28,26 +28,27 @@ function queryPlaces(uri, options) {
 function queryHistoryVisits(uri) {
   let options = PlacesUtils.history.getNewQueryOptions();
   options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY;
   options.resultType = Ci.nsINavHistoryQueryOptions.RESULTS_AS_VISIT;
   options.sortingMode = Ci.nsINavHistoryQueryOptions.SORT_BY_DATE_ASCENDING;
   return queryPlaces(uri, options);
 }
 
-function onNextTitleChanged(callback) {
+function onNextVisit(callback) {
   PlacesUtils.history.addObserver({
     onBeginUpdateBatch: function onBeginUpdateBatch() {},
     onEndUpdateBatch: function onEndUpdateBatch() {},
     onPageChanged: function onPageChanged() {},
     onTitleChanged: function onTitleChanged() {
+    },
+    onVisit: function onVisit() {
       PlacesUtils.history.removeObserver(this);
       Utils.nextTick(callback);
     },
-    onVisit: function onVisit() {},
     onDeleteVisits: function onDeleteVisits() {},
     onPageExpired: function onPageExpired() {},
     onDeleteURI: function onDeleteURI() {},
     onClearHistory: function onClearHistory() {},
     QueryInterface: XPCOMUtils.generateQI([
       Ci.nsINavHistoryObserver,
       Ci.nsINavHistoryObserver_MOZILLA_1_9_1_ADDITIONS,
       Ci.nsISupportsWeakReference
@@ -120,17 +121,17 @@ add_test(function test_store() {
     do_check_eq(record.title, "Get Firefox!");
     do_check_eq(record.visits.length, 1);
     do_check_eq(record.visits[0].date, TIMESTAMP1);
     do_check_eq(record.visits[0].type, Ci.nsINavHistoryService.TRANSITION_LINK);
 
     _("Let's modify the record and have the store update the database.");
     let secondvisit = {date: TIMESTAMP2,
                        type: Ci.nsINavHistoryService.TRANSITION_TYPED};
-    onNextTitleChanged(ensureThrows(function() {
+    onNextVisit(ensureThrows(function() {
       let queryres = queryHistoryVisits(fxuri);
       do_check_eq(queryres.length, 2);
       do_check_eq(queryres[0].time, TIMESTAMP1);
       do_check_eq(queryres[0].title, "Hol Dir Firefox!");
       do_check_eq(queryres[1].time, TIMESTAMP2);
       do_check_eq(queryres[1].title, "Hol Dir Firefox!");
       run_next_test();
     }));
@@ -142,17 +143,17 @@ add_test(function test_store() {
     ]);
   }
 });
 
 add_test(function test_store_create() {
   _("Create a brand new record through the store.");
   tbguid = Utils.makeGUID();
   tburi = Utils.makeURI("http://getthunderbird.com");
-  onNextTitleChanged(ensureThrows(function() {
+  onNextVisit(ensureThrows(function() {
     do_check_attribute_count(store.getAllIDs(), 2);
     let queryres = queryHistoryVisits(tburi);
     do_check_eq(queryres.length, 1);
     do_check_eq(queryres[0].time, TIMESTAMP3);
     do_check_eq(queryres[0].title, "The bird is the word!");
     run_next_test();
   }));
   applyEnsureNoFailures([