Bug 1510139 - Part 2. Do not invalidate vector image containers synchronously. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Thu, 03 Jan 2019 12:55:06 -0500
changeset 513994 b709624ce8fa0ca4146d8d0675036440e6807615
parent 513993 325d06d3ba7341035f01ae14bc882643d0235083
child 513995 41184f42f451ae006abb9de0033f6c870d7ab05e
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1510139
milestone66.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 1510139 - Part 2. Do not invalidate vector image containers synchronously. r=tnikkel Originally we would invalidate image containers synchronously when SVGRootRenderingObserver::OnRenderingChange was called. However at this point in time, the layout tree is in the middle of updating its own state, and triggering a paint will be problematic. Animated vector images did not suffer from this problem because they would defer to the next refresh tick, but non-animated do not receive refresh tick events. As such, we should just defer the invalidation to immediately after the layout tree has finished updating itself. Differential Revision: https://phabricator.services.mozilla.com/D15668
image/VectorImage.cpp
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -537,37 +537,38 @@ VectorImage::RequestRefresh(const TimeSt
     tracker->TriggerPendingAnimationsOnNextTick(aTime);
   }
 
   EvaluateAnimation();
 
   mSVGDocumentWrapper->TickRefreshDriver();
 
   if (mHasPendingInvalidation) {
-    mHasPendingInvalidation = false;
     SendInvalidationNotifications();
   }
 }
 
 void VectorImage::SendInvalidationNotifications() {
   // Animated images don't send out invalidation notifications as soon as
   // they're generated. Instead, InvalidateObserversOnNextRefreshDriverTick
   // records that there are pending invalidations and then returns immediately.
   // The notifications are actually sent from RequestRefresh(). We send these
   // notifications there to ensure that there is actually a document observing
   // us. Otherwise, the notifications are just wasted effort.
   //
-  // Non-animated images call this method directly from
+  // Non-animated images post an event to call this method from
   // InvalidateObserversOnNextRefreshDriverTick, because RequestRefresh is never
   // called for them. Ordinarily this isn't needed, since we send out
   // invalidation notifications in OnSVGDocumentLoaded, but in rare cases the
   // SVG document may not be 100% ready to render at that time. In those cases
   // we would miss the subsequent invalidations if we didn't send out the
-  // notifications directly in |InvalidateObservers...|.
+  // notifications indirectly in |InvalidateObservers...|.
 
+  MOZ_ASSERT(mHasPendingInvalidation);
+  mHasPendingInvalidation = false;
   SurfaceCache::RemoveImage(ImageKey(this));
 
   if (UpdateImageContainer(Nothing())) {
     // If we have image containers, that means we probably won't get a Draw call
     // from the owner since they are using the container. We must assume all
     // invalidations need to be handled.
     MOZ_ASSERT(mRenderingObserver, "Should have a rendering observer by now");
     mRenderingObserver->ResumeHonoringInvalidations();
@@ -1467,21 +1468,47 @@ VectorImage::OnDataAvailable(nsIRequest*
   return mSVGDocumentWrapper->OnDataAvailable(aRequest, aCtxt, aInStr,
                                               aSourceOffset, aCount);
 }
 
 // --------------------------
 // Invalidation helper method
 
 void VectorImage::InvalidateObserversOnNextRefreshDriverTick() {
+  if (mHasPendingInvalidation) {
+    return;
+  }
+
+  mHasPendingInvalidation = true;
+
+  // Animated images can wait for the refresh tick.
   if (mHaveAnimations) {
-    mHasPendingInvalidation = true;
+    return;
+  }
+
+  // Non-animated images won't get the refresh tick, so we should just send an
+  // invalidation outside the current execution context. We need to defer
+  // because the layout tree is in the middle of invalidation, and the tree
+  // state needs to be consistent. Specifically only some of the frames have
+  // had the NS_FRAME_DESCENDANT_NEEDS_PAINT and/or NS_FRAME_NEEDS_PAINT bits
+  // set by InvalidateFrameInternal in layout/generic/nsFrame.cpp. These bits
+  // get cleared when we repaint the SVG into a surface by
+  // nsIFrame::ClearInvalidationStateBits in nsDisplayList::PaintRoot.
+  nsCOMPtr<nsIEventTarget> eventTarget;
+  if (mProgressTracker) {
+    eventTarget = mProgressTracker->GetEventTarget();
   } else {
-    SendInvalidationNotifications();
+    eventTarget = do_GetMainThread();
   }
+
+  RefPtr<VectorImage> self(this);
+  nsCOMPtr<nsIRunnable> ev(NS_NewRunnableFunction(
+      "VectorImage::SendInvalidationNotifications",
+      [=]() -> void { self->SendInvalidationNotifications(); }));
+  eventTarget->Dispatch(ev.forget(), NS_DISPATCH_NORMAL);
 }
 
 void VectorImage::PropagateUseCounters(Document* aParentDocument) {
   Document* doc = mSVGDocumentWrapper->GetDocument();
   if (doc) {
     doc->PropagateUseCounters(aParentDocument);
   }
 }