Bug 1363036 - Add MOZ_DIAGNOSTIC_ASSERTs for bug hunting. r=smaug
authorSamael Wang <freesamael@gmail.com>
Tue, 16 May 2017 10:54:01 +0800
changeset 358651 1a5e3003f692b2490a3352a1f76ac0be69c76115
parent 358650 2a0949cc856f91e4d3b596d5f35f4c50d960537d
child 358652 b3f8917bd944c021017735270cd6ac1422cddcad
push id90370
push usercbook@mozilla.com
push dateWed, 17 May 2017 08:08:45 +0000
treeherdermozilla-inbound@228a1e9bcc4b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1363036
milestone55.0a1
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 1363036 - Add MOZ_DIAGNOSTIC_ASSERTs for bug hunting. r=smaug MozReview-Commit-ID: JPnkkJlkwDY
docshell/shistory/nsSHEntry.cpp
docshell/shistory/nsSHEntryShared.cpp
docshell/shistory/nsSHistory.cpp
xpcom/ds/nsExpirationTracker.h
--- a/docshell/shistory/nsSHEntry.cpp
+++ b/docshell/shistory/nsSHEntry.cpp
@@ -961,11 +961,14 @@ nsSHEntry::SetLastTouched(uint32_t aLast
 {
   mShared->mLastTouched = aLastTouched;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSHEntry::SetSHistory(nsISHistory* aSHistory)
 {
-  mShared->mSHistory = do_GetWeakReference(aSHistory);
+  nsWeakPtr shistory = do_GetWeakReference(aSHistory);
+  // mSHistory can not be changed once it's set
+  MOZ_DIAGNOSTIC_ASSERT(!mShared->mSHistory || (mShared->mSHistory == shistory));
+  mShared->mSHistory = shistory;
   return NS_OK;
 }
--- a/docshell/shistory/nsSHEntryShared.cpp
+++ b/docshell/shistory/nsSHEntryShared.cpp
@@ -129,16 +129,20 @@ nsSHEntryShared::SetContentViewer(nsICon
 {
   NS_PRECONDITION(!aViewer || !mContentViewer,
                   "SHEntryShared already contains viewer");
 
   if (mContentViewer || !aViewer) {
     DropPresentationState();
   }
 
+  // If we're setting mContentViewer to null, state should already be cleared
+  // in the DropPresentationState() call above; If we're setting it to a
+  // non-null content viewer, the entry shouldn't have been tracked either.
+  MOZ_DIAGNOSTIC_ASSERT(!GetExpirationState()->IsTracked());
   mContentViewer = aViewer;
 
   if (mContentViewer) {
     // mSHistory is only set for root entries, but in general bfcache only
     // applies to root entries as well. BFCache for subframe navigation has been
     // disabled since 2005 in bug 304860.
     nsCOMPtr<nsISHistoryInternal> shistory = do_QueryReferent(mSHistory);
     if (shistory) {
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -1315,16 +1315,17 @@ nsSHistory::AddToExpirationTracker(nsIBF
   mHistoryTracker->AddObject(entry);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSHistory::RemoveFromExpirationTracker(nsIBFCacheEntry* aEntry)
 {
   RefPtr<nsSHEntryShared> entry = static_cast<nsSHEntryShared*>(aEntry);
+  MOZ_DIAGNOSTIC_ASSERT(mHistoryTracker && !mHistoryTracker->IsEmpty());
   if (!mHistoryTracker || !entry) {
     return NS_ERROR_FAILURE;
   }
 
   mHistoryTracker->RemoveObject(entry);
   return NS_OK;
 }
 
@@ -1922,26 +1923,29 @@ nsSHistory::InitiateLoad(nsISHEntry* aFr
   return aFrameDS->LoadURI(nextURI, loadInfo,
                            nsIWebNavigation::LOAD_FLAGS_NONE, false);
 
 }
 
 NS_IMETHODIMP
 nsSHistory::SetRootDocShell(nsIDocShell* aDocShell)
 {
+  MOZ_DIAGNOSTIC_ASSERT(!aDocShell || !mRootDocShell);
   mRootDocShell = aDocShell;
 
   // Init mHistoryTracker on setting mRootDocShell so we can bind its event
   // target to the tabGroup.
   if (mRootDocShell) {
     nsCOMPtr<nsPIDOMWindowOuter> win = mRootDocShell->GetWindow();
     if (!win) {
       return NS_ERROR_UNEXPECTED;
     }
 
+    // mHistroyTracker can only be set once.
+    MOZ_DIAGNOSTIC_ASSERT(!mHistoryTracker);
     RefPtr<mozilla::dom::TabGroup> tabGroup = win->TabGroup();
     mHistoryTracker = mozilla::MakeUnique<HistoryTracker>(
       this,
       mozilla::Preferences::GetUint(CONTENT_VIEWER_TIMEOUT_SECONDS,
                                     CONTENT_VIEWER_TIMEOUT_SECONDS_DEFAULT),
       tabGroup->EventTargetFor(mozilla::TaskCategory::Other));
   }
 
--- a/xpcom/ds/nsExpirationTracker.h
+++ b/xpcom/ds/nsExpirationTracker.h
@@ -142,18 +142,18 @@ public:
   /**
    * Add an object to be tracked. It must not already be tracked. It will
    * be added to the newest generation, i.e., as if it was just used.
    * @return an error on out-of-memory
    */
   nsresult AddObjectLocked(T* aObj, const AutoLock& aAutoLock)
   {
     nsExpirationState* state = aObj->GetExpirationState();
-    NS_ASSERTION(!state->IsTracked(),
-                 "Tried to add an object that's already tracked");
+    MOZ_DIAGNOSTIC_ASSERT(!state->IsTracked(),
+                          "Tried to add an object that's already tracked");
     nsTArray<T*>& generation = mGenerations[mNewestGeneration];
     uint32_t index = generation.Length();
     if (index > nsExpirationState::MAX_INDEX_IN_GENERATION) {
       NS_WARNING("More than 256M elements tracked, this is probably a problem");
       return NS_ERROR_OUT_OF_MEMORY;
     }
     if (index == 0) {
       // We might need to start the timer
@@ -171,27 +171,28 @@ public:
   }
 
   /**
    * Remove an object from the tracker. It must currently be tracked.
    */
   void RemoveObjectLocked(T* aObj, const AutoLock& aAutoLock)
   {
     nsExpirationState* state = aObj->GetExpirationState();
-    NS_ASSERTION(state->IsTracked(), "Tried to remove an object that's not tracked");
+    MOZ_DIAGNOSTIC_ASSERT(state->IsTracked(), "Tried to remove an object that's not tracked");
     nsTArray<T*>& generation = mGenerations[state->mGeneration];
     uint32_t index = state->mIndexInGeneration;
-    NS_ASSERTION(generation.Length() > index &&
-                 generation[index] == aObj, "Object is lying about its index");
+    MOZ_DIAGNOSTIC_ASSERT(generation.Length() > index &&
+                          generation[index] == aObj, "Object is lying about its index");
     // Move the last object to fill the hole created by removing aObj
     uint32_t last = generation.Length() - 1;
     T* lastObj = generation[last];
     generation[index] = lastObj;
     lastObj->GetExpirationState()->mIndexInGeneration = index;
     generation.RemoveElementAt(last);
+    MOZ_DIAGNOSTIC_ASSERT(generation.Length() == last);
     state->mGeneration = nsExpirationState::NOT_TRACKED;
     // We do not check whether we need to stop the timer here. The timer
     // will check that itself next time it fires. Checking here would not
     // be efficient since we'd need to track all generations. Also we could
     // thrash by incessantly creating and destroying timers if someone
     // kept adding and removing an object from the tracker.
   }