Bug 1368776 - Part 14. Make ImageResource::GetImageContainerImpl handle differing suggested sizes. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 17 Nov 2017 06:45:27 -0500
changeset 392475 ea3f6961ec397c07c6a6d4ffc2c72c402a6ea1dc
parent 392474 400c34e4ec89e774e976781d2809d8930bd80b22
child 392476 b9a29d94ccac646c9336fa75e084bbc8581501ad
push id32921
push usernerli@mozilla.com
push dateFri, 17 Nov 2017 22:02:18 +0000
treeherdermozilla-central@daa0dcd1616c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1368776
milestone59.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 1368776 - Part 14. Make ImageResource::GetImageContainerImpl handle differing suggested sizes. r=tnikkel
image/Image.cpp
image/Image.h
--- a/image/Image.cpp
+++ b/image/Image.cpp
@@ -47,57 +47,50 @@ ImageMemoryCounter::ImageMemoryCounter(I
   }
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // Image Base Types
 ///////////////////////////////////////////////////////////////////////////////
 
-DrawResult
-ImageResource::AddCurrentImage(ImageContainer* aContainer,
-                               const IntSize& aSize,
-                               uint32_t aFlags,
+void
+ImageResource::SetCurrentImage(ImageContainer* aContainer,
+                               SourceSurface* aSurface,
                                bool aInTransaction)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aContainer);
 
-  DrawResult drawResult;
-  IntSize size;
-  RefPtr<SourceSurface> surface;
-  Tie(drawResult, size, surface) =
-    GetFrameInternal(aSize, FRAME_CURRENT, aFlags | FLAG_ASYNC_NOTIFY);
-  if (!surface) {
+  if (!aSurface) {
     // The OS threw out some or all of our buffer. We'll need to wait for the
     // redecode (which was automatically triggered by GetFrame) to complete.
-    return drawResult;
+    return;
   }
 
   // |image| holds a reference to a SourceSurface which in turn holds a lock on
   // the current frame's data buffer, ensuring that it doesn't get freed as
   // long as the layer system keeps this ImageContainer alive.
-  RefPtr<layers::Image> image = new layers::SourceSurfaceImage(surface);
+  RefPtr<layers::Image> image = new layers::SourceSurfaceImage(aSurface);
 
   // We can share the producer ID with other containers because it is only
   // used internally to validate the frames given to a particular container
   // so that another object cannot add its own. Similarly the frame ID is
   // only used internally to ensure it is always increasing, and skipping
   // IDs from an individual container's perspective is acceptable.
   AutoTArray<ImageContainer::NonOwningImage, 1> imageList;
   imageList.AppendElement(ImageContainer::NonOwningImage(image, TimeStamp(),
                                                          mLastFrameID++,
                                                          mImageProducerID));
 
   if (aInTransaction) {
     aContainer->SetCurrentImagesInTransaction(imageList);
   } else {
     aContainer->SetCurrentImages(imageList);
   }
-  return drawResult;
 }
 
 already_AddRefed<ImageContainer>
 ImageResource::GetImageContainerImpl(LayerManager* aManager,
                                      const IntSize& aSize,
                                      uint32_t aFlags)
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -145,48 +138,114 @@ ImageResource::GetImageContainerImpl(Lay
         // Temporary conditions where we need to rerequest the frame to recover.
         break;
       case DrawResult::WRONG_SIZE:
         // Unused by GetFrameInternal
       default:
         MOZ_ASSERT_UNREACHABLE("Unhandled DrawResult type!");
         return container.forget();
     }
