Bug 717872 - Move frame ownership to a separate container object, FrameSequence, which only exposes const access to its frames. r=seth

The eventual goal here is to have FrameBlenders be able to share
FrameSequences.

--HG--
extra : rebase_source : 97b8be9f53ed5cb14f0d46655d8fd7cdfa6bab67

image/src/FrameBlender.cpp

@@ -32,11 +32,11 @@ FrameBlender::GetFrame(uint32_t framenum) const
 {
   if (!mAnim) {
     NS_ASSERTION(framenum == 0, "Don't ask for a frame > 0 if we're not animated!");
-    return mFrames.SafeElementAt(0, FrameDataPair());
+    return mFrames.GetFrame(0);
   }
   if (mAnim->lastCompositedFrameIndex == int32_t(framenum))
     return mAnim->compositingFrame;
-  return mFrames.SafeElementAt(framenum, FrameDataPair());
+  return mFrames.GetFrame(framenum);
 }
 
 imgFrame*
@@ -44,69 +44,59 @@ FrameBlender::RawGetFrame(uint32_t framenum) const
 {
   if (!mAnim) {
     NS_ASSERTION(framenum == 0, "Don't ask for a frame > 0 if we're not animated!");
-    return mFrames.SafeElementAt(0, FrameDataPair());
+    return mFrames.GetFrame(0);
   }
 
-  return mFrames.SafeElementAt(framenum, FrameDataPair());
+  return mFrames.GetFrame(framenum);
 }
 
 uint32_t
 FrameBlender::GetNumFrames() const
 {
-  return mFrames.Length();
+  return mFrames.GetNumFrames();
 }
 
 void
 FrameBlender::RemoveFrame(uint32_t framenum)
 {
-  NS_ABORT_IF_FALSE(framenum < mFrames.Length(), "Deleting invalid frame!");
+  NS_ABORT_IF_FALSE(framenum < GetNumFrames(), "Deleting invalid frame!");
 
-  mFrames.RemoveElementAt(framenum);
+  mFrames.RemoveFrame(framenum);
 }
 
 void
 FrameBlender::ClearFrames()
 {
-  // Since FrameDataPair holds an nsAutoPtr to its frame, clearing the mFrames
-  // array also deletes all the frames.
-  mFrames.Clear();
+  mFrames.ClearFrames();
 }
 
 void
 FrameBlender::InsertFrame(uint32_t framenum, imgFrame* aFrame)
 {
-  NS_ABORT_IF_FALSE(framenum <= mFrames.Length(), "Inserting invalid frame!");
-  mFrames.InsertElementAt(framenum, aFrame);
+  NS_ABORT_IF_FALSE(framenum <= GetNumFrames(), "Inserting invalid frame!");
+  mFrames.InsertFrame(framenum, aFrame);
   if (GetNumFrames() > 1) {
     EnsureAnimExists();
-
-    // Whenever we have more than one frame, we always lock *all* our frames
-    // so we have all the image data pointers.
-    mFrames[framenum].LockAndGetData();
   }
 }
 
 imgFrame*
 FrameBlender::SwapFrame(uint32_t framenum, imgFrame* aFrame)
 {
-  NS_ABORT_IF_FALSE(framenum < mFrames.Length(), "Swapping invalid frame!");
+  NS_ABORT_IF_FALSE(framenum < GetNumFrames(), "Swapping invalid frame!");
 
-  FrameDataPair ret;
+  imgFrame* ret;
 
   // Steal the imgFrame from wherever it's currently stored
   if (mAnim && mAnim->lastCompositedFrameIndex == int32_t(framenum)) {
-    ret = mAnim->compositingFrame;
+    ret = mAnim->compositingFrame.Forget();
     mAnim->lastCompositedFrameIndex = -1;
-  } else if (framenum < mFrames.Length()) {
-    ret = mFrames[framenum];
+    nsAutoPtr<imgFrame> toDelete(mFrames.SwapFrame(framenum, aFrame));
+  } else {
+    ret = mFrames.SwapFrame(framenum, aFrame);
   }
 
-  mFrames.RemoveElementAt(framenum);
-  if (aFrame) {
-    InsertFrame(framenum, aFrame);
-  }
-
-  return ret.Forget();
+  return ret;
 }
 
 void
@@ -118,11 +108,7 @@ FrameBlender::EnsureAnimExists()
 
     // We should only get into this code path directly after we've created our
     // second frame (hence we know we're animated).
-    MOZ_ASSERT(mFrames.Length() == 2);
-
-    // Whenever we have more than one frame, we always lock *all* our frames
-    // so we have all the image data pointers.
-    mFrames[0].LockAndGetData();
+    MOZ_ASSERT(GetNumFrames() == 2);
   }
 }
 
@@ -138,8 +124,8 @@ FrameBlender::DoBlend(nsIntRect* aDirtyRect,
     return false;
   }
 
-  const FrameDataPair& prevFrame = mFrames[aPrevFrameIndex];
-  const FrameDataPair& nextFrame = mFrames[aNextFrameIndex];
+  const FrameDataPair& prevFrame = mFrames.GetFrame(aPrevFrameIndex);
+  const FrameDataPair& nextFrame = mFrames.GetFrame(aNextFrameIndex);
   if (!prevFrame.HasFrameData() || !nextFrame.HasFrameData()) {
     return false;
   }
@@ -555,12 +541,7 @@ size_t
 FrameBlender::SizeOfDecodedWithComputedFallbackIfHeap(gfxASurface::MemoryLocation aLocation,
                                                       MallocSizeOf aMallocSizeOf) const
 {
-  size_t n = 0;
-  for (uint32_t i = 0; i < mFrames.Length(); ++i) {
-    imgFrame* frame = mFrames.SafeElementAt(i, FrameDataPair());
-    NS_ABORT_IF_FALSE(frame, "Null frame in frame array!");
-    n += frame->SizeOfExcludingThisWithComputedFallbackIfHeap(aLocation, aMallocSizeOf);
-  }
+  size_t n = mFrames.SizeOfDecodedWithComputedFallbackIfHeap(aLocation, aMallocSizeOf);
 
   if (mAnim) {
     if (mAnim->compositingFrame) {

image/src/FrameBlender.h

@@ -7,132 +7,15 @@
 #ifndef mozilla_imagelib_FrameBlender_h_
 #define mozilla_imagelib_FrameBlender_h_
 
-#include "nsTArray.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/TimeStamp.h"
 #include "gfxASurface.h"
 #include "imgFrame.h"
+#include "FrameSequence.h"
 
 namespace mozilla {
 namespace image {
 
-/**
- * FrameDataPair is a slightly-smart tuple of (frame, raw frame data) where the
- * raw frame data is allowed to be (and is, initially) null.
- *
- * If you call LockAndGetData, you will be able to call GetFrameData() on that
- * instance, and when the FrameDataPair is destructed, the imgFrame lock will
- * be unlocked.
- */
-class FrameDataPair
-{
-public:
-  explicit FrameDataPair(imgFrame* frame)
-    : mFrame(frame)
-    , mFrameData(nullptr)
-  {}
-
-  FrameDataPair()
-    : mFrame(nullptr)
-    , mFrameData(nullptr)
-  {}
-
-  FrameDataPair(FrameDataPair& other)
-  {
-    mFrame = other.mFrame;
-    mFrameData = other.mFrameData;
-
-    // since mFrame is an nsAutoPtr, the assignment operator above actually
-    // nulls out other.mFrame. In order to fully assume ownership over the
-    // frame, we also null out the other's mFrameData.
-    other.mFrameData = nullptr;
-  }
-
-  ~FrameDataPair()
-  {
-    if (mFrameData) {
-      mFrame->UnlockImageData();
-    }
-  }
-
-  // Lock the frame and store its mFrameData. The frame will be unlocked (and
-  // deleted) when this FrameDataPair is deleted.
-  void LockAndGetData()
-  {
-    if (mFrame) {
-      if (NS_SUCCEEDED(mFrame->LockImageData())) {
-        if (mFrame->GetIsPaletted()) {
-          mFrameData = reinterpret_cast<uint8_t*>(mFrame->GetPaletteData());
-        } else {
-          mFrameData = mFrame->GetImageData();
-        }
-      }
-    }
-  }
-
-  // Null out this FrameDataPair and return its frame. You must ensure the
-  // frame will be deleted separately.
-  imgFrame* Forget()
-  {
-    if (mFrameData) {
-      mFrame->UnlockImageData();
-    }
-
-    imgFrame* frame = mFrame.forget();
-    mFrameData = nullptr;
-    return frame;
-  }
-
-  bool HasFrameData() const
-  {
-    if (mFrameData) {
-      MOZ_ASSERT(!!mFrame);
-    }
-    return !!mFrameData;
-  }
-
-  uint8_t* GetFrameData() const
-  {
-    return mFrameData;
-  }
-
-  imgFrame* GetFrame() const
-  {
-    return mFrame;
-  }
-
-  // Resets this FrameDataPair to work with a different frame. Takes ownership
-  // of the frame, deleting the old frame (if any).
-  void SetFrame(imgFrame* frame)
-  {
-    if (mFrameData) {
-      mFrame->UnlockImageData();
-    }
-
-    mFrame = frame;
-    mFrameData = nullptr;
-  }
-
-  operator imgFrame*() const
-  {
-    return GetFrame();
-  }
-
-  imgFrame* operator->() const
-  {
-    return GetFrame();
-  }
-
-  bool operator==(imgFrame* other) const
-  {
-    return mFrame == other;
-  }
-
-private:
-  nsAutoPtr<imgFrame> mFrame;
-  uint8_t* mFrameData;
-};
-
 /**
  * FrameBlender stores and gives access to imgFrames. It also knows how to
  * blend frames from previous to next, looping if necessary.
@@ -275,7 +158,7 @@ private:
 
 private: // data
   //! All the frames of the image
-  nsTArray<FrameDataPair> mFrames;
+  FrameSequence mFrames;
   nsIntSize mSize;
   Anim* mAnim;
 };

image/src/FrameSequence.cpp

@@ -0,0 +1,106 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "FrameSequence.h"
+
+using namespace mozilla;
+using namespace mozilla::image;
+
+namespace mozilla {
+namespace image {
+
+FrameSequence::~FrameSequence()
+{
+  ClearFrames();
+}
+
+const FrameDataPair&
+FrameSequence::GetFrame(uint32_t framenum) const
+{
+  if (framenum >= mFrames.Length()) {
+    static FrameDataPair empty;
+    return empty;
+  }
+
+  return mFrames[framenum];
+}
+
+uint32_t
+FrameSequence::GetNumFrames() const
+{
+  return mFrames.Length();
+}
+
+void
+FrameSequence::RemoveFrame(uint32_t framenum)
+{
+  NS_ABORT_IF_FALSE(framenum < mFrames.Length(), "Deleting invalid frame!");
+
+  mFrames.RemoveElementAt(framenum);
+}
+
+void
+FrameSequence::ClearFrames()
+{
+  // Since FrameDataPair holds an nsAutoPtr to its frame, clearing the mFrames
+  // array also deletes all the frames.
+  mFrames.Clear();
+}
+
+void
+FrameSequence::InsertFrame(uint32_t framenum, imgFrame* aFrame)
+{
+  NS_ABORT_IF_FALSE(framenum <= mFrames.Length(), "Inserting invalid frame!");
+  mFrames.InsertElementAt(framenum, aFrame);
+  if (GetNumFrames() > 1) {
+    // If we're creating our second element, we now know we're animated.
+    // Therefore, we need to lock the first frame too.
+    if (GetNumFrames() == 2) {
+      mFrames[0].LockAndGetData();
+    }
+
+    // Whenever we have more than one frame, we always lock *all* our frames
+    // so we have all the image data pointers.
+    mFrames[framenum].LockAndGetData();
+  }
+}
+
+imgFrame*
+FrameSequence::SwapFrame(uint32_t framenum, imgFrame* aFrame)
+{
+  NS_ABORT_IF_FALSE(framenum < mFrames.Length(), "Swapping invalid frame!");
+
+  FrameDataPair ret;
+
+  // Steal the imgFrame.
+  if (framenum < mFrames.Length()) {
+    ret = mFrames[framenum];
+  }
+
+  if (aFrame) {
+    mFrames.ReplaceElementAt(framenum, aFrame);
+  } else {
+    mFrames.RemoveElementAt(framenum);
+  }
+
+  return ret.Forget();
+}
+
+size_t
+FrameSequence::SizeOfDecodedWithComputedFallbackIfHeap(gfxASurface::MemoryLocation aLocation,
+                                                       MallocSizeOf aMallocSizeOf) const
+{
+  size_t n = 0;
+  for (uint32_t i = 0; i < mFrames.Length(); ++i) {
+    imgFrame* frame = mFrames.SafeElementAt(i, FrameDataPair());
+    NS_ABORT_IF_FALSE(frame, "Null frame in frame array!");
+    n += frame->SizeOfExcludingThisWithComputedFallbackIfHeap(aLocation, aMallocSizeOf);
+  }
+
+  return n;
+}
+
+} // namespace image
+} // namespace mozilla

image/src/FrameSequence.h

@@ -0,0 +1,185 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_imagelib_FrameSequence_h_
+#define mozilla_imagelib_FrameSequence_h_
+
+#include "nsTArray.h"
+#include "mozilla/MemoryReporting.h"
+#include "gfxASurface.h"
+#include "imgFrame.h"
+
+namespace mozilla {
+namespace image {
+
+/**
+ * FrameDataPair is a slightly-smart tuple of (frame, raw frame data) where the
+ * raw frame data is allowed to be (and is, initially) null.
+ *
+ * If you call LockAndGetData, you will be able to call GetFrameData() on that
+ * instance, and when the FrameDataPair is destructed, the imgFrame lock will
+ * be unlocked.
+ */
+class FrameDataPair
+{
+public:
+  explicit FrameDataPair(imgFrame* frame)
+    : mFrame(frame)
+    , mFrameData(nullptr)
+  {}
+
+  FrameDataPair()
+    : mFrame(nullptr)
+    , mFrameData(nullptr)
+  {}
+
+  FrameDataPair(FrameDataPair& other)
+  {
+    mFrame = other.mFrame;
+    mFrameData = other.mFrameData;
+
+    // since mFrame is an nsAutoPtr, the assignment operator above actually
+    // nulls out other.mFrame. In order to fully assume ownership over the
+    // frame, we also null out the other's mFrameData.
+    other.mFrameData = nullptr;
+  }
+
+  ~FrameDataPair()
+  {
+    if (mFrameData) {
+      mFrame->UnlockImageData();
+    }
+  }
+
+  // Lock the frame and store its mFrameData. The frame will be unlocked (and
+  // deleted) when this FrameDataPair is deleted.
+  void LockAndGetData()
+  {
+    if (mFrame) {
+      if (NS_SUCCEEDED(mFrame->LockImageData())) {
+        if (mFrame->GetIsPaletted()) {
+          mFrameData = reinterpret_cast<uint8_t*>(mFrame->GetPaletteData());
+        } else {
+          mFrameData = mFrame->GetImageData();
+        }
+      }
+    }
+  }
+
+  // Null out this FrameDataPair and return its frame. You must ensure the
+  // frame will be deleted separately.
+  imgFrame* Forget()
+  {
+    if (mFrameData) {
+      mFrame->UnlockImageData();
+    }
+
+    imgFrame* frame = mFrame.forget();
+    mFrameData = nullptr;
+    return frame;
+  }
+
+  bool HasFrameData() const
+  {
+    if (mFrameData) {
+      MOZ_ASSERT(!!mFrame);
+    }
+    return !!mFrameData;
+  }
+
+  uint8_t* GetFrameData() const
+  {
+    return mFrameData;
+  }
+
+  imgFrame* GetFrame() const
+  {
+    return mFrame;
+  }
+
+  // Resets this FrameDataPair to work with a different frame. Takes ownership
+  // of the frame, deleting the old frame (if any).
+  void SetFrame(imgFrame* frame)
+  {
+    if (mFrameData) {
+      mFrame->UnlockImageData();
+    }
+
+    mFrame = frame;
+    mFrameData = nullptr;
+  }
+
+  operator imgFrame*() const
+  {
+    return GetFrame();
+  }
+
+  imgFrame* operator->() const
+  {
+    return GetFrame();
+  }
+
+  bool operator==(imgFrame* other) const
+  {
+    return mFrame == other;
+  }
+
+private:
+  nsAutoPtr<imgFrame> mFrame;
+  uint8_t* mFrameData;
+};
+
+/**
+ * FrameSequence stores image frames (and their associated raw data pointers).
+ * It is little more than a smart array.
+ */
+class FrameSequence
+{
+public:
+
+  ~FrameSequence();
+
+  /**
+   * Get the read-only (frame, data) pair at index aIndex.
+   */
+  const FrameDataPair& GetFrame(uint32_t aIndex) const;
+
+  /**
+   * Insert a frame into the array. FrameSequence takes ownership of the frame.
+   */
+  void InsertFrame(uint32_t framenum, imgFrame* aFrame);
+
+  /**
+   * Remove (and delete) the frame at index framenum.
+   */
+  void RemoveFrame(uint32_t framenum);
+
+  /**
+   * Swap aFrame with the frame at sequence framenum, and return that frame.
+   * You take ownership over the frame returned.
+   */
+  imgFrame* SwapFrame(uint32_t framenum, imgFrame* aFrame);
+
+  /**
+   * Remove (and delete) all frames.
+   */
+  void ClearFrames();
+
+  /* The total number of frames in this image. */
+  uint32_t GetNumFrames() const;
+
+  size_t SizeOfDecodedWithComputedFallbackIfHeap(gfxASurface::MemoryLocation aLocation,
+                                                 mozilla::MallocSizeOf aMallocSizeOf) const;
+
+private: // data
+  //! All the frames of the image
+  nsTArray<FrameDataPair> mFrames;
+};
+
+} // namespace image
+} // namespace mozilla
+
+#endif /* mozilla_imagelib_FrameSequence_h_ */

image/src/moz.build

@@ -18,6 +18,7 @@ CPP_SOURCES += [
     'Decoder.cpp',
     'DiscardTracker.cpp',
     'FrameBlender.cpp',
+    'FrameSequence.cpp',
     'FrozenImage.cpp',
     'Image.cpp',
     'ImageFactory.cpp',