Bug 207022 r=annie.sullivan Redirects sometimes appear in the wrong order (places is disabled by default)

browser/components/places/src/nsNavHistory.cpp

@@ -1919,9 +1919,12 @@ NS_IMETHODIMP
 nsNavHistory::AddURI(nsIURI *aURI, PRBool aRedirect,
                      PRBool aToplevel, nsIURI *aReferrer)
 {
-  // add to main DB
-  PRInt64 pageid = 0;
-  PRTime now = GetNow();
+  // Note that here we should NOT use the GetNow function. That function caches
+  // the value of "now" until next time the event loop runs. This gives better
+  // performance, but here we may get many notifications without running the
+  // event loop. We must preserve these events' ordering. This most commonly
+  // happens on redirects.
+  PRTime now = PR_Now();
 
   // check for transition types
   PRUint32 transitionType = 0;
@@ -1956,6 +1959,7 @@ nsNavHistory::AddURI(nsIURI *aURI, PRBool aRedirect,
     }
   }
 
+  PRInt64 pageid = 0;
   nsresult rv = InternalAdd(aURI, aReferrer, 0, transitionType, nsnull, now,
                             aRedirect, aToplevel, &pageid);
   NS_ENSURE_SUCCESS(rv, rv);

browser/components/places/src/nsNavHistoryResult.cpp

@@ -1498,19 +1498,26 @@ PRInt32 PR_CALLBACK nsNavHistoryResult::SortComparison_TitleGreater(
 
 // nsNavHistoryResult::SortComparison_Date*
 //
-//    Don't bother doing conflict resolution. Since dates are stored in
-//    microseconds, it will be very difficult to get collisions. This would be
-//    most likely for imported history, which I'm not too worried about.
+//    Equal times will be very unusual, but it is important that there is some
+//    deterministic ordering of the results so they don't move around. Use URLs
+//    for conflict resolution. If those are the same, we probably don't care
+//    about the relative ordering.
 
 PRInt32 PR_CALLBACK nsNavHistoryResult::SortComparison_DateLess(
     nsNavHistoryResultNode* a, nsNavHistoryResultNode* b, void* closure)
 {
-  return ComparePRTime(a->mTime, b->mTime);
+  PRInt32 value = ComparePRTime(a->mTime, b->mTime);
+  if (value == 0)
+    return a->mUrl.Compare(b->mUrl.get());
+  return value;
 }
 PRInt32 PR_CALLBACK nsNavHistoryResult::SortComparison_DateGreater(
     nsNavHistoryResultNode* a, nsNavHistoryResultNode* b, void* closure)
 {
-  return -ComparePRTime(a->mTime, b->mTime);
+  PRInt32 value = -ComparePRTime(a->mTime, b->mTime);
+  if (value == 0)
+    return -a->mUrl.Compare(b->mUrl.get());
+  return value;
 }