Bug 1494092. Remove SVGFilterObserverList::IsInObserverLists and related code. r=mattwoodrow
authorJonathan Watt <jwatt@jwatt.org>
Mon, 27 Aug 2018 17:05:37 +0100
changeset 496781 d2218beee05235644592c9c7f36ec8424e1b1f3c
parent 496780 0f22d244af4dea74766377b7fc3e3bcd292f9057
child 496782 74f5bf70f7dc9d26d0017bd1ab535b1edaf5619e
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1494092
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 1494092. Remove SVGFilterObserverList::IsInObserverLists and related code. r=mattwoodrow This code is no longer necessary since we now invalidate using Display List Based Invalidation instead of using recursive calls up the frame tree. The tests that are marked as failing have only been passing due to a bug in the code that's being removed from nsSVGIntegrationUtils.cpp which coincidentally hides the fact that we are actually invalidating in those tests given their particular structure (which the tests are supposed to be checking we're not doing). Differential Revision: https://phabricator.services.mozilla.com/D6850
layout/reftests/invalidation/reftest.list
layout/svg/SVGObserverUtils.cpp
layout/svg/SVGObserverUtils.h
layout/svg/nsSVGIntegrationUtils.cpp
--- a/layout/reftests/invalidation/reftest.list
+++ b/layout/reftests/invalidation/reftest.list
@@ -39,18 +39,18 @@ pref(layout.animated-image-layers.enable
 == filter-userspace-offset.svg?offsetContainer=foreignObject&mask=boundingBox filter-userspace-offset.svg
 == filter-userspace-offset.svg?offsetContainer=rect&mask=userSpace-at100 filter-userspace-offset.svg
 == filter-userspace-offset.svg?offsetContainer=use&mask=userSpace-atZero filter-userspace-offset.svg
 == filter-userspace-offset.svg?offsetContainer=innerSVG&mask=userSpace-atZero filter-userspace-offset.svg
 == filter-userspace-offset.svg?offsetContainer=foreignObject&mask=userSpace-at100 filter-userspace-offset.svg
 == filter-userspace-offset.svg?offsetContainer=rect&filter=matrix-fillPaint-boundingBox filter-userspace-offset.svg
 == filter-userspace-offset.svg?offsetContainer=rect&filter=matrix-fillPaint-userSpace-at100 filter-userspace-offset.svg
 
-fails-if(webrender) != scroll-inactive-layers.html about:blank
-fails-if(webrender) != scroll-inactive-layers-2.html about:blank
+fails-if(webrender) fails-if(!Android) != scroll-inactive-layers.html about:blank   # bug 1494110 for the fails-if(!Android) (Android is a false pass, no doubt)
+fails-if(webrender) fails-if(!Android) != scroll-inactive-layers-2.html about:blank # bug 1494110 for the fails-if(!Android) (Android is a false pass, no doubt)
 != inactive-layertree-visible-region-1.html about:blank
 != inactive-layertree-visible-region-2.html about:blank
 != transform-floating-point-invalidation.html about:blank
 != transform-floating-point-invalidation.html?reverse about:blank
 != nudge-to-integer-invalidation.html about:blank
 != nudge-to-integer-invalidation.html?reverse about:blank
 != clipped-animated-transform-1.html about:blank
 != paintedlayer-recycling-1.html about:blank
--- a/layout/svg/SVGObserverUtils.cpp
+++ b/layout/svg/SVGObserverUtils.cpp
@@ -362,27 +362,16 @@ SVGFilterObserverList::ReferencesValidRe
   for (uint32_t i = 0; i < mObservers.Length(); i++) {
     if (!mObservers[i]->ReferencesValidResource()) {
       return false;
     }
   }
   return true;
 }
 
-bool
-SVGFilterObserverList::IsInObserverLists() const
-{
-  for (uint32_t i = 0; i < mObservers.Length(); i++) {
-    if (!mObservers[i]->IsInObserverList()) {
-      return false;
-    }
-  }
-  return true;
-}
-
 void
 SVGFilterObserverListForCSSProp::OnRenderingChange()
 {
   nsIFrame* frame = mFrameReference.Get();
   if (!frame)
     return;
 
   // Repaint asynchronously in case the filter frame is being torn down
--- a/layout/svg/SVGObserverUtils.h
+++ b/layout/svg/SVGObserverUtils.h
@@ -114,18 +114,16 @@ public:
    */
   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();
 
-  bool IsInObserverList() const { return mInObserverList; }
-
   nsIFrame* GetReferencedFrame();
   /**
    * @param aOK this is only for the convenience of callers. We set *aOK to false
    * if the frame is the wrong type
    */
   nsIFrame* GetReferencedFrame(mozilla::LayoutFrameType aFrameType, bool* aOK);
 
   Element* GetReferencedElement();
@@ -361,17 +359,16 @@ NS_DEFINE_STATIC_IID_ACCESSOR(SVGFilterO
 class SVGFilterObserverList : public nsISupports
 {
 public:
   SVGFilterObserverList(const nsTArray<nsStyleFilter>& aFilters,
                         nsIContent* aFilteredElement,
                         nsIFrame* aFiltedFrame = nullptr);
 
   bool ReferencesValidResources();
-  bool IsInObserverLists() const;
   void Invalidate() { OnRenderingChange(); }
 
   // nsISupports
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(SVGFilterObserverList)
 
 protected:
   virtual ~SVGFilterObserverList();
--- a/layout/svg/nsSVGIntegrationUtils.cpp
+++ b/layout/svg/nsSVGIntegrationUtils.cpp
@@ -320,27 +320,28 @@ nsSVGIntegrationUtils::AdjustInvalidArea
   }
 
   // Don't bother calling GetEffectProperties; the filter property should
   // already have been set up during reflow/ComputeFrameEffectsRect
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(aFrame);
   SVGFilterObserverListForCSSProp* observers =
     SVGObserverUtils::GetFilterObserverList(firstFrame);
-  if (!observers || !observers->IsInObserverLists()) {
-    return aInvalidRegion;
-  }
 
   int32_t appUnitsPerDevPixel = aFrame->PresContext()->AppUnitsPerDevPixel();
 
   if (!observers || !observers->ReferencesValidResources()) {
     // The frame is either not there or not currently available,
     // perhaps because we're in the middle of tearing stuff down.
     // Be conservative, return our visual overflow rect relative
     // to the reference frame.
+    // XXX we may get here purely due to an SVG mask, in which case there is
+    // no need to do this it causes over-invalidation. See:
+    // https://bugzilla.mozilla.org/show_bug.cgi?id=1494110
+    // Do we still even need this for filters? If so, why?
     nsRect overflow = aFrame->GetVisualOverflowRect() + aToReferenceFrame;
     return overflow.ToOutsidePixels(appUnitsPerDevPixel);
   }
 
   // Convert aInvalidRegion into bounding box frame space in app units:
   nsPoint toBoundingBox =
     aFrame->GetOffsetTo(firstFrame) + GetOffsetToBoundingBox(firstFrame);
   // The initial rect was relative to the reference frame, so we need to