Bug 1444387 - Part 1. Avoid using fallback if an image is not ready. r=jrmuizel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 13 Mar 2018 09:16:04 -0400
changeset 407891 68ab33989e635a35ba5393e1f116e95ac76547a3
parent 407890 f1e6aeb55b95a345abb3f3726f0610b038c77f48
child 407892 16155f54ef561415aaa28896b84c3da23e4ec508
push id100796
push useraosmond@gmail.com
push dateTue, 13 Mar 2018 13:16:18 +0000
treeherdermozilla-inbound@16155f54ef56 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1444387
milestone61.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 1444387 - Part 1. Avoid using fallback if an image is not ready. r=jrmuizel If an image container is empty, it will not produce an image key for use with WebRender. This is generally not a sign of failure because the producer likely has yet to populate the container with data. As such, we should not immediately attempt to fallback. In fact, fallback can make things worse in this situation, as we will create an image client to send over the data, but then find that there is no data to share (or find that there is, due to a race with the producer thread, and use image clients when we could use shared surfaces).
layout/generic/nsImageFrame.cpp
layout/generic/nsPluginFrame.cpp
layout/generic/nsVideoFrame.cpp
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -1739,17 +1739,21 @@ nsDisplayImage::CreateWebRenderCommands(
     nsLayoutUtils::ComputeImageContainerDrawingParameters(mImage, mFrame, destRect,
                                                           aSc, flags, svgContext);
   RefPtr<ImageContainer> container =
     mImage->GetImageContainerAtSize(aManager, decodeSize, svgContext, flags);
   if (!container) {
     return false;
   }
 
-  return aManager->CommandBuilder().PushImage(this, container, aBuilder, aResources, aSc, destRect);
+  // If the image container is empty, we don't want to fallback. Any other
+  // failure will be due to resource constraints and fallback is unlikely to
+  // help us. Hence we can ignore the return value from PushImage.
+  aManager->CommandBuilder().PushImage(this, container, aBuilder, aResources, aSc, destRect);
+  return true;
 }
 
 ImgDrawResult
 nsImageFrame::PaintImage(gfxContext& aRenderingContext, nsPoint aPt,
                          const nsRect& aDirtyRect, imgIContainer* aImage,
                          uint32_t aFlags)
 {
   DrawTarget* drawTarget = aRenderingContext.GetDrawTarget();
--- a/layout/generic/nsPluginFrame.cpp
+++ b/layout/generic/nsPluginFrame.cpp
@@ -1437,18 +1437,22 @@ nsPluginFrame::CreateWebRenderCommands(n
 #endif
 
   RefPtr<LayerManager> lm = aDisplayListBuilder->GetWidgetLayerManager();
   if (!mDidCompositeObserver || !mDidCompositeObserver->IsValid(lm)) {
     mDidCompositeObserver = MakeUnique<PluginFrameDidCompositeObserver>(mInstanceOwner, lm);
   }
   lm->AddDidCompositeObserver(mDidCompositeObserver.get());
 
+  // If the image container is empty, we don't want to fallback. Any other
+  // failure will be due to resource constraints and fallback is unlikely to
+  // help us. Hence we can ignore the return value from PushImage.
   LayoutDeviceRect dest(r.x, r.y, size.width, size.height);
-  return aManager->CommandBuilder().PushImage(aItem, container, aBuilder, aResources, aSc, dest);
+  aManager->CommandBuilder().PushImage(aItem, container, aBuilder, aResources, aSc, dest);
+  return true;
 }
 
 
 already_AddRefed<Layer>
 nsPluginFrame::BuildLayer(nsDisplayListBuilder* aBuilder,
                           LayerManager* aManager,
                           nsDisplayItem* aItem,
                           const ContainerLayerParameters& aContainerParameters)
--- a/layout/generic/nsVideoFrame.cpp
+++ b/layout/generic/nsVideoFrame.cpp
@@ -489,18 +489,22 @@ public:
 
     VideoInfo::Rotation rotationDeg = element->RotationDegrees();
     IntSize scaleHint(static_cast<int32_t>(destGFXRect.Width()),
                       static_cast<int32_t>(destGFXRect.Height()));
     // scaleHint is set regardless of rotation, so swap w/h if needed.
     SwapScaleWidthHeightForRotation(scaleHint, rotationDeg);
     container->SetScaleHint(scaleHint);
 
+    // If the image container is empty, we don't want to fallback. Any other
+    // failure will be due to resource constraints and fallback is unlikely to
+    // help us. Hence we can ignore the return value from PushImage.
     LayoutDeviceRect rect(destGFXRect.x, destGFXRect.y, destGFXRect.width, destGFXRect.height);
-    return aManager->CommandBuilder().PushImage(this, container, aBuilder, aResources, aSc, rect);
+    aManager->CommandBuilder().PushImage(this, container, aBuilder, aResources, aSc, rect);
+    return true;
   }
 
   // It would be great if we could override GetOpaqueRegion to return nonempty here,
   // but it's probably not safe to do so in general. Video frames are
   // updated asynchronously from decoder threads, and it's possible that
   // we might have an opaque video frame when GetOpaqueRegion is called, but
   // when we come to paint, the video frame is transparent or has gone
   // away completely (e.g. because of a decoder error). The problem would