Bug 1550531. Fix -moz-element memory leak and related shutdown hangs. r=emilio
authorJonathan Watt <jwatt@jwatt.org>
Wed, 22 May 2019 20:14:47 +0100
changeset 475394 f75308d601f2a3d434cb91b47b238e1e2194706f
parent 475164 32efcd22ed9c27d664699b8d8f22f8da92857b07
child 475395 1af5cf2905662442515ff17166e79cb3e10e252f
push id36061
push usercbrindusan@mozilla.com
push dateFri, 24 May 2019 21:49:59 +0000
treeherdermozilla-central@5d3e1ea77693 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1550531
milestone69.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 1550531. Fix -moz-element memory leak and related shutdown hangs. r=emilio
layout/svg/SVGObserverUtils.cpp
layout/svg/SVGObserverUtils.h
--- a/layout/svg/SVGObserverUtils.cpp
+++ b/layout/svg/SVGObserverUtils.cpp
@@ -10,32 +10,93 @@
 // Keep others in (case-insensitive) order:
 #include "mozilla/css/ImageLoader.h"
 #include "mozilla/dom/CanvasRenderingContext2D.h"
 #include "mozilla/net/ReferrerPolicy.h"
 #include "mozilla/PresShell.h"
 #include "mozilla/RestyleManager.h"
 #include "nsCSSFrameConstructor.h"
 #include "nsCycleCollectionParticipant.h"
+#include "nsHashKeys.h"
 #include "nsIContent.h"
 #include "nsIContentInlines.h"
+#include "nsInterfaceHashtable.h"
 #include "nsIReflowCallback.h"
 #include "nsISupportsImpl.h"
 #include "nsSVGClipPathFrame.h"
 #include "nsSVGFilterFrame.h"
 #include "nsSVGMarkerFrame.h"
 #include "nsSVGMaskFrame.h"
 #include "nsSVGPaintServerFrame.h"
+#include "nsTHashtable.h"
+#include "nsURIHashKey.h"
 #include "SVGGeometryElement.h"
 #include "SVGTextPathElement.h"
 #include "SVGUseElement.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
+bool URLAndReferrerInfo::operator==(const URLAndReferrerInfo& aRHS) const {
+  bool uriEqual = false, referrerEqual = false;
+  this->mURI->Equals(aRHS.mURI, &uriEqual);
+  this->mReferrer->Equals(aRHS.mReferrer, &referrerEqual);
+
+  return uriEqual && referrerEqual &&
+         this->mReferrerPolicy == aRHS.mReferrerPolicy;
+}
+
+class URLAndReferrerInfoHashKey : public PLDHashEntryHdr {
+ public:
+  using KeyType = const URLAndReferrerInfo*;
+  using KeyTypePointer = const URLAndReferrerInfo*;
+
+  explicit URLAndReferrerInfoHashKey(const URLAndReferrerInfo* aKey)
+      : mKey(aKey) {
+    MOZ_COUNT_CTOR(URLAndReferrerInfoHashKey);
+  }
+  URLAndReferrerInfoHashKey(URLAndReferrerInfoHashKey&& aToMove)
+      : PLDHashEntryHdr(std::move(aToMove)), mKey(std::move(aToMove.mKey)) {
+    MOZ_COUNT_CTOR(URLAndReferrerInfoHashKey);
+  }
+  ~URLAndReferrerInfoHashKey() { MOZ_COUNT_DTOR(URLAndReferrerInfoHashKey); }
+
+  const URLAndReferrerInfo* GetKey() const { return mKey; }
+
+  bool KeyEquals(const URLAndReferrerInfo* aKey) const {
+    if (!mKey) {
+      return !aKey;
+    }
+    return *mKey == *aKey;
+  }
+
+  static const URLAndReferrerInfo* KeyToPointer(
+      const URLAndReferrerInfo* aKey) {
+    return aKey;
+  }
+
+  static PLDHashNumber HashKey(const URLAndReferrerInfo* aKey) {
+    if (!aKey) {
+      // If the key is null, return hash for empty string.
+      return HashString(EmptyCString());
+    }
+    nsAutoCString urlSpec, referrerSpec;
+    // nsURIHashKey ignores GetSpec() failures, so we do too:
+    Unused << aKey->GetURI()->GetSpec(urlSpec);
+    Unused << aKey->GetReferrer()->GetSpec(referrerSpec);
+    auto refPolicy = aKey->GetReferrerPolicy();
+    return AddToHash(HashString(urlSpec), HashString(referrerSpec), refPolicy);
+  }
+
+  enum { ALLOW_MEMMOVE = true };
+
+ protected:
+  RefPtr<const URLAndReferrerInfo> mKey;
+};
+
 static already_AddRefed<URLAndReferrerInfo> ResolveURLUsingLocalRef(
     nsIFrame* aFrame, const css::URLValue* aURL) {
   MOZ_ASSERT(aFrame);
 
   if (!aURL) {
     return nullptr;
   }
 
@@ -983,18 +1044,17 @@ void SVGRenderingObserver::DebugObserver
                "failed to track whether we're in our referenced element's "
                "observer set!");
   } else {
     MOZ_ASSERT(!mInObserverList, "In whose observer set are we, then?");
   }
 }
 #endif
 
-typedef nsInterfaceHashtable<nsRefPtrHashKey<URLAndReferrerInfo>,
-                             nsIMutationObserver>
+typedef nsInterfaceHashtable<URLAndReferrerInfoHashKey, nsIMutationObserver>
     URIObserverHashtable;
 
 using PaintingPropertyDescriptor =
     const mozilla::FramePropertyDescriptor<nsSVGPaintingProperty>*;
 
 static void DestroyFilterProperty(SVGFilterObserverListForCSSProp* aProp) {
   // SVGFilterObserverListForCSSProp is cycle-collected, so dropping the last
   // reference doesn't necessarily destroy it. We need to tell it that the
@@ -1371,18 +1431,16 @@ Element* SVGObserverUtils::GetAndObserve
   nsCOMPtr<nsIURI> base = aFrame->GetContent()->GetBaseURI();
   nsContentUtils::NewURIWithDocumentCharset(
       getter_AddRefs(targetURI), elementId,
       aFrame->GetContent()->GetUncomposedDoc(), base);
   RefPtr<URLAndReferrerInfo> url = new URLAndReferrerInfo(
       targetURI, aFrame->GetContent()->OwnerDoc()->GetDocumentURI(),
       aFrame->GetContent()->OwnerDoc()->GetReferrerPolicy());
 
-  // XXXjwatt: this is broken - we're using the address of a new
-  // URLAndReferrerInfo as the hash key every time!
   SVGMozElementObserver* observer =
       static_cast<SVGMozElementObserver*>(hashtable->GetWeak(url));
   if (!observer) {
     observer = new SVGMozElementObserver(url, aFrame, /* aWatchImage */ true);
     hashtable->Put(url, observer);
   }
   return observer->GetAndObserveReferencedElement();
 }
--- a/layout/svg/SVGObserverUtils.h
+++ b/layout/svg/SVGObserverUtils.h
@@ -6,28 +6,24 @@
 
 #ifndef NSSVGEFFECTS_H_
 #define NSSVGEFFECTS_H_
 
 #include "mozilla/Attributes.h"
 #include "mozilla/dom/IDTracker.h"
 #include "FrameProperties.h"
 #include "mozilla/dom/Element.h"
-#include "nsHashKeys.h"
 #include "nsID.h"
 #include "nsIFrame.h"
 #include "nsIMutationObserver.h"
-#include "nsInterfaceHashtable.h"
 #include "nsISupportsBase.h"
 #include "nsISupportsImpl.h"
 #include "nsStringFwd.h"
 #include "nsStubMutationObserver.h"
 #include "nsSVGUtils.h"
-#include "nsTHashtable.h"
-#include "nsURIHashKey.h"
 #include "nsCycleCollectionParticipant.h"
 
 class nsAtom;
 class nsIURI;
 class nsSVGClipPathFrame;
 class nsSVGMarkerFrame;
 class nsSVGPaintServerFrame;
 class nsSVGFilterFrame;
@@ -59,19 +55,23 @@ class URLAndReferrerInfo {
       : mURI(aURI),
         mReferrer(aExtraData->GetReferrer()),
         mReferrerPolicy(aExtraData->GetReferrerPolicy()) {
     MOZ_ASSERT(aURI);
   }
 
   NS_INLINE_DECL_REFCOUNTING(URLAndReferrerInfo)
 
-  nsIURI* GetURI() { return mURI; }
-  nsIURI* GetReferrer() { return mReferrer; }
-  mozilla::net::ReferrerPolicy GetReferrerPolicy() { return mReferrerPolicy; }
+  nsIURI* GetURI() const { return mURI; }
+  nsIURI* GetReferrer() const { return mReferrer; }
+  mozilla::net::ReferrerPolicy GetReferrerPolicy() const {
+    return mReferrerPolicy;
+  }
+
+  bool operator==(const URLAndReferrerInfo& aRHS) const;
 
  private:
   ~URLAndReferrerInfo() = default;
 
   nsCOMPtr<nsIURI> mURI;
   nsCOMPtr<nsIURI> mReferrer;
   mozilla::net::ReferrerPolicy mReferrerPolicy;
 };