Bug 1415598. r=gijs, a=jcristau
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 07 Dec 2017 15:28:41 +0100
changeset 445283 ff2ac648087a6f224cb5ed79663c016237a56cc2
parent 445282 366eae257e7e7d243149cc19096c18c3f15e8a32
child 445284 cc9318833c015f22f8bf57ff75e87bac07818761
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgijs, jcristau
bugs1415598
milestone58.0
Bug 1415598. r=gijs, a=jcristau
toolkit/components/places/History.cpp
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -1961,16 +1961,17 @@ GetLinkDocument(Link* aLink)
   // method.
   Element* element = aLink->GetElement();
   return element ? element->OwnerDoc() : nullptr;
 }
 
 NS_IMETHODIMP
 History::NotifyVisited(nsIURI* aURI)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   NS_ENSURE_ARG(aURI);
   // NOTE: This can be run within the SystemGroup, and thus cannot directly
   // interact with webpages.
 
   nsAutoScriptBlocker scriptBlocker;
 
   if (XRE_IsParentProcess()) {
     nsTArray<ContentParent*> cplist;
@@ -2012,16 +2013,17 @@ History::NotifyVisited(nsIURI* aURI)
   }
 
   return NS_OK;
 }
 
 void
 History::NotifyVisitedForDocument(nsIURI* aURI, nsIDocument* aDocument)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   // Make sure that nothing invalidates our observer array while we're walking
   // over it.
   nsAutoScriptBlocker scriptBlocker;
 
   // If we have no observers for this URI, we have nothing to notify about.
   KeyClass* key = mObservers.GetEntry(aURI);
   if (!key) {
     return;
@@ -2635,16 +2637,17 @@ History::VisitURI(nsIURI* aURI,
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 History::RegisterVisitedCallback(nsIURI* aURI,
                                  Link* aLink)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   NS_ASSERTION(aURI, "Must pass a non-null URI!");
   if (XRE_IsContentProcess()) {
     NS_PRECONDITION(aLink, "Must pass a non-null Link!");
   }
 
   // Obtain our array of observers for this URI.
 #ifdef DEBUG
   bool keyAlreadyExists = !!mObservers.GetEntry(aURI);
@@ -2664,17 +2667,26 @@ History::RegisterVisitedCallback(nsIURI*
 
     // In IPC builds, we are passed a nullptr Link from
     // ContentParent::RecvStartVisitedQuery.  Since we won't be adding a
     // nullptr entry to our list of observers, and the code after this point
     // assumes that aLink is non-nullptr, we will need to return now.
     if (NS_FAILED(rv) || !aLink) {
       // Remove our array from the hashtable so we don't keep it around.
       MOZ_ASSERT(key == mObservers.GetEntry(aURI), "The URIs hash mutated!");
-      mObservers.RemoveEntry(key);
+      // In some case calling RemoveEntry on the key obtained by PutEntry
+      // crashes for currently unknown reasons.  Our suspect is that something
+      // between PutEntry and this call causes a nested loop that either removes
+      // the entry or reallocs the hash.
+      // TODO (Bug 1412647): we must figure the root cause for these issues and
+      // remove this stop-gap crash fix.
+      key = mObservers.GetEntry(aURI);
+      if (key) {
+        mObservers.RemoveEntry(key);
+      }
       return rv;
     }
   }
   // In IPC builds, we are passed a nullptr Link from
   // ContentParent::RecvStartVisitedQuery.  All of our code after this point
   // assumes aLink is non-nullptr, so we have to return now.
   else if (!aLink) {
     NS_ASSERTION(XRE_IsParentProcess(),
@@ -2703,16 +2715,17 @@ History::RegisterVisitedCallback(nsIURI*
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 History::UnregisterVisitedCallback(nsIURI* aURI,
                                    Link* aLink)
 {
+  MOZ_ASSERT(NS_IsMainThread());
   // TODO: aURI is sometimes null - see bug 548685
   NS_ASSERTION(aURI, "Must pass a non-null URI!");
   NS_ASSERTION(aLink, "Must pass a non-null Link object!");
 
   // Get the array, and remove the item from it.
   KeyClass* key = mObservers.GetEntry(aURI);
   if (!key) {
     NS_ERROR("Trying to unregister for a URI that wasn't registered!");