Bug 1554777 - Call SchedulePaint() rather than MarkNeedsDisplayItemRebuild() when we get the size available notification for a style image. r=mattwoodrow,tnikkel
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 15 Aug 2019 19:03:18 +0000
changeset 488473 da9ae31b5000eccbc38438680952ff08d06addea
parent 488472 870a350fb1360725bb7206cf8c392ef3a8d400e5
child 488474 87ed773c0a80425133bae39146f06de4a7425974
push id92769
push userealvarez@mozilla.com
push dateFri, 16 Aug 2019 09:49:34 +0000
treeherderautoland@87ed773c0a80 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow, tnikkel
bugs1554777
milestone70.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 1554777 - Call SchedulePaint() rather than MarkNeedsDisplayItemRebuild() when we get the size available notification for a style image. r=mattwoodrow,tnikkel So the issue here is that this test-case has a zero-sized border, but still with a border-image (which we should paint). So the first time we get here, the image is not loaded, and thus we don't get here: https://searchfox.org/mozilla-central/rev/e0b0c38ee83f99d3cf868bad525ace4a395039f1/layout/painting/nsDisplayList.h#4254 Which means that we end up with zero bounds and thus we don't even get to the border painting code. However, when the image loads, we get to MarkNeedsDisplayItemRebuild(), but that doesn't schedule any paint, only marks the frames as modified in order for display items to be rebuilt _eventually_. Thus eventually, where we force a repaint by other means, we paint correctly. This only works in more general cases because we get to the nsImageRenderer code which does vastly different stuff. InvalidateFrame() seems to do the right thing and schedule a paint, so use it. It's not clear to me which one of nsIFrame::InvalidateFrame() or nsIFrame::SchedulePaint() we should use... If I understand correctly, InvalidateFrame() will only do something iff there are display items for the frame, so that should make the IsVisible() check redundant. Note however that I think there's still a race condition, if we get to paint in between the SIZE_AVAILABLE notification (the one where we actually invalidate the display items), and the LOAD notification (the one the border-image code checks). I'll send a separate patch for that, I think SIZE_AVAILABLE should be a strong-enough hint and that allows us to remove nsStyleImage::IsLoaded()... The RequestReflow stuff also looks highly suspicious... shape-outside sync-decodes, and it seems we could end up invalidating reflow from the reflow code... Differential Revision: https://phabricator.services.mozilla.com/D41005
layout/style/ImageLoader.cpp
--- a/layout/style/ImageLoader.cpp
+++ b/layout/style/ImageLoader.cpp
@@ -691,20 +691,19 @@ nsresult ImageLoader::OnSizeAvailable(im
 
   aImage->SetAnimationMode(presContext->ImageAnimationMode());
 
   FrameSet* frameSet = mRequestToFrameMap.Get(aRequest);
   if (!frameSet) {
     return NS_OK;
   }
 
-  for (FrameWithFlags& fwf : *frameSet) {
-    nsIFrame* frame = fwf.mFrame;
-    if (frame->StyleVisibility()->IsVisible()) {
-      frame->MarkNeedsDisplayItemRebuild();
+  for (const FrameWithFlags& fwf : *frameSet) {
+    if (fwf.mFrame->StyleVisibility()->IsVisible()) {
+      fwf.mFrame->SchedulePaint();
     }
   }
 
   return NS_OK;
 }
 
 nsresult ImageLoader::OnImageIsAnimated(imgIRequest* aRequest) {
   if (!mDocument) {