Bug 628805 - Add test for unstored sessionId to test_async_history_api.js and fix the way session ids are stored with places visits [r=sdwilsh, a=blocking-final]
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Tue, 08 Feb 2011 13:15:18 -0800
changeset 62168 ef4d491f9ba0fbece23448cfce69710e1d639680
parent 62167 175623f0113a5a428eb6950710bf3936bd3b5e9f
child 62169 ee0596d83d99e22925f7a85ff792eb702bd05e34
push id18626
push usermleibovic@mozilla.com
push dateTue, 08 Feb 2011 21:15:39 +0000
treeherdermozilla-central@ef4d491f9ba0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssdwilsh, blocking-final
bugs628805
milestone2.0b12pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 628805 - Add test for unstored sessionId to test_async_history_api.js and fix the way session ids are stored with places visits [r=sdwilsh, a=blocking-final]
toolkit/components/places/src/History.cpp
toolkit/components/places/tests/unit/test_async_history_api.js
--- a/toolkit/components/places/src/History.cpp
+++ b/toolkit/components/places/src/History.cpp
@@ -728,21 +728,33 @@ private:
     (void)mReferrers.SetLength(mPlaces.Length());
 
     nsNavHistory* navHistory = nsNavHistory::GetHistoryService();
     NS_ABORT_IF_FALSE(navHistory, "Could not get nsNavHistory?!");
 
     for (nsTArray<VisitData>::size_type i = 0; i < mPlaces.Length(); i++) {
       mReferrers[i].spec = mPlaces[i].referrerSpec;
 
+      // If we are inserting a place into an empty mPlaces array, we need to
+      // check to make sure we do not store a bogus session id that is higher
+      // than the current maximum session id.
+      if (i == 0) {
+        PRInt64 newSessionId = navHistory->GetNewSessionID();
+        if (mPlaces[0].sessionId > newSessionId) {
+          mPlaces[0].sessionId = newSessionId;
+        }
+      }
+
       // Speculatively get a new session id for our visit if the current session
-      // id is non-valid.  While it is true that we will use the session id from
-      // the referrer if the visit was "recent" enough, we cannot call this
-      // method off of the main thread, so we have to consume an id now.
-      if (mPlaces[i].sessionId <= 0) {
+      // id is non-valid or if it is larger than the current largest session id.
+      // While it is true that we will use the session id from the referrer if
+      // the visit was "recent" enough, we cannot call this method off of the
+      // main thread, so we have to consume an id now.
+      if (mPlaces[i].sessionId <= 0 ||
+          (i > 0 && mPlaces[i].sessionId >= mPlaces[0].sessionId)) {
         mPlaces[i].sessionId = navHistory->GetNewSessionID();
       }
 
 #ifdef DEBUG
       nsCOMPtr<nsIURI> uri;
       (void)NS_NewURI(getter_AddRefs(uri), mPlaces[i].spec);
       NS_ASSERTION(CanAddURI(uri),
                    "Passed a VisitData with a URI we cannot add to history!");
--- a/toolkit/components/places/tests/unit/test_async_history_api.js
+++ b/toolkit/components/places/tests/unit/test_async_history_api.js
@@ -493,16 +493,65 @@ function test_invalid_sessionId_ignored(
     do_check_true(stmt.executeStep());
     do_check_neq(stmt.row.session, place.visits[0].sessionId);
     stmt.finalize();
 
     run_next_test();
   });
 }
 
+function test_unstored_sessionId_ignored()
+{
+  let place = {
+    uri: NetUtil.newURI(TEST_DOMAIN +
+                        "test_unstored_sessionId_ignored"),
+    visits: [
+      new VisitInfo(),
+    ],
+  };
+
+  // Find max session id in database.
+  let stmt = DBConn().createStatement(
+    "SELECT MAX(session) as max_session " +
+    "FROM moz_historyvisits"
+  );
+  do_check_true(stmt.executeStep());
+  let maxSessionId = stmt.row.max_session;
+  stmt.finalize();
+
+  // Create bogus sessionId that is not in database.
+  place.visits[0].sessionId = maxSessionId + 10;
+  do_check_false(gGlobalHistory.isVisited(place.uri));
+
+  gHistory.updatePlaces(place, function(aResultCode, aPlaceInfo) {
+    do_check_true(Components.isSuccessCode(aResultCode));
+    let uri = aPlaceInfo.uri;
+    do_check_true(gGlobalHistory.isVisited(uri));
+
+    // Check to make sure we do not persist bogus sessionId with the visit.
+    let visit = aPlaceInfo.visits[0];
+    do_check_neq(visit.sessionId, place.visits[0].sessionId);
+
+    // Check to make sure we do not persist bogus sessionId in the database.
+    let stmt = DBConn().createStatement(
+      "SELECT MAX(session) as max_session " +
+      "FROM moz_historyvisits"
+    );
+    do_check_true(stmt.executeStep());
+
+    // Max sessionId should increase by 1 because we will generate a new
+    // non-bogus sessionId.
+    let newMaxSessionId = stmt.row.max_session;
+    do_check_eq(maxSessionId + 1, newMaxSessionId);
+    stmt.finalize();
+
+    run_next_test();
+  });
+}
+
 function test_observer_topic_dispatched_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_observer_topic_dispatched_when_complete"),
@@ -967,16 +1016,17 @@ let gTests = [
   test_no_visits_throws,
   test_add_visit_no_date_throws,
   test_add_visit_no_transitionType_throws,
   test_add_visit_invalid_transitionType_throws,
   test_non_addable_uri_errors,
   test_invalid_referrerURI_ignored,
   test_nonnsIURI_referrerURI_ignored,
   test_invalid_sessionId_ignored,
+  test_unstored_sessionId_ignored,
   test_observer_topic_dispatched_when_complete,
   test_add_visit,
   test_properties_saved,
   test_guid_saved,
   test_referrer_saved,
   test_sessionId_saved,
   test_guid_change_saved,
   test_title_change_saved,