Bug 1495562. Rename SVGRenderingObserverList to SVGRenderingObserverSet. r=longsonr
☠☠ backed out by 1d68c706356c ☠ ☠
authorJonathan Watt <jwatt@jwatt.org>
Mon, 24 Sep 2018 11:45:17 +0100
changeset 494825 ec5d3c12ad4c9b8d3c2eea2b0f71f5dc3c15f15f
parent 494824 28792d9adea23297a9c5e330fcbfab0203e2350c
child 494826 a14e1d156c61ce3534ef13f6741fc9f84cf125a6
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr
bugs1495562
milestone64.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 1495562. Rename SVGRenderingObserverList to SVGRenderingObserverSet. r=longsonr Differential Revision: https://phabricator.services.mozilla.com/D7330
layout/svg/SVGObserverUtils.cpp
layout/svg/SVGObserverUtils.h
xpcom/ds/StaticAtoms.py
--- a/layout/svg/SVGObserverUtils.cpp
+++ b/layout/svg/SVGObserverUtils.cpp
@@ -161,17 +161,17 @@ SVGRenderingObserver::GetAndObserveRefer
 void
 SVGRenderingObserver::OnNonDOMMutationRenderingChange()
 {
   mInObserverList = false;
   OnRenderingChange();
 }
 
 void
-SVGRenderingObserver::NotifyEvictedFromRenderingObserverList()
+SVGRenderingObserver::NotifyEvictedFromRenderingObserverSet()
 {
   mInObserverList = false; // We've been removed from rendering-obs. list.
   StopObserving();            // Remove ourselves from mutation-obs. list.
 }
 
 void
 SVGRenderingObserver::AttributeChanged(dom::Element* aElement,
                                        int32_t aNameSpaceID,
@@ -275,17 +275,17 @@ protected:
  *
  * In SVGIDRenderingObserver's ctor, the new object adds itself to the
  * mutation observer list maintained by the referenced element. In this way the
  * SVGIDRenderingObserver is notified if there are any attribute or content
  * tree changes to the element or any of its *descendants*.
  *
  * In SVGIDRenderingObserver::GetAndObserveReferencedElement() the
  * SVGIDRenderingObserver object also adds itself to an
- * SVGRenderingObserverList object belonging to the referenced
+ * SVGRenderingObserverSet object belonging to the referenced
  * element.
  *
  * XXX: it would be nice to have a clear and concise executive summary of the
  * benefits/necessity of maintaining a second observer list.
  */
 SVGIDRenderingObserver::SVGIDRenderingObserver(URLAndReferrerInfo* aURI,
                                                nsIContent* aObservingContent,
                                                bool aReferenceImage)
@@ -887,48 +887,53 @@ SVGTemplateElementObserver::OnRenderingC
     // about coalescing multiple invalidations by using a change hint as we do
     // in nsSVGRenderingObserverProperty::OnRenderingChange.
     SVGObserverUtils::InvalidateDirectRenderingObservers(frame);
   }
 }
 
 
 /**
- * XXXjwatt: unlike this other *List classes above, this is stored on the
- * referenced element, not the referencing element.  Unlike the other list
- * classes it is also not an ordered list - it's an unordered set. We should
- * rename this accordingly to SVGRenderingObserverSet to make things clearer.
+ * An instance of this class is stored on an observed frame (as a frame
+ * property) whenever the frame has active rendering observers.  It is used to
+ * store pointers to the SVGRenderingObserver instances belonging to any
+ * observing frames, allowing invalidations from the observed frame to be sent
+ * to all observing frames.
+ *
+ * SVGRenderingObserver instances that are added are not strongly referenced,
+ * so they must remove themselves before they die.
  *
- * A manager for one-shot SVGRenderingObserver tracking.
- * nsSVGRenderingObservers can be added or removed. They are not strongly
- * referenced so an observer must be removed before it dies.
- * When InvalidateAll is called, all outstanding references get
- * OnNonDOMMutationRenderingChange()
- * called on them and the list is cleared. The intent is that
- * the observer will force repainting of whatever part of the document
- * is needed, and then at paint time the observer will do a clean lookup
- * of the referenced element and [re-]add itself to the element's observer list.
+ * This class is "single-shot", which is to say that when something about the
+ * observed element changes, InvalidateAll() clears our hashtable of
+ * SVGRenderingObservers.  SVGRenderingObserver objects will be added back
+ * again if/when the observing frame looks up our observed frame to use it.
+ *
+ * XXXjwatt: is this the best thing to do nowadays?  Back when that mechanism
+ * landed in bug 330498 we had two pass, recursive invalidation up the frame
+ * tree, and I think reference loops were a problem.  Nowadays maybe a flag
+ * on the SVGRenderingObserver objects to coalesce invalidations may work
+ * better?
  *
  * InvalidateAll must be called before this object is destroyed, i.e.
  * before the referenced frame is destroyed. This should normally happen
  * via nsSVGContainerFrame::RemoveFrame, since only frames in the frame
  * tree should be referenced.
  */
-class SVGRenderingObserverList
+class SVGRenderingObserverSet
 {
 public:
-  SVGRenderingObserverList()
+  SVGRenderingObserverSet()
     : mObservers(4)
   {
-    MOZ_COUNT_CTOR(SVGRenderingObserverList);
+    MOZ_COUNT_CTOR(SVGRenderingObserverSet);
   }
 
-  ~SVGRenderingObserverList() {
+  ~SVGRenderingObserverSet() {
     InvalidateAll();
-    MOZ_COUNT_DTOR(SVGRenderingObserverList);
+    MOZ_COUNT_DTOR(SVGRenderingObserverSet);
   }
 
   void Add(SVGRenderingObserver* aObserver) {
     mObservers.PutEntry(aObserver);
   }
   void Remove(SVGRenderingObserver* aObserver) {
     mObservers.RemoveEntry(aObserver);
   }
@@ -959,17 +964,17 @@ public:
    */
   void RemoveAll();
 
 private:
   nsTHashtable<nsPtrHashKey<SVGRenderingObserver>> mObservers;
 };
 
 void
-SVGRenderingObserverList::InvalidateAll()
+SVGRenderingObserverSet::InvalidateAll()
 {
   if (mObservers.IsEmpty()) {
     return;
   }
 
   AutoTArray<SVGRenderingObserver*,10> observers;
 
   for (auto it = mObservers.Iter(); !it.Done(); it.Next()) {
@@ -978,17 +983,17 @@ SVGRenderingObserverList::InvalidateAll(
   mObservers.Clear();
 
   for (uint32_t i = 0; i < observers.Length(); ++i) {
     observers[i]->OnNonDOMMutationRenderingChange();
   }
 }
 
 void
-SVGRenderingObserverList::InvalidateAllForReflow()
+SVGRenderingObserverSet::InvalidateAllForReflow()
 {
   if (mObservers.IsEmpty()) {
     return;
   }
 
   AutoTArray<SVGRenderingObserver*,10> observers;
 
   for (auto it = mObservers.Iter(); !it.Done(); it.Next()) {
@@ -1000,53 +1005,53 @@ SVGRenderingObserverList::InvalidateAllF
   }
 
   for (uint32_t i = 0; i < observers.Length(); ++i) {
     observers[i]->OnNonDOMMutationRenderingChange();
   }
 }
 
 void
-SVGRenderingObserverList::RemoveAll()
+SVGRenderingObserverSet::RemoveAll()
 {
   AutoTArray<SVGRenderingObserver*,10> observers;
 
   for (auto it = mObservers.Iter(); !it.Done(); it.Next()) {
     observers.AppendElement(it.Get()->GetKey());
   }
   mObservers.Clear();
 
   // Our list is now cleared.  We need to notify the observers we've removed,
   // so they can update their state & remove themselves as mutation-observers.
   for (uint32_t i = 0; i < observers.Length(); ++i) {
-    observers[i]->NotifyEvictedFromRenderingObserverList();
+    observers[i]->NotifyEvictedFromRenderingObserverSet();
   }
 }
 
 
-static SVGRenderingObserverList*
-GetObserverList(Element* aElement)
+static SVGRenderingObserverSet*
+GetObserverSet(Element* aElement)
 {
-  return static_cast<SVGRenderingObserverList*>
-    (aElement->GetProperty(nsGkAtoms::renderingobserverlist));
+  return static_cast<SVGRenderingObserverSet*>
+    (aElement->GetProperty(nsGkAtoms::renderingobserverset));
 }
 
 #ifdef DEBUG
-// Defined down here because we need SVGRenderingObserverList's definition.
+// Defined down here because we need SVGRenderingObserverSet's definition.
 void
 SVGRenderingObserver::DebugObserverSet()
 {
   Element* referencedElement = GetReferencedElementWithoutObserving();
   if (referencedElement) {
-    SVGRenderingObserverList* observerList = GetObserverList(referencedElement);
-    bool inObserverList = observerList && observerList->Contains(this);
-    MOZ_ASSERT(inObserverList == mInObserverList,
-      "failed to track whether we're in our referenced element's observer list!");
+    SVGRenderingObserverSet* observers = GetObserverSet(referencedElement);
+    bool inObserverSet = observers && observers->Contains(this);
+    MOZ_ASSERT(inObserverSet == mInObserverList,
+      "failed to track whether we're in our referenced element's observer set!");
   } else {
-    MOZ_ASSERT(!mInObserverList, "In whose observer list are we, then?");
+    MOZ_ASSERT(!mInObserverList, "In whose observer set are we, then?");
   }
 }
 #endif
 
 
 typedef nsInterfaceHashtable<nsRefPtrHashKey<URLAndReferrerInfo>,
                              nsIMutationObserver> URIObserverHashtable;
 
@@ -1546,80 +1551,80 @@ SVGObserverUtils::UpdateEffects(nsIFrame
     GetEffectProperty(markerURL, aFrame, MarkerEndProperty());
   }
 }
 
 void
 SVGObserverUtils::AddRenderingObserver(Element* aElement,
                                        SVGRenderingObserver* aObserver)
 {
-  SVGRenderingObserverList* observerList = GetObserverList(aElement);
-  if (!observerList) {
-    observerList = new SVGRenderingObserverList();
-    if (!observerList)
+  SVGRenderingObserverSet* observers = GetObserverSet(aElement);
+  if (!observers) {
+    observers = new SVGRenderingObserverSet();
+    if (!observers) {
       return;
-    aElement->SetProperty(nsGkAtoms::renderingobserverlist, observerList,
-                          nsINode::DeleteProperty<SVGRenderingObserverList>);
+    }
+    aElement->SetProperty(nsGkAtoms::renderingobserverset, observers,
+                          nsINode::DeleteProperty<SVGRenderingObserverSet>);
   }
   aElement->SetHasRenderingObservers(true);
-  observerList->Add(aObserver);
+  observers->Add(aObserver);
 }
 
 void
 SVGObserverUtils::RemoveRenderingObserver(Element* aElement,
                                           SVGRenderingObserver* aObserver)
 {
-  SVGRenderingObserverList* observerList = GetObserverList(aElement);
-  if (observerList) {
-    NS_ASSERTION(observerList->Contains(aObserver),
+  SVGRenderingObserverSet* observers = GetObserverSet(aElement);
+  if (observers) {
+    NS_ASSERTION(observers->Contains(aObserver),
                  "removing observer from an element we're not observing?");
-    observerList->Remove(aObserver);
-    if (observerList->IsEmpty()) {
+    observers->Remove(aObserver);
+    if (observers->IsEmpty()) {
       aElement->SetHasRenderingObservers(false);
     }
   }
 }
 
 void
 SVGObserverUtils::RemoveAllRenderingObservers(Element* aElement)
 {
-  SVGRenderingObserverList* observerList = GetObserverList(aElement);
-  if (observerList) {
-    observerList->RemoveAll();
+  SVGRenderingObserverSet* observers = GetObserverSet(aElement);
+  if (observers) {
+    observers->RemoveAll();
     aElement->SetHasRenderingObservers(false);
   }
 }
 
 void
 SVGObserverUtils::InvalidateRenderingObservers(nsIFrame* aFrame)
 {
   NS_ASSERTION(!aFrame->GetPrevContinuation(), "aFrame must be first continuation");
 
   nsIContent* content = aFrame->GetContent();
   if (!content || !content->IsElement())
     return;
 
   // If the rendering has changed, the bounds may well have changed too:
   aFrame->DeleteProperty(nsSVGUtils::ObjectBoundingBoxProperty());
 
-  SVGRenderingObserverList* observerList =
-    GetObserverList(content->AsElement());
-  if (observerList) {
-    observerList->InvalidateAll();
+  SVGRenderingObserverSet* observers = GetObserverSet(content->AsElement());
+  if (observers) {
+    observers->InvalidateAll();
     return;
   }
 
   // Check ancestor SVG containers. The root frame cannot be of type
   // eSVGContainer so we don't have to check f for null here.
   for (nsIFrame *f = aFrame->GetParent();
        f->IsFrameOfType(nsIFrame::eSVGContainer); f = f->GetParent()) {
     if (f->GetContent()->IsElement()) {
-      observerList = GetObserverList(f->GetContent()->AsElement());
-      if (observerList) {
-        observerList->InvalidateAll();
+      observers = GetObserverSet(f->GetContent()->AsElement());
+      if (observers) {
+        observers->InvalidateAll();
         return;
       }
     }
   }
 }
 
 void
 SVGObserverUtils::InvalidateDirectRenderingObservers(Element* aElement,
@@ -1627,22 +1632,22 @@ SVGObserverUtils::InvalidateDirectRender
 {
   nsIFrame* frame = aElement->GetPrimaryFrame();
   if (frame) {
     // If the rendering has changed, the bounds may well have changed too:
     frame->DeleteProperty(nsSVGUtils::ObjectBoundingBoxProperty());
   }
 
   if (aElement->HasRenderingObservers()) {
-    SVGRenderingObserverList* observerList = GetObserverList(aElement);
-    if (observerList) {
+    SVGRenderingObserverSet* observers = GetObserverSet(aElement);
+    if (observers) {
       if (aFlags & INVALIDATE_REFLOW) {
-        observerList->InvalidateAllForReflow();
+        observers->InvalidateAllForReflow();
       } else {
-        observerList->InvalidateAll();
+        observers->InvalidateAll();
       }
     }
   }
 }
 
 void
 SVGObserverUtils::InvalidateDirectRenderingObservers(nsIFrame* aFrame,
                                                      uint32_t aFlags /* = 0 */)
--- a/layout/svg/SVGObserverUtils.h
+++ b/layout/svg/SVGObserverUtils.h
@@ -116,17 +116,17 @@ public:
    * observe an SVG pattern, and an element in that pattern references and
    * observes a gradient that has changed).
    */
   void OnNonDOMMutationRenderingChange();
 
   // When a SVGRenderingObserver list gets forcibly cleared, it uses this
   // callback to notify every observer that's cleared from it, so they can
   // react.
-  void NotifyEvictedFromRenderingObserverList();
+  void NotifyEvictedFromRenderingObserverSet();
 
   nsIFrame* GetAndObserveReferencedFrame();
   /**
    * @param aOK this is only for the convenience of callers. We set *aOK to false
    * if the frame is the wrong type
    */
   nsIFrame* GetAndObserveReferencedFrame(mozilla::LayoutFrameType aFrameType,
                                          bool* aOK);
--- a/xpcom/ds/StaticAtoms.py
+++ b/xpcom/ds/StaticAtoms.py
@@ -946,17 +946,17 @@ STATIC_ATOMS = [
     Atom("readonly", "readonly"),
     Atom("rect", "rect"),
     Atom("rectangle", "rectangle"),
     Atom("refresh", "refresh"),
     Atom("rel", "rel"),
     Atom("rem", "rem"),
     Atom("remote", "remote"),
     Atom("removeelement", "removeelement"),
-    Atom("renderingobserverlist", "renderingobserverlist"),
+    Atom("renderingobserverset", "renderingobserverset"),
     Atom("repeat", "repeat"),
     Atom("replace", "replace"),
     Atom("requestcontextid", "requestcontextid"),
     Atom("required", "required"),
     Atom("reserved", "reserved"),
     Atom("reset", "reset"),
     Atom("resizeafter", "resizeafter"),
     Atom("resizebefore", "resizebefore"),