Bug 1117072 - updatePlaces can mistakenly overwrite typed and hidden attributes of a page. r=ttaubert, a=sledru
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 27 Jan 2015 18:36:40 +0100
changeset 243606 85a7c4ca81f9
parent 243605 03be92be95c2
child 243607 fff54c008d7d
push id4412
push usermak77@bonardo.net
push date2015-01-30 22:49 +0000
treeherdermozilla-beta@85a7c4ca81f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert, sledru
bugs1117072
milestone36.0
Bug 1117072 - updatePlaces can mistakenly overwrite typed and hidden attributes of a page. r=ttaubert, a=sledru
toolkit/components/places/History.cpp
toolkit/components/places/tests/unit/test_async_history_api.js
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -907,34 +907,50 @@ public:
     if (mHistory->IsShuttingDown()) {
       // If we were already shutting down, we cannot insert the URIs.
       return NS_OK;
     }
 
     mozStorageTransaction transaction(mDBConn, false,
                                       mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
-    VisitData* lastPlace = nullptr;
+    VisitData* lastFetchedPlace = nullptr;
     for (nsTArray<VisitData>::size_type i = 0; i < mPlaces.Length(); i++) {
       VisitData& place = mPlaces.ElementAt(i);
       VisitData& referrer = mReferrers.ElementAt(i);
 
+      // Fetching from the database can overwrite this information, so save it
+      // apart.
+      bool typed = place.typed;
+      bool hidden = place.hidden;
+
       // We can avoid a database lookup if it's the same place as the last
       // visit we added.
-      bool known = lastPlace && lastPlace->IsSamePlaceAs(place);
+      bool known = lastFetchedPlace && lastFetchedPlace->IsSamePlaceAs(place);
       if (!known) {
         nsresult rv = mHistory->FetchPageInfo(place, &known);
         if (NS_FAILED(rv)) {
           if (!!mCallback) {
             nsCOMPtr<nsIRunnable> event =
               new NotifyPlaceInfoCallback(mCallback, place, true, rv);
             return NS_DispatchToMainThread(event);
           }
           return NS_OK;
         }
+        lastFetchedPlace = &mPlaces.ElementAt(i);
+      }
+
+      // 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.
+      if (hidden != lastFetchedPlace->hidden) {
+        place.hidden = false;
       }
 
       FetchReferrerInfo(referrer, place);
 
       nsresult rv = DoDatabaseInserts(known, place, referrer);
       if (!!mCallback) {
         nsCOMPtr<nsIRunnable> event =
           new NotifyPlaceInfoCallback(mCallback, place, true, rv);
@@ -948,18 +964,16 @@ public:
       NS_ENSURE_SUCCESS(rv, rv);
 
       // Notify about title change if needed.
       if ((!known && !place.title.IsVoid()) || place.titleChanged) {
         event = new NotifyTitleObservers(place.spec, place.title, place.guid);
         rv = NS_DispatchToMainThread(event);
         NS_ENSURE_SUCCESS(rv, rv);
       }
-
-      lastPlace = &mPlaces.ElementAt(i);
     }
 
     nsresult rv = transaction.Commit();
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
 private:
@@ -2286,34 +2300,25 @@ History::FetchPageInfo(VisitData& _place
     _place.title = title;
   }
   // Otherwise, just indicate if the title has changed.
   else {
     _place.titleChanged = !(_place.title.Equals(title) ||
                             (_place.title.IsEmpty() && title.IsVoid()));
   }
 
-  if (_place.hidden) {
-    // If this transition was hidden, it is possible that others were not.
-    // Any one visible transition makes this location visible. If database
-    // has location as visible, reflect that in our data structure.
-    int32_t hidden;
-    rv = stmt->GetInt32(3, &hidden);
-    NS_ENSURE_SUCCESS(rv, rv);
-    _place.hidden = !!hidden;
-  }
-
-  if (!_place.typed) {
-    // If this transition wasn't typed, others might have been. If database
-    // has location as typed, reflect that in our data structure.
-    int32_t typed;
-    rv = stmt->GetInt32(4, &typed);
-    NS_ENSURE_SUCCESS(rv, rv);
-    _place.typed = !!typed;
-  }
+  int32_t hidden;
+  rv = stmt->GetInt32(3, &hidden);
+  NS_ENSURE_SUCCESS(rv, rv);
+  _place.hidden = !!hidden;
+
+  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);
   return NS_OK;
 }
 
 MOZ_DEFINE_MALLOC_SIZE_OF(HistoryMallocSizeOf)
 
--- a/toolkit/components/places/tests/unit/test_async_history_api.js
+++ b/toolkit/components/places/tests/unit/test_async_history_api.js
@@ -19,67 +19,61 @@ const RECENT_EVENT_THRESHOLD = 15 * 60 *
  *
  * @param [optional] aTransitionType
  *        The transition type of the visit.  Defaults to TRANSITION_LINK if not
  *        provided.
  * @param [optional] aVisitTime
  *        The time of the visit.  Defaults to now if not provided.
  */
 function VisitInfo(aTransitionType,
-                   aVisitTime)
-{
+                   aVisitTime) {
   this.transitionType =
     aTransitionType === undefined ? TRANSITION_LINK : aTransitionType;
   this.visitDate = aVisitTime || Date.now() * 1000;
 }
 
 function promiseUpdatePlaces(aPlaces) {
-  let deferred = Promise.defer();
-  PlacesUtils.asyncHistory.updatePlaces(aPlaces, {
-    _errors: [],
-    _results: [],
-    handleError: function handleError(aResultCode, aPlace) {
-      this._errors.push({ resultCode: aResultCode, info: aPlace});
-    },
-    handleResult: function handleResult(aPlace) {
-      this._results.push(aPlace);
-    },
-    handleCompletion: function handleCompletion() {
-      deferred.resolve({ errors: this._errors, results: this._results });
-    }
+  return new Promise((resolve, reject) => {
+    PlacesUtils.asyncHistory.updatePlaces(aPlaces, {
+      _errors: [],
+      _results: [],
+      handleError(aResultCode, aPlace) {
+        this._errors.push({ resultCode: aResultCode, info: aPlace});
+      },
+      handleResult(aPlace) {
+        this._results.push(aPlace);
+      },
+      handleCompletion() {
+        resolve({ errors: this._errors, results: this._results });
+      }
+    });
   });
-
-  return deferred.promise;
 }
 
 /**
  * Listens for a title change notification, and calls aCallback when it gets it.
  *
  * @param aURI
  *        The URI of the page we expect a notification for.
  * @param aExpectedTitle
  *        The expected title of the URI we expect a notification for.
  * @param aCallback
  *        The method to call when we have gotten the proper notification about
  *        the title changing.
  */
 function TitleChangedObserver(aURI,
                               aExpectedTitle,
-                              aCallback)
-{
+                              aCallback) {
   this.uri = aURI;
   this.expectedTitle = aExpectedTitle;
   this.callback = aCallback;
 }
 TitleChangedObserver.prototype = {
   __proto__: NavHistoryObserver.prototype,
-  onTitleChanged: function(aURI,
-                           aTitle,
-                           aGUID)
-  {
+  onTitleChanged(aURI, aTitle, aGUID) {
     do_log_info("onTitleChanged(" + aURI.spec + ", " + aTitle + ", " + aGUID + ")");
     if (!this.uri.equals(aURI)) {
       return;
     }
     do_check_eq(aTitle, this.expectedTitle);
     do_check_guid_for_uri(aURI, aGUID);
     this.callback();
   },
@@ -143,24 +137,22 @@ function do_check_title_for_uri(aURI,
   do_check_true(stmt.executeStep(), stack);
   do_check_eq(stmt.row.title, aTitle, stack);
   stmt.finalize();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Test Functions
 
-function test_interface_exists()
-{
+add_task(function* test_interface_exists() {
   let history = Cc["@mozilla.org/browser/history;1"].getService(Ci.nsISupports);
   do_check_true(history instanceof Ci.mozIAsyncHistory);
-}
+});
 
-function test_invalid_uri_throws()
-{
+add_task(function* test_invalid_uri_throws() {
   // First, test passing in nothing.
   let place = {
     visits: [
       new VisitInfo(),
     ],
   };
   try {
     yield promiseUpdatePlaces(place);
@@ -183,20 +175,19 @@ function test_invalid_uri_throws()
     try {
       yield promiseUpdatePlaces(place);
       do_throw("Should have thrown!");
     }
     catch (e) {
       do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
     }
   }
-}
+});
 
-function test_invalid_places_throws()
-{
+add_task(function* test_invalid_places_throws() {
   // First, test passing in nothing.
   try {
     PlacesUtils.asyncHistory.updatePlaces();
     do_throw("Should have thrown!");
   }
   catch (e) {
     do_check_eq(e.result, Cr.NS_ERROR_XPC_NOT_ENOUGH_ARGS);
   }
@@ -214,20 +205,19 @@ function test_invalid_places_throws()
     try {
       yield promiseUpdatePlaces(value);
       do_throw("Should have thrown!");
     }
     catch (e) {
       do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
     }
   }
-}
+});
 
-function test_invalid_guid_throws()
-{
+add_task(function* test_invalid_guid_throws() {
   // First check invalid length guid.
   let place = {
     guid: "BAD_GUID",
     uri: NetUtil.newURI(TEST_DOMAIN + "test_invalid_guid_throws"),
     visits: [
       new VisitInfo(),
     ],
   };
@@ -244,20 +234,19 @@ function test_invalid_guid_throws()
   do_check_eq(place.guid.length, 12);
   try {
     yield promiseUpdatePlaces(place);
     do_throw("Should have thrown!");
   }
   catch (e) {
     do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
   }
-}
+});
 
-function test_no_visits_throws()
-{
+add_task(function* test_no_visits_throws() {
   const TEST_URI =
     NetUtil.newURI(TEST_DOMAIN + "test_no_id_or_guid_no_visits_throws");
   const TEST_GUID = "_RANDOMGUID_";
   const TEST_PLACEID = 2;
 
   let log_test_conditions = function(aPlace) {
     let str = "Testing place with " +
       (aPlace.uri ? "uri" : "no uri") + ", " +
@@ -285,56 +274,53 @@ function test_no_visits_throws()
           do_throw("Should have thrown!");
         }
         catch (e) {
           do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
         }
       }
     }
   }
-}
+});
 
-function test_add_visit_no_date_throws()
-{
+add_task(function* test_add_visit_no_date_throws() {
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_add_visit_no_date_throws"),
     visits: [
       new VisitInfo(),
     ],
   };
   delete place.visits[0].visitDate;
   try {
     yield promiseUpdatePlaces(place);
     do_throw("Should have thrown!");
   }
   catch (e) {
     do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
   }
-}
+});
 
-function test_add_visit_no_transitionType_throws()
-{
+add_task(function* test_add_visit_no_transitionType_throws() {
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_add_visit_no_transitionType_throws"),
     visits: [
       new VisitInfo(),
     ],
   };
   delete place.visits[0].transitionType;
   try {
     yield promiseUpdatePlaces(place);
     do_throw("Should have thrown!");
   }
   catch (e) {
     do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
   }
-}
+});
 
-function test_add_visit_invalid_transitionType_throws()
-{
+add_task(function* test_add_visit_invalid_transitionType_throws() {
   // First, test something that has a transition type lower than the first one.
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN +
                         "test_add_visit_invalid_transitionType_throws"),
     visits: [
       new VisitInfo(TRANSITION_LINK - 1),
     ],
   };
@@ -350,20 +336,19 @@ function test_add_visit_invalid_transiti
   place.visits[0] = new VisitInfo(TRANSITION_FRAMED_LINK + 1);
   try {
     yield promiseUpdatePlaces(place);
     do_throw("Should have thrown!");
   }
   catch (e) {
     do_check_eq(e.result, Cr.NS_ERROR_INVALID_ARG);
   }
-}
+});
 
-function test_non_addable_uri_errors()
-{
+add_task(function* test_non_addable_uri_errors() {
   // Array of protocols that nsINavHistoryService::canAddURI returns false for.
   const URLS = [
     "about:config",
     "imap://cyrus.andrew.cmu.edu/archive.imap",
     "news://new.mozilla.org/mozilla.dev.apps.firefox",
     "mailbox:Inbox",
     "moz-anno:favicon:http://mozilla.org/made-up-favicon",
     "view-source:http://mozilla.org",
@@ -399,20 +384,19 @@ function test_non_addable_uri_errors()
     do_throw("Unexpected success.");
   }
   for (let place of placesResult.errors) {
     do_log_info("Checking '" + place.info.uri.spec + "'");
     do_check_eq(place.resultCode, Cr.NS_ERROR_INVALID_ARG);
     do_check_false(yield promiseIsURIVisited(place.info.uri));
   }
   yield promiseAsyncUpdates();
-}
+});
 
-function test_duplicate_guid_errors()
-{
+add_task(function* test_duplicate_guid_errors() {
   // This test ensures that trying to add a visit, with a guid already found in
   // another visit, fails.
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_duplicate_guid_fails_first"),
     visits: [
       new VisitInfo(),
     ],
   };
@@ -438,20 +422,19 @@ function test_duplicate_guid_errors()
   if (placesResult.results.length > 0) {
     do_throw("Unexpected success.");
   }
   let badPlaceInfo = placesResult.errors[0];
   do_check_eq(badPlaceInfo.resultCode, Cr.NS_ERROR_STORAGE_CONSTRAINT);
   do_check_false(yield promiseIsURIVisited(badPlaceInfo.info.uri));
 
   yield promiseAsyncUpdates();
-}
+});
 
-function test_invalid_referrerURI_ignored()
-{
+add_task(function* test_invalid_referrerURI_ignored() {
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN +
                         "test_invalid_referrerURI_ignored"),
     visits: [
       new VisitInfo(),
     ],
   };
   place.visits[0].referrerURI = NetUtil.newURI(place.uri.spec + "_unvisistedURI");
@@ -475,20 +458,19 @@ function test_invalid_referrerURI_ignore
      WHERE id = :visit_id`
   );
   stmt.params.visit_id = placeInfo.visits[0].visitId;
   do_check_true(stmt.executeStep());
   do_check_eq(stmt.row.from_visit, 0);
   stmt.finalize();
 
   yield promiseAsyncUpdates();
-}
+});
 
-function test_nonnsIURI_referrerURI_ignored()
-{
+add_task(function* test_nonnsIURI_referrerURI_ignored() {
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN +
                         "test_nonnsIURI_referrerURI_ignored"),
     visits: [
       new VisitInfo(),
     ],
   };
   place.visits[0].referrerURI = place.uri.spec + "_nonnsIURI";
@@ -508,20 +490,19 @@ function test_nonnsIURI_referrerURI_igno
      WHERE id = :visit_id`
   );
   stmt.params.visit_id = placeInfo.visits[0].visitId;
   do_check_true(stmt.executeStep());
   do_check_eq(stmt.row.from_visit, 0);
   stmt.finalize();
 
   yield promiseAsyncUpdates();
-}
+});
 
-function test_old_referrer_ignored()
-{
+add_task(function* test_old_referrer_ignored() {
   // This tests that a referrer for a visit which is not recent (specifically,
   // older than 15 minutes as per RECENT_EVENT_THRESHOLD) is not saved by
   // updatePlaces.
   let oldTime = (Date.now() * 1000) - (RECENT_EVENT_THRESHOLD + 1);
   let referrerPlace = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_old_referrer_ignored_referrer"),
     visits: [
       new VisitInfo(TRANSITION_LINK, oldTime),
@@ -568,20 +549,19 @@ function test_old_referrer_ignored()
      AND from_visit = 0`
   );
   stmt.params.page_url = place.uri.spec;
   do_check_true(stmt.executeStep());
   do_check_eq(stmt.row.count, 1);
   stmt.finalize();
 
   yield promiseAsyncUpdates();
-}
+});
 
-function test_place_id_ignored()
-{
+add_task(function* test_place_id_ignored() {
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_place_id_ignored_first"),
     visits: [
       new VisitInfo(),
     ],
   };
 
   do_check_false(yield promiseIsURIVisited(place.uri));
@@ -609,20 +589,19 @@ function test_place_id_ignored()
     do_throw("Unexpected error.");
   }
   placeInfo = placesResult.results[0];
 
   do_check_neq(placeInfo.placeId, placeId);
   do_check_true(yield promiseIsURIVisited(badPlace.uri));
 
   yield promiseAsyncUpdates();
-}
+});
 
-function test_handleCompletion_called_when_complete()
-{
+add_task(function* test_handleCompletion_called_when_complete() {
   // We test a normal visit, and embeded visit, and a uri that would fail
   // the canAddURI test to make sure that the notification happens after *all*
   // of them have had a callback.
   let places = [
     { uri: NetUtil.newURI(TEST_DOMAIN +
                           "test_handleCompletion_called_when_complete"),
       visits: [
         new VisitInfo(),
@@ -651,20 +630,19 @@ function test_handleCompletion_called_wh
   }
   for (let error of placesResult.errors) {
     callbackCountFailure++;
   }
 
   do_check_eq(callbackCountSuccess, EXPECTED_COUNT_SUCCESS);
   do_check_eq(callbackCountFailure, EXPECTED_COUNT_FAILURE);
   yield promiseAsyncUpdates();
-}
+});
 
-function test_add_visit()
-{
+add_task(function* test_add_visit() {
   const VISIT_TIME = Date.now() * 1000;
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_add_visit"),
     title: "test_add_visit title",
     visits: [],
   };
   for (let transitionType = TRANSITION_LINK;
        transitionType <= TRANSITION_FRAMED_LINK;
@@ -715,20 +693,19 @@ function test_add_visit()
       do_check_true(visit.visitId > 0);
     }
 
     // If we have had all of our callbacks, continue running tests.
     if (++callbackCount == place.visits.length) {
       yield promiseAsyncUpdates();
     }
   }
-}
+});
 
-function test_properties_saved()
-{
+add_task(function* test_properties_saved() {
   // Check each transition type to make sure it is saved properly.
   let places = [];
   for (let transitionType = TRANSITION_LINK;
        transitionType <= TRANSITION_FRAMED_LINK;
        transitionType++) {
     let place = {
       uri: NetUtil.newURI(TEST_DOMAIN + "test_properties_saved/" +
                           transitionType),
@@ -799,20 +776,19 @@ function test_properties_saved()
     do_check_eq(stmt.row.count, EXPECTED_COUNT);
     stmt.finalize();
 
     // If we have had all of our callbacks, continue running tests.
     if (++callbackCount == places.length) {
       yield promiseAsyncUpdates();
     }
   }
-}
+});
 
-function test_guid_saved()
-{
+add_task(function* test_guid_saved() {
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_guid_saved"),
     guid: "__TESTGUID__",
     visits: [
       new VisitInfo(),
     ],
   };
   do_check_valid_places_guid(place.guid);
@@ -823,20 +799,19 @@ function test_guid_saved()
     do_throw("Unexpected error.");
   }
   let placeInfo = placesResult.results[0];
   let uri = placeInfo.uri;
   do_check_true(yield promiseIsURIVisited(uri));
   do_check_eq(placeInfo.guid, place.guid);
   do_check_guid_for_uri(uri, place.guid);
   yield promiseAsyncUpdates();
-}
+});
 
-function test_referrer_saved()
-{
+add_task(function* test_referrer_saved() {
   let places = [
     { uri: NetUtil.newURI(TEST_DOMAIN + "test_referrer_saved/referrer"),
       visits: [
         new VisitInfo(),
       ],
     },
     { uri: NetUtil.newURI(TEST_DOMAIN + "test_referrer_saved/test"),
       visits: [
@@ -876,20 +851,19 @@ function test_referrer_saved()
       stmt.params.referrer = visit.referrerURI.spec;
       do_check_true(stmt.executeStep());
       do_check_eq(stmt.row.count, 1);
       stmt.finalize();
 
       yield promiseAsyncUpdates();
     }
   }
-}
+});
 
-function test_guid_change_saved()
-{
+add_task(function* test_guid_change_saved() {
   // First, add a visit for it.
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_guid_change_saved"),
     visits: [
       new VisitInfo(),
     ],
   };
   do_check_false(yield promiseIsURIVisited(place.uri));
@@ -903,20 +877,19 @@ function test_guid_change_saved()
   place.visits = [new VisitInfo()];
   placesResult = yield promiseUpdatePlaces(place);
   if (placesResult.errors.length > 0) {
     do_throw("Unexpected error.");
   }
   do_check_guid_for_uri(place.uri, place.guid);
 
   yield promiseAsyncUpdates();
-}
+});
 
-function test_title_change_saved()
-{
+add_task(function* test_title_change_saved() {
   // First, add a visit for it.
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_title_change_saved"),
     title: "original title",
     visits: [
       new VisitInfo(),
     ],
   };
@@ -950,20 +923,19 @@ function test_title_change_saved()
   place.visits = [new VisitInfo()];
   placesResult = yield promiseUpdatePlaces(place);
   if (placesResult.errors.length > 0) {
     do_throw("Unexpected error.");
   }
   do_check_title_for_uri(place.uri, place.title);
 
   yield promiseAsyncUpdates();
-}
+});
 
-function test_no_title_does_not_clear_title()
-{
+add_task(function* test_no_title_does_not_clear_title() {
   const TITLE = "test title";
   // First, add a visit for it.
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_no_title_does_not_clear_title"),
     title: TITLE,
     visits: [
       new VisitInfo(),
     ],
@@ -979,20 +951,19 @@ function test_no_title_does_not_clear_ti
   place.visits = [new VisitInfo()];
   placesResult = yield promiseUpdatePlaces(place);
   if (placesResult.errors.length > 0) {
     do_throw("Unexpected error.");
   }
   do_check_title_for_uri(place.uri, TITLE);
 
   yield promiseAsyncUpdates();
-}
+});
 
-function test_title_change_notifies()
-{
+add_task(function* test_title_change_notifies() {
   // There are three cases to test.  The first case is to make sure we do not
   // get notified if we do not specify a title.
   let place = {
     uri: NetUtil.newURI(TEST_DOMAIN + "test_title_change_notifies"),
     visits: [
       new VisitInfo(),
     ],
   };
@@ -1010,96 +981,94 @@ function test_title_change_notifies()
   }
 
   // The second case to test is that we get the notification when we add
   // it for the first time.  The first case will fail before our callback if it
   // is busted, so we can do this now.
   place.uri = NetUtil.newURI(place.uri.spec + "/new-visit-with-title");
   place.title = "title 1";
   function promiseTitleChangedObserver(aPlace) {
-    let deferred = Promise.defer();
-    let callbackCount = 0;
-    let observer = new TitleChangedObserver(aPlace.uri, aPlace.title, function() {
-      switch (++callbackCount) {
-        case 1:
-          // The third case to test is to make sure we get a notification when
-          // we change an existing place.
-          observer.expectedTitle = place.title = "title 2";
-          place.visits = [new VisitInfo()];
-          PlacesUtils.asyncHistory.updatePlaces(place);
-          break;
-        case 2:
-          PlacesUtils.history.removeObserver(silentObserver);
-          PlacesUtils.history.removeObserver(observer);
-          deferred.resolve();
-          break;
-      };
+    return new Promise((resolve, reject) => {
+      let callbackCount = 0;
+      let observer = new TitleChangedObserver(aPlace.uri, aPlace.title, function() {
+        switch (++callbackCount) {
+          case 1:
+            // The third case to test is to make sure we get a notification when
+            // we change an existing place.
+            observer.expectedTitle = place.title = "title 2";
+            place.visits = [new VisitInfo()];
+            PlacesUtils.asyncHistory.updatePlaces(place);
+            break;
+          case 2:
+            PlacesUtils.history.removeObserver(silentObserver);
+            PlacesUtils.history.removeObserver(observer);
+            resolve();
+            break;
+        };
+      });
+
+      PlacesUtils.history.addObserver(observer, false);
+      PlacesUtils.asyncHistory.updatePlaces(aPlace);
     });
-
-    PlacesUtils.history.addObserver(observer, false);
-    PlacesUtils.asyncHistory.updatePlaces(aPlace);
-    return deferred.promise;
   }
 
   yield promiseTitleChangedObserver(place);
   yield promiseAsyncUpdates();
-}
+});
 
-function test_visit_notifies()
-{
+add_task(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(),
     ],
   };
   do_check_false(yield promiseIsURIVisited(place.uri));
 
   function promiseVisitObserver(aPlace) {
-    let deferred = Promise.defer();
-    let callbackCount = 0;
-    let finisher = function() {
-      if (++callbackCount == 2) {
-        deferred.resolve();
+    return new Promise((resolve, reject) => {
+      let callbackCount = 0;
+      let finisher = function() {
+        if (++callbackCount == 2) {
+          resolve();
+        }
       }
-    }
-    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);
+      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);
 
-      PlacesUtils.history.removeObserver(visitObserver);
-      finisher();
+        PlacesUtils.history.removeObserver(visitObserver);
+        finisher();
+      });
+      PlacesUtils.history.addObserver(visitObserver, false);
+      let observer = function(aSubject, aTopic, aData) {
+        do_log_info("observe(" + aSubject + ", " + aTopic + ", " + aData + ")");
+        do_check_true(aSubject instanceof Ci.nsIURI);
+        do_check_true(aSubject.equals(place.uri));
+
+        Services.obs.removeObserver(observer, URI_VISIT_SAVED);
+        finisher();
+      };
+      Services.obs.addObserver(observer, URI_VISIT_SAVED, false);
+      PlacesUtils.asyncHistory.updatePlaces(place);
     });
-    PlacesUtils.history.addObserver(visitObserver, false);
-    let observer = function(aSubject, aTopic, aData) {
-      do_log_info("observe(" + aSubject + ", " + aTopic + ", " + aData + ")");
-      do_check_true(aSubject instanceof Ci.nsIURI);
-      do_check_true(aSubject.equals(place.uri));
-
-      Services.obs.removeObserver(observer, URI_VISIT_SAVED);
-      finisher();
-    };
-    Services.obs.addObserver(observer, URI_VISIT_SAVED, false);
-    PlacesUtils.asyncHistory.updatePlaces(place);
-    return deferred.promise;
   }
 
   yield promiseVisitObserver(place);
   yield promiseAsyncUpdates();
-}
+});
 
 // test with empty mozIVisitInfoCallback object
-function test_callbacks_not_supplied()
-{
+add_task(function* test_callbacks_not_supplied() {
   const URLS = [
     "imap://cyrus.andrew.cmu.edu/archive.imap",  // bad URI
     "http://mozilla.org/" // valid URI
   ];
   let places = [];
   URLS.forEach(function(url) {
     try {
       let place = {
@@ -1116,47 +1085,43 @@ function test_callbacks_not_supplied()
       // but the account is not set up and so the URL is invalid for us.
       // Note this in the log but ignore as it's not the subject of this test.
       do_log_info("Could not construct URI for '" + url + "'; ignoring");
     }
   });
 
   PlacesUtils.asyncHistory.updatePlaces(places, {});
   yield promiseAsyncUpdates();
-}
-
-////////////////////////////////////////////////////////////////////////////////
-//// Test Runner
+});
 
-[
-  test_interface_exists,
-  test_invalid_uri_throws,
-  test_invalid_places_throws,
-  test_invalid_guid_throws,
-  test_no_visits_throws,
-  test_add_visit_no_date_throws,
-  test_add_visit_no_transitionType_throws,
-  test_add_visit_invalid_transitionType_throws,
-  // Note: all asynchronous tests (every test below this point) should wait for
-  // async updates before calling run_next_test.
-  test_non_addable_uri_errors,
-  test_duplicate_guid_errors,
-  test_invalid_referrerURI_ignored,
-  test_nonnsIURI_referrerURI_ignored,
-  test_old_referrer_ignored,
-  test_place_id_ignored,
-  test_handleCompletion_called_when_complete,
-  test_add_visit,
-  test_properties_saved,
-  test_guid_saved,
-  test_referrer_saved,
-  test_guid_change_saved,
-  test_title_change_saved,
-  test_no_title_does_not_clear_title,
-  test_title_change_notifies,
-  test_visit_notifies,
-  test_callbacks_not_supplied,
-].forEach(add_task);
+// Test that we don't wrongly overwrite typed and hidden when adding new visits.
+add_task(function* test_typed_hidden_not_overwritten() {
+  yield promiseClearHistory();
+  let places = [
+    { uri: NetUtil.newURI("http://mozilla.org/"),
+      title: "test",
+      visits: [
+        new VisitInfo(TRANSITION_TYPED),
+        new VisitInfo(TRANSITION_LINK)
+      ]
+    },
+    { uri: NetUtil.newURI("http://mozilla.org/"),
+      title: "test",
+      visits: [
+        new VisitInfo(TRANSITION_FRAMED_LINK)
+      ]
+    },
+  ];
+  yield promiseUpdatePlaces(places);
+
+  let db = yield PlacesUtils.promiseDBConnection();
+  let rows = yield db.execute("SELECT hidden, typed FROM moz_places WHERE url = :url",
+                              { url: "http://mozilla.org/" });
+  Assert.equal(rows[0].getResultByName("typed"), 1,
+               "The page should be marked as typed");
+  Assert.equal(rows[0].getResultByName("hidden"), 0,
+               "The page should be marked as not hidden");
+});
 
 function run_test()
 {
   run_next_test();
 }