Bug 1049812 - Part 1 : Removes static_cast introduced by bug 922727 and set a bool property in DEBUG to ensure PlaceHashKey properties are set. r=mak
authorArnaud Sourioux <six.dsn@gmail.com>
Fri, 05 Sep 2014 00:19:00 +0200
changeset 227270 27ea646472f9075e19137cb3e141be68fe9f9fbf
parent 227269 6a6f51ad334885757e529e7b10b69b6936b115fd
child 227271 d38ec8e9cfd605464b74a8e2e46311b91fd91022
push idunknown
push userunknown
push dateunknown
reviewersmak
bugs1049812, 922727
milestone35.0a1
Bug 1049812 - Part 1 : Removes static_cast introduced by bug 922727 and set a bool property in DEBUG to ensure PlaceHashKey properties are set. r=mak
toolkit/components/places/History.cpp
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -183,40 +183,76 @@ struct RemoveVisitsFilter {
 ////////////////////////////////////////////////////////////////////////////////
 //// PlaceHashKey
 
 class PlaceHashKey : public nsCStringHashKey
 {
   public:
     explicit PlaceHashKey(const nsACString& aSpec)
     : nsCStringHashKey(&aSpec)
-    , visitCount(-1)
-    , bookmarked(-1)
+    , visitCount(0)
+    , bookmarked(false)
+#ifdef DEBUG
+    , isInitialized(false)
+#endif
     {
     }
 
     explicit PlaceHashKey(const nsACString* aSpec)
     : nsCStringHashKey(aSpec)
-    , visitCount(-1)
-    , bookmarked(-1)
+    , visitCount(0)
+    , bookmarked(false)
+#ifdef DEBUG
+    , isInitialized(false)
+#endif
     {
     }
 
     PlaceHashKey(const PlaceHashKey& aOther)
     : nsCStringHashKey(&aOther.GetKey())
     {
       MOZ_ASSERT(false, "Do not call me!");
     }
 
-    // Visit count for this place.
-    int32_t visitCount;
-    // Whether this place is bookmarked.
-    int32_t bookmarked;
+  void SetProperties(uint32_t aVisitCount, bool aBookmarked)
+  {
+    visitCount = aVisitCount;
+    bookmarked = aBookmarked;
+#ifdef DEBUG
+    isInitialized = true;
+#endif
+  }
+
+  uint32_t VisitCount() const
+  {
+#ifdef DEBUG
+    MOZ_ASSERT(isInitialized, "PlaceHashKey::visitCount not set");
+#endif
+    return visitCount;
+  }
+
+  bool IsBookmarked() const
+  {
+#ifdef DEBUG
+    MOZ_ASSERT(isInitialized, "PlaceHashKey::bookmarked not set");
+#endif
+    return bookmarked;
+  }
+
     // Array of VisitData objects.
     nsTArray<VisitData> visits;
+private:
+    // Visit count for this place.
+    uint32_t visitCount;
+    // Whether this place is bookmarked.
+    bool bookmarked;
+#ifdef DEBUG
+  // Whether previous attributes are set.
+  bool isInitialized;
+#endif
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Anonymous Helpers
 
 namespace {
 
 /**
@@ -1563,36 +1599,34 @@ NS_IMPL_ISUPPORTS(
  * Enumerator used by NotifyRemoveVisits to transfer the hash entries.
  */
 static PLDHashOperator TransferHashEntries(PlaceHashKey* aEntry,
                                            void* aHash)
 {
   nsTHashtable<PlaceHashKey>* hash =
     static_cast<nsTHashtable<PlaceHashKey> *>(aHash);
   PlaceHashKey* copy = hash->PutEntry(aEntry->GetKey());
-  copy->visitCount = aEntry->visitCount;
-  copy->bookmarked = aEntry->bookmarked;
+  copy->SetProperties(aEntry->VisitCount(), aEntry->IsBookmarked());
   aEntry->visits.SwapElements(copy->visits);
   return PL_DHASH_NEXT;
 }
 
 /**
  * Enumerator used by NotifyRemoveVisits to notify removals.
  */
 static PLDHashOperator NotifyVisitRemoval(PlaceHashKey* aEntry,
                                           void* aHistory)
 {
   nsNavHistory* history = static_cast<nsNavHistory *>(aHistory);
   const nsTArray<VisitData>& visits = aEntry->visits;
   nsCOMPtr<nsIURI> uri;
   (void)NS_NewURI(getter_AddRefs(uri), visits[0].spec);
-  // XXX visitCount should really just be unsigned (bug 1049812)
   bool removingPage =
-    visits.Length() == static_cast<size_t>(aEntry->visitCount) &&
-    !aEntry->bookmarked;
+    visits.Length() == aEntry->VisitCount() &&
+    !aEntry->IsBookmarked();
   // FindRemovableVisits only sets the transition type on the VisitData objects
   // it collects if the visits were filtered by transition type.
   // RemoveVisitsFilter currently only supports filtering by transition type, so
   // FindRemovableVisits will either find all visits, or all visits of a given
   // type. Therefore, if transitionType is set on this visit, we pass the
   // transition type to NotifyOnPageExpired which in turns passes it to
   // OnDeleteVisits to indicate that all visits of a given type were removed.
   uint32_t transition = visits[0].transitionType < UINT32_MAX ?
@@ -1659,19 +1693,18 @@ private:
 /**
  * Enumerator used by RemoveVisits to populate list of removed place ids.
  */
 static PLDHashOperator ListToBeRemovedPlaceIds(PlaceHashKey* aEntry,
                                                void* aIdsList)
 {
   const nsTArray<VisitData>& visits = aEntry->visits;
   // Only orphan ids should be listed.
-  // XXX visitCount should really just be unsigned (bug 1049812)
-  if (visits.Length() == static_cast<size_t>(aEntry->visitCount) &&
-      !aEntry->bookmarked) {
+  if (visits.Length() == aEntry->VisitCount() &&
+      !aEntry->IsBookmarked()) {
     nsCString* list = static_cast<nsCString*>(aIdsList);
     if (!list->IsEmpty())
       list->Append(',');
     list->AppendInt(visits[0].placeId);
   }
   return PL_DHASH_NEXT;
 }
 
@@ -1810,18 +1843,17 @@ private:
       NS_ENSURE_SUCCESS(rv, rv);
       rv = stmt->GetInt32(6, &bookmarked);
       NS_ENSURE_SUCCESS(rv, rv);
 
       PlaceHashKey* entry = aPlaces.GetEntry(visit.spec);
       if (!entry) {
         entry = aPlaces.PutEntry(visit.spec);
       }
-      entry->visitCount = visitCount;
-      entry->bookmarked = bookmarked;
+      entry->SetProperties(static_cast<uint32_t>(visitCount), static_cast<bool>(bookmarked));
       entry->visits.AppendElement(visit);
     }
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
 
   nsresult