-  } else {
+  }
+
+#ifdef DEBUG
+  NotifyDrawingObservers();
+#endif
+
+  DrawResult drawResult;
+  IntSize bestSize;
+  RefPtr<SourceSurface> surface;
+  Tie(drawResult, bestSize, surface) =
+    GetFrameInternal(size, FRAME_CURRENT, aFlags | FLAG_ASYNC_NOTIFY);
+
+  // The requested size might be refused by the surface cache (i.e. due to
+  // factor-of-2 mode). In that case we don't want to create an entry for this
+  // specific size, but rather re-use the entry for the substituted size.
+  if (bestSize != size) {
+    MOZ_ASSERT(!bestSize.IsEmpty());
+
+    // We can only remove the entry if we no longer have a container, because if
+    // there are strong references to it remaining, we need to still update it
+    // in UpdateImageContainer.
+    if (i >= 0 && !container) {
+      mImageContainers.RemoveElementAt(i);
+    }
+
+    // Forget about the stale container, if any. This lets the entry creation
+    // logic do its job below, if it turns out there is no existing best entry
+    // or the best entry doesn't have a container.
+    container = nullptr;
+
+    // We need to do the entry search again for the new size. We skip pruning
+    // because we did this above once already, but ImageContainer is threadsafe,
+    // so there is a remote possibility it got freed.
+    i = mImageContainers.Length() - 1;
+    for (; i >= 0; --i) {
+      entry = &mImageContainers[i];
+      if (bestSize == entry->mSize) {
+        container = entry->mContainer.get();
+        if (container) {
+          switch (entry->mLastDrawResult) {
+            case DrawResult::SUCCESS:
+            case DrawResult::BAD_IMAGE:
+            case DrawResult::BAD_ARGS:
+              return container.forget();
+            case DrawResult::NOT_READY:
+            case DrawResult::INCOMPLETE:
+            case DrawResult::TEMPORARY_ERROR:
+              // Temporary conditions where we need to rerequest the frame to
+              // recover. We have already done so!
+              break;
+           case DrawResult::WRONG_SIZE:
+              // Unused by GetFrameInternal
+            default:
+              MOZ_ASSERT_UNREACHABLE("Unhandled DrawResult type!");
+              return container.forget();
+          }
+        }
+        break;
+      }
+    }
+  }
+
+  if (!container) {
     // We need a new ImageContainer, so create one.
     container = LayerManager::CreateImageContainer();
 
     if (i >= 0) {
       entry->mContainer = container;
     } else {
       entry = mImageContainers.AppendElement(
-        ImageContainerEntry(size, container.get()));
+        ImageContainerEntry(bestSize, container.get()));
     }
   }
 
-#ifdef DEBUG
-  NotifyDrawingObservers();
-#endif
-
-  entry->mLastDrawResult =
-    AddCurrentImage(container, size, aFlags, true);
+  SetCurrentImage(container, surface, true);
+  entry->mLastDrawResult = drawResult;
   return container.forget();
 }
 
 void
 ImageResource::UpdateImageContainer()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   for (int i = mImageContainers.Length() - 1; i >= 0; --i) {
     ImageContainerEntry& entry = mImageContainers[i];
     RefPtr<ImageContainer> container = entry.mContainer.get();
     if (container) {
-      entry.mLastDrawResult =
-        AddCurrentImage(container, entry.mSize, FLAG_NONE, false);
+      IntSize bestSize;
+      RefPtr<SourceSurface> surface;
+      Tie(entry.mLastDrawResult, bestSize, surface) =
+        GetFrameInternal(entry.mSize, FRAME_CURRENT, FLAG_ASYNC_NOTIFY);
+
+      // It is possible that this is a factor-of-2 substitution. Since we
+      // managed to convert the weak reference into a strong reference, that
+      // means that an imagelib user still is holding onto the container. thus
+      // we cannot consolidate and must keep updating the duplicate container.
+      SetCurrentImage(container, surface, false);
     } else {
       // Stop tracking if our weak pointer to the image container was freed.
       mImageContainers.RemoveElementAt(i);
     }
   }
 }
 
 void
--- a/image/Image.h
+++ b/image/Image.h
@@ -364,20 +364,19 @@ protected:
                           const gfx::IntSize& aSize,
                           uint32_t aFlags);
 
   void UpdateImageContainer();
 
   void ReleaseImageContainer();
 
 private:
-  DrawResult AddCurrentImage(layers::ImageContainer* aContainer,
-                             const gfx::IntSize& aSize,
-                             uint32_t aFlags,
-                             bool aInTransaction);
+  void SetCurrentImage(layers::ImageContainer* aContainer,
+                       gfx::SourceSurface* aSurface,
+                       bool aInTransaction);
 
   struct ImageContainerEntry {
     ImageContainerEntry(const gfx::IntSize& aSize,
                         layers::ImageContainer* aContainer)
       : mSize(aSize)
       , mContainer(aContainer)
       , mLastDrawResult(DrawResult::NOT_READY)
     { }