Bug 1368776 - Part 5. Refactor ImageResource::GetCurrentImage to reduce code duplication. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Fri, 17 Nov 2017 06:45:25 -0500
changeset 444221 ae1b16e57d97af77c2fedc5395ffb0800362e198
parent 444220 7ebbec7a624ae13e6a5048b0a57ae9ee87747d41
child 444222 e9c89d2b9f99fd87749b63bf5b027b39d28a09d7
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [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 5. Refactor ImageResource::GetCurrentImage to reduce code duplication. r=tnikkel
image/Image.cpp
image/Image.h
--- a/image/Image.cpp
+++ b/image/Image.cpp
@@ -47,36 +47,50 @@ ImageMemoryCounter::ImageMemoryCounter(I
   }
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // Image Base Types
 ///////////////////////////////////////////////////////////////////////////////
 
-Pair<DrawResult, RefPtr<layers::Image>>
-ImageResource::GetCurrentImage(ImageContainer* aContainer,
+DrawResult
+ImageResource::AddCurrentImage(ImageContainer* aContainer,
                                const IntSize& aSize,
-                               uint32_t aFlags)
+                               uint32_t aFlags,
+                               bool aInTransaction)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aContainer);
 
   DrawResult drawResult;
   RefPtr<SourceSurface> surface;
   Tie(drawResult, surface) =
     GetFrameInternal(aSize, FRAME_CURRENT, aFlags | FLAG_ASYNC_NOTIFY);
   if (!surface) {
     // 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 MakePair(drawResult, RefPtr<layers::Image>());
+    return drawResult;
   }
 
+  // |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);
-  return MakePair(drawResult, Move(image));
+  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());
@@ -114,65 +128,38 @@ ImageResource::GetImageContainerImpl(Lay
         MOZ_ASSERT_UNREACHABLE("Unhandled DrawResult type!");
         return container.forget();
     }
   } else {
     // We need a new ImageContainer, so create one.
     container = LayerManager::CreateImageContainer();
   }
 
-  DrawResult drawResult;
-  RefPtr<layers::Image> image;
-  Tie(drawResult, image) = GetCurrentImage(container, aSize, aFlags);
-  if (!image) {
-    return nullptr;
-  }
-
 #ifdef DEBUG
   NotifyDrawingObservers();
 #endif
 
-  // |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.
-  AutoTArray<ImageContainer::NonOwningImage, 1> imageList;
-  imageList.AppendElement(ImageContainer::NonOwningImage(image, TimeStamp(),
-                                                         mLastFrameID++,
-                                                         mImageProducerID));
-  container->SetCurrentImagesInTransaction(imageList);
-
-  mLastImageContainerDrawResult = drawResult;
+  mLastImageContainerDrawResult =
+    AddCurrentImage(container, aSize, aFlags, true);
   mImageContainer = container;
-
   return container.forget();
 }
 
 void
 ImageResource::UpdateImageContainer(const IntSize& aSize)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   RefPtr<layers::ImageContainer> container = mImageContainer.get();
   if (!container) {
     return;
   }
 
-  DrawResult drawResult;
-  RefPtr<layers::Image> image;
-  Tie(drawResult, image) = GetCurrentImage(container, aSize, FLAG_NONE);
-  if (!image) {
-    return;
-  }
-
-  mLastImageContainerDrawResult = drawResult;
-  AutoTArray<ImageContainer::NonOwningImage, 1> imageList;
-  imageList.AppendElement(ImageContainer::NonOwningImage(image, TimeStamp(),
-                                                         mLastFrameID++,
-                                                         mImageProducerID));
-  container->SetCurrentImages(imageList);
+  mLastImageContainerDrawResult =
+    AddCurrentImage(container, aSize, FLAG_NONE, false);
 }
 
 void
 ImageResource::ReleaseImageContainer()
 {
   MOZ_ASSERT(NS_IsMainThread());
   mImageContainer = nullptr;
 }
--- a/image/Image.h
+++ b/image/Image.h
@@ -339,31 +339,31 @@ protected:
   virtual Pair<DrawResult, RefPtr<gfx::SourceSurface>>
     GetFrameInternal(const gfx::IntSize& aSize,
                      uint32_t aWhichFrame,
                      uint32_t aFlags)
   {
     return MakePair(DrawResult::BAD_IMAGE, RefPtr<gfx::SourceSurface>());
   }
 
-  Pair<DrawResult, RefPtr<layers::Image>>
-    GetCurrentImage(layers::ImageContainer* aContainer,
-                    const gfx::IntSize& aSize,
-                    uint32_t aFlags);
-
   already_AddRefed<layers::ImageContainer>
     GetImageContainerImpl(layers::LayerManager* aManager,
                           const gfx::IntSize& aSize,
                           uint32_t aFlags);
 
   void UpdateImageContainer(const gfx::IntSize& aSize);
 
   void ReleaseImageContainer();
 
 private:
+  DrawResult AddCurrentImage(layers::ImageContainer* aContainer,
+                             const gfx::IntSize& aSize,
+                             uint32_t aFlags,
+                             bool aInTransaction);
+
   // A weak pointer to our ImageContainer, which stays alive only as long as
   // the layer system needs it.
   WeakPtr<layers::ImageContainer> mImageContainer;
 
   layers::ImageContainer::ProducerID mImageProducerID;
   layers::ImageContainer::FrameID mLastFrameID;
 
   // If mImageContainer is non-null, this contains the DrawResult we obtained