Bug 1362761 - Improve logging in PrefixSet. r?gcp draft
authorFrancois Marier <francois@mozilla.com>
Wed, 21 Feb 2018 17:55:12 -0800
changeset 758228 6f3b7e26b6b894acd96fef1bc52f63bb78e436ae
parent 758227 19b663f7b130b1ec15e0f65dce004b77273f1ac9
push id99989
push userfmarier@mozilla.com
push dateThu, 22 Feb 2018 01:57:57 +0000
reviewersgcp
bugs1362761
milestone60.0a1
Bug 1362761 - Improve logging in PrefixSet. r?gcp In addition to including the name of the prefix set in all of the LOG messages, the VariablePrefixSet class now initializes its dependent fixed-size prefix set correctly. MozReview-Commit-ID: C7c78HLcXY3
toolkit/components/url-classifier/VariableLengthPrefixSet.cpp
toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
toolkit/components/url-classifier/nsUrlClassifierPrefixSet.h
--- a/toolkit/components/url-classifier/VariableLengthPrefixSet.cpp
+++ b/toolkit/components/url-classifier/VariableLengthPrefixSet.cpp
@@ -43,17 +43,17 @@ VariableLengthPrefixSet::Init(const nsAC
   mMemoryReportPath =
     nsPrintfCString(
       "explicit/storage/prefix-set/%s",
       (!aName.IsEmpty() ? PromiseFlatCString(aName).get() : "?!")
     );
 
   RegisterWeakMemoryReporter(this);
 
-  return NS_OK;
+  return mFixedPrefixSet->Init(aName);
 }
 
 VariableLengthPrefixSet::~VariableLengthPrefixSet()
 {
   UnregisterWeakMemoryReporter(this);
 }
 
 nsresult
--- a/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
@@ -46,45 +46,52 @@ nsUrlClassifierPrefixSet::nsUrlClassifie
   , mTotalPrefixes(0)
   , mMemoryReportPath()
 {
 }
 
 NS_IMETHODIMP
 nsUrlClassifierPrefixSet::Init(const nsACString& aName)
 {
+  mName = aName;
   mMemoryReportPath =
     nsPrintfCString(
       "explicit/storage/prefix-set/%s",
       (!aName.IsEmpty() ? PromiseFlatCString(aName).get() : "?!")
     );
 
   RegisterWeakMemoryReporter(this);
 
   return NS_OK;
 }
 
 nsUrlClassifierPrefixSet::~nsUrlClassifierPrefixSet()
 {
   UnregisterWeakMemoryReporter(this);
 }
 
+void
+nsUrlClassifierPrefixSet::Clear()
+{
+  LOG(("[%s] Clearing PrefixSet", mName.get()));
+  mIndexDeltas.Clear();
+  mIndexPrefixes.Clear();
+  mTotalPrefixes = 0;
+}
+
 NS_IMETHODIMP
 nsUrlClassifierPrefixSet::SetPrefixes(const uint32_t* aArray, uint32_t aLength)
 {
   MutexAutoLock lock(mLock);
 
   nsresult rv = NS_OK;
 
   if (aLength <= 0) {
     if (mIndexPrefixes.Length() > 0) {
-      LOG(("Clearing PrefixSet"));
-      mIndexDeltas.Clear();
-      mIndexPrefixes.Clear();
-      mTotalPrefixes = 0;
+      Clear();
     }
   } else {
     rv = MakePrefixSet(aArray, aLength);
   }
 
   return rv;
 }
 
@@ -98,18 +105,17 @@ nsUrlClassifierPrefixSet::MakePrefixSet(
   }
 
 #ifdef DEBUG
   for (uint32_t i = 1; i < aLength; i++) {
     MOZ_ASSERT(aPrefixes[i] >= aPrefixes[i-1]);
   }
 #endif
 
-  mIndexPrefixes.Clear();
-  mIndexDeltas.Clear();
+  Clear();
   mTotalPrefixes = aLength;
 
   mIndexPrefixes.AppendElement(aPrefixes[0]);
   mIndexDeltas.AppendElement();
 
   uint32_t numOfDeltas = 0;
   uint32_t totalDeltas = 0;
   uint32_t previousItem = aPrefixes[0];
@@ -397,17 +403,17 @@ nsUrlClassifierPrefixSet::StoreToFile(ns
   nsCOMPtr<nsIOutputStream> out;
   rv = NS_NewBufferedOutputStream(getter_AddRefs(out), localOutFile.forget(),
                                   std::min(fileSize, MAX_BUFFER_SIZE));
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = WritePrefixes(out);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  LOG(("Saving PrefixSet successful\n"));
+  LOG(("[%s] Storing PrefixSet successful", mName.get()));
 
   return NS_OK;
 }
 
 nsresult
 nsUrlClassifierPrefixSet::LoadPrefixes(nsIInputStream* in)
 {
   mCanary.Check();
@@ -427,17 +433,17 @@ nsUrlClassifierPrefixSet::LoadPrefixes(n
     NS_ENSURE_SUCCESS(rv, rv);
     NS_ENSURE_TRUE(read == sizeof(uint32_t), NS_ERROR_FAILURE);
 
     rv = in->Read(reinterpret_cast<char*>(&deltaSize), sizeof(uint32_t), &read);
     NS_ENSURE_SUCCESS(rv, rv);
     NS_ENSURE_TRUE(read == sizeof(uint32_t), NS_ERROR_FAILURE);
 
     if (indexSize == 0) {
-      LOG(("stored PrefixSet is empty!"));
+      LOG(("[%s] Stored PrefixSet is empty!", mName.get()));
       return NS_OK;
     }
 
     if (deltaSize > (indexSize * DELTAS_LIMIT)) {
       return NS_ERROR_FILE_CORRUPTED;
     }
 
     nsTArray<uint32_t> indexStarts;
@@ -474,22 +480,22 @@ nsUrlClassifierPrefixSet::LoadPrefixes(n
         mTotalPrefixes += numInDelta;
         toRead = numInDelta * sizeof(uint16_t);
         rv = in->Read(reinterpret_cast<char*>(mIndexDeltas[i].Elements()), toRead, &read);
         NS_ENSURE_SUCCESS(rv, rv);
         NS_ENSURE_TRUE(read == toRead, NS_ERROR_FAILURE);
       }
     }
   } else {
-    LOG(("Version magic mismatch, not loading"));
+    LOG(("[%s] Version magic mismatch, not loading", mName.get()));
     return NS_ERROR_FILE_CORRUPTED;
   }
 
   MOZ_ASSERT(mIndexPrefixes.Length() == mIndexDeltas.Length());
-  LOG(("Loading PrefixSet successful"));
+  LOG(("[%s] Loading PrefixSet successful", mName.get()));
 
   return NS_OK;
 }
 
 uint32_t
 nsUrlClassifierPrefixSet::CalculatePreallocateSize()
 {
   uint32_t fileSize = 4 * sizeof(uint32_t);
@@ -507,17 +513,17 @@ nsUrlClassifierPrefixSet::WritePrefixes(
   // In Bug 1362761, crashes happened while reading mIndexDeltas[i].
   // We suspect that this is due to memory corruption so to test this
   // hypothesis, we will crash the browser. Once we have established
   // memory corruption as the root cause, we can attempt to gracefully
   // handle this.
   uint32_t checksum;
   CalculateTArrayChecksum(mIndexDeltas, &checksum);
   if (checksum != mIndexDeltasChecksum) {
-    LOG(("The contents of mIndexDeltas doesn't match the checksum!"));
+    LOG(("[%s] The contents of mIndexDeltas doesn't match the checksum!", mName.get()));
     MOZ_CRASH("Memory corruption detected in mIndexDeltas.");
   }
 
   uint32_t written;
   uint32_t writelen = sizeof(uint32_t);
   uint32_t magic = PREFIXSET_VERSION_MAGIC;
   nsresult rv = out->Write(reinterpret_cast<char*>(&magic), writelen, &written);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -564,17 +570,17 @@ nsUrlClassifierPrefixSet::WritePrefixes(
     for (uint32_t i = 0; i < indexDeltaSize; i++) {
       writelen = mIndexDeltas[i].Length() * sizeof(uint16_t);
       rv = out->Write(reinterpret_cast<char*>(mIndexDeltas[i].Elements()), writelen, &written);
       NS_ENSURE_SUCCESS(rv, rv);
       NS_ENSURE_TRUE(written == writelen, NS_ERROR_FAILURE);
     }
   }
 
-  LOG(("Saving PrefixSet successful\n"));
+  LOG(("[%s] Writing PrefixSet successful", mName.get()));
 
   return NS_OK;
 }
 
 template<typename T>
 void
 nsUrlClassifierPrefixSet::CalculateTArrayChecksum(nsTArray<T>& aArray,
                                                   uint32_t* outChecksum)
--- a/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.h
@@ -56,16 +56,17 @@ public:
 private:
   virtual ~nsUrlClassifierPrefixSet();
 
   static const uint32_t MAX_BUFFER_SIZE = 64 * 1024;
   static const uint32_t DELTAS_LIMIT = 120;
   static const uint32_t MAX_INDEX_DIFF = (1 << 16);
   static const uint32_t PREFIXSET_VERSION_MAGIC = 1;
 
+  void Clear();
   nsresult MakePrefixSet(const uint32_t* aArray, uint32_t aLength);
   uint32_t BinSearch(uint32_t start, uint32_t end, uint32_t target);
 
   uint32_t CalculatePreallocateSize();
   nsresult WritePrefixes(nsIOutputStream* out);
   nsresult LoadPrefixes(nsIInputStream* in);
 
   template<typename T>
@@ -84,13 +85,14 @@ private:
   // prefix from mIndexPrefix. Then every "delta" corresponds
   // to a prefix in the PrefixSet.
   nsTArray<nsTArray<uint16_t> > mIndexDeltas;
   uint32_t mIndexDeltasChecksum;
 
   // how many prefixes we have.
   uint32_t mTotalPrefixes;
 
+  nsCString mName;
   nsCString mMemoryReportPath;
   mozilla::CorruptionCanary mCanary;
 };
 
 #endif