Bug 673470 - Don't resort if not needed. Fix comparator. Cleanups. r=dcamp

toolkit/components/url-classifier/Entries.h

@@ -128,19 +128,25 @@ struct SafebrowsingHash
   }
 #endif
   PRUint32 ToUint32() const {
-    PRUint32 res = 0;
-    memcpy(&res, buf, NS_MIN<size_t>(4, S));
-    return res;
+      return *((uint32*)buf);
   }
   void FromUint32(PRUint32 aHash) {
-    memcpy(buf, &aHash, NS_MIN<size_t>(4, S));
+      *((uint32*)buf) = aHash;
   }
 };
 
 class PrefixComparator {
 public:
   static int Compare(const PRUint8* a, const PRUint8* b) {
-    return *((uint32*)a) - *((uint32*)b);
+      uint32 first = *((uint32*)a);
+      uint32 second = *((uint32*)b);
+      if (first > second) {
+          return 1;
+      } else if (first == second) {
+          return 0;
+      } else {
+          return -1;
+      }
   }
 };
 typedef SafebrowsingHash<PREFIX_SIZE, PrefixComparator> Prefix;

toolkit/components/url-classifier/HashStore.cpp

@@ -81,8 +81,8 @@
 //    0...numAddChunks               uint32 addChunk
 //    0...numSubChunks               uint32 subChunk
 //    byte sliced (numAddPrefixes)   uint32 add chunk of AddPrefixes
-//    byte sliced (numSubPrefixes)   uint32 sub chunk of SubPrefixes
 //    byte sliced (numSubPrefixes)   uint32 add chunk of SubPrefixes
+//    byte sliced (numSubPrefixes)   uint32 sub chunk of SubPrefixes
 //    byte sliced (numSubPrefixes)   uint32 SubPrefixes
 //    0...numAddCompletes            32-byte Completions + uint32 addChunk
 //    0...numSubCompletes            32-byte Completions + uint32 addChunk
@@ -996,13 +996,37 @@ RemoveMatchingPrefixes(const SubPrefixArray& aSubs, nsTArray<T>* aFullHashes)
   Erase(aFullHashes, out, hashIter);
 }
 
+#ifdef DEBUG
+template <class T>
+static void EnsureSorted(nsTArray<T>* aArray)
+{
+  T* start = aArray->Elements();
+  T* end = aArray->Elements() + aArray->Length();
+  T* iter = start;
+  T* previous = start;
+
+  while (iter != end) {
+    previous = iter;
+    ++iter;
+    if (iter != end) {
+      MOZ_ASSERT(iter->Compare(*previous) >= 0);
+    }
+  }
+
+  return;
+}
+#endif
+
 nsresult
 HashStore::ProcessSubs()
 {
-  EntrySort(mAddPrefixes);
-  EntrySort(mSubPrefixes);
-  EntrySort(mAddCompletes);
-  EntrySort(mSubCompletes);
+#ifdef DEBUG
+  EnsureSorted(&mAddPrefixes);
+  EnsureSorted(&mSubPrefixes);
+  EnsureSorted(&mAddCompletes);
+  EnsureSorted(&mSubCompletes);
+  LOG(("All databases seem to have a consistent sort order."));
+#endif
 
   RemoveMatchingPrefixes(mSubPrefixes, &mAddCompletes);
   RemoveMatchingPrefixes(mSubPrefixes, &mSubCompletes);
@@ -1019,6 +1043,14 @@ HashStore::ProcessSubs()
   KnockoutSubs(&mSubPrefixes, &mAddPrefixes);
   KnockoutSubs(&mSubCompletes, &mAddCompletes);
 
+#ifdef DEBUG
+  EnsureSorted(&mAddPrefixes);
+  EnsureSorted(&mSubPrefixes);
+  EnsureSorted(&mAddCompletes);
+  EnsureSorted(&mSubCompletes);
+  LOG(("All databases seem to have a consistent sort order."));
+#endif
+
   return NS_OK;
 }
 

toolkit/components/url-classifier/LookupCache.cpp

@@ -688,6 +688,26 @@ bool LookupCache::IsPrimed()
   return mPrimed;
 }
 
+#ifdef DEBUG
+template <class T>
+static void EnsureSorted(T* aArray)
+{
+  typename T::elem_type* start = aArray->Elements();
+  typename T::elem_type* end = aArray->Elements() + aArray->Length();
+  typename T::elem_type* iter = start;
+  typename T::elem_type* previous = start;
+
+  while (iter != end) {
+    previous = iter;
+    ++iter;
+    if (iter != end) {
+      MOZ_ASSERT(*previous <= *iter);
+    }
+  }
+  return;
+}
+#endif
+
 nsresult
 LookupCache::ConstructPrefixSet(AddPrefixArray& aAddPrefixes)
 {
@@ -709,8 +729,10 @@ LookupCache::ConstructPrefixSet(AddPrefixArray& aAddPrefixes)
     // DB is empty, but put a sentinel to show that we looked
     array.AppendElement(0);
   }
+#ifdef DEBUG
   // PrefixSet requires sorted order
-  array.Sort();
+  EnsureSorted(&array);
+#endif
 
   // construct new one, replace old entries
   rv = mPrefixSet->SetPrefixes(array.Elements(), array.Length());

toolkit/components/url-classifier/ProtocolParser.cpp

@@ -510,7 +510,6 @@ ProtocolParser::ProcessPlaintextChunk(const nsACString& aChunk)
         NS_ASSERTION(mChunkState.hashSize == 4, "Only 32- or 4-byte hashes can be used for add chunks.");
         Prefix hash;
         Completion domHash;
-        Prefix newHash;
         rv = LookupCache::GetKey(Substring(iter, end), &domHash, mCryptoHash);
         NS_ENSURE_SUCCESS(rv, rv);
         hash.FromPlaintext(Substring(iter, end), mCryptoHash);
@@ -518,6 +517,7 @@ ProtocolParser::ProcessPlaintextChunk(const nsACString& aChunk)
         rv = LookupCache::KeyedHash(hash.ToUint32(), domHash.ToUint32(), mHashKey,
                                     &codedHash, !mPerClientRandomize);
         NS_ENSURE_SUCCESS(rv, rv);
+        Prefix newHash;
         newHash.FromUint32(codedHash);
         mTableUpdate->NewSubPrefix(addChunk, newHash, mChunkState.num);
         // Needed to knock out completes