Bug 1143575. Let callers of ImageContainer::SetCurrentImages specify frame IDs. r=nical

dom/media/VideoFrameContainer.cpp

@@ -20,6 +20,7 @@ VideoFrameContainer::VideoFrameContainer(dom::HTMLMediaElement* aElement,
                                          already_AddRefed<ImageContainer> aContainer)
   : mElement(aElement),
     mImageContainer(aContainer), mMutex("nsVideoFrameContainer"),
+    mFrameID(0),
     mIntrinsicSizeChanged(false), mImageSizeChanged(false)
 {
   NS_ASSERTION(aElement, "aElement must not be null");
@@ -51,10 +52,12 @@ void VideoFrameContainer::SetCurrentFrame(const gfxIntSize& aIntrinsicSize,
   nsTArray<ImageContainer::OwningImage> kungFuDeathGrip;
   mImageContainer->GetCurrentImages(&kungFuDeathGrip);
 
+  ++mFrameID;
+
   if (aImage) {
     nsAutoTArray<ImageContainer::NonOwningImage,1> imageList;
     imageList.AppendElement(
-        ImageContainer::NonOwningImage(aImage, aTargetTime));
+        ImageContainer::NonOwningImage(aImage, aTargetTime, mFrameID));
     mImageContainer->SetCurrentImages(imageList);
   } else {
     mImageContainer->ClearAllImages();

dom/media/VideoFrameContainer.h

@@ -12,6 +12,7 @@
 #include "gfxPoint.h"
 #include "nsCOMPtr.h"
 #include "nsAutoPtr.h"
+#include "ImageContainer.h"
 
 namespace mozilla {
 
@@ -77,6 +78,9 @@ protected:
   // specifies that the Image should be stretched to have the correct aspect
   // ratio.
   gfxIntSize mIntrinsicSize;
+  // For SetCurrentFrame callers we maintain our own mFrameID which is auto-
+  // incremented at every SetCurrentFrame.
+  ImageContainer::FrameID mFrameID;
   // True when the intrinsic size has been changed by SetCurrentFrame() since
   // the last call to Invalidate().
   // The next call to Invalidate() will recalculate

dom/plugins/ipc/PluginInstanceParent.cpp

@@ -635,7 +635,7 @@ PluginInstanceParent::RecvShow(const NPRect& updatedRect,
 
         nsAutoTArray<ImageContainer::NonOwningImage,1> imageList;
         imageList.AppendElement(
-            ImageContainer::NonOwningImage(image, TimeStamp()));
+            ImageContainer::NonOwningImage(image));
         container->SetCurrentImages(imageList);
     }
     else if (mImageContainer) {

gfx/layers/ImageContainer.cpp

@@ -171,6 +171,7 @@ ImageContainer::ImageContainer(Mode flag)
   mGenerationCounter(++sGenerationCounter),
   mPaintCount(0),
   mDroppedImageCount(0),
+  mCurrentImageFrameID(0),
   mCurrentImageComposited(false),
   mImageFactory(new ImageFactory()),
   mRecycleBin(new BufferRecycleBin()),
@@ -236,16 +237,18 @@ ImageContainer::CreateImage(ImageFormat aFormat)
 
 void
 ImageContainer::SetCurrentImageInternal(Image *aImage,
-                                        const TimeStamp& aTimeStamp)
+                                        const TimeStamp& aTimeStamp,
+                                        FrameID aFrameID)
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
 
-  if (mActiveImage != aImage) {
+  if (mActiveImage != aImage || aFrameID != mCurrentImageFrameID) {
     if (!mCurrentImageComposited && !mCurrentImageTimeStamp.IsNull() &&
         (aTimeStamp.IsNull() || aTimeStamp > mCurrentImageTimeStamp)) {
-      mFrameIDsNotYetComposited.AppendElement(mGenerationCounter);
+      mFrameIDsNotYetComposited.AppendElement(mCurrentImageFrameID);
     }
     mGenerationCounter = ++sGenerationCounter;
+    mCurrentImageFrameID = aFrameID;
     mCurrentImageComposited = false;
     mActiveImage = aImage;
     mCurrentImageTimeStamp = aTimeStamp;
@@ -256,7 +259,7 @@ void
 ImageContainer::ClearImagesFromImageBridge()
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
-  SetCurrentImageInternal(nullptr, TimeStamp());
+  SetCurrentImageInternal(nullptr, TimeStamp(), 0);
 }
 
 void
@@ -268,7 +271,8 @@ ImageContainer::SetCurrentImages(const nsTArray<NonOwningImage>& aImages)
     ImageBridgeChild::DispatchImageClientUpdate(mImageClient, this);
   }
   MOZ_ASSERT(aImages.Length() == 1);
-  SetCurrentImageInternal(aImages[0].mImage, aImages[0].mTimeStamp);
+  SetCurrentImageInternal(aImages[0].mImage, aImages[0].mTimeStamp,
+                          aImages[0].mFrameID);
 }
 
  void
@@ -282,7 +286,7 @@ ImageContainer::ClearAllImages()
   }
 
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
-  SetCurrentImageInternal(nullptr, TimeStamp());
+  SetCurrentImageInternal(nullptr, TimeStamp(), 0);
 }
 
 void
@@ -291,7 +295,7 @@ ImageContainer::SetCurrentImageInTransaction(Image *aImage)
   NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
   NS_ASSERTION(!mImageClient, "Should use async image transfer with ImageBridge.");
 
-  SetCurrentImageInternal(aImage, TimeStamp());
+  SetCurrentImageInternal(aImage, TimeStamp(), 0);
 }
 
 bool ImageContainer::IsAsync() const
@@ -326,7 +330,7 @@ ImageContainer::GetCurrentImages(nsTArray<OwningImage>* aImages,
   if (mActiveImage) {
     OwningImage* img = aImages->AppendElement();
     img->mImage = mActiveImage;
-    img->mFrameID = mGenerationCounter;
+    img->mFrameID = mCurrentImageFrameID;
     img->mProducerID = 0;
   }
   if (aGenerationCounter) {
@@ -366,7 +370,7 @@ ImageContainer::NotifyCompositeInternal(const ImageCompositeNotification& aNotif
       break;
     }
   }
-  if (aNotification.frameID() == mGenerationCounter) {
+  if (aNotification.frameID() == mCurrentImageFrameID) {
     mCurrentImageComposited = true;
   }
 

gfx/layers/ImageContainer.h

@@ -303,10 +303,13 @@ public:
   B2G_ACL_EXPORT already_AddRefed<Image> CreateImage(ImageFormat aFormat);
 
   struct NonOwningImage {
-    NonOwningImage(Image* aImage, TimeStamp aTimeStamp)
-      : mImage(aImage), mTimeStamp(aTimeStamp) {}
+    explicit NonOwningImage(Image* aImage = nullptr,
+                            TimeStamp aTimeStamp = TimeStamp(),
+                            FrameID aFrameID = 0)
+      : mImage(aImage), mTimeStamp(aTimeStamp), mFrameID(aFrameID) {}
     Image* mImage;
     TimeStamp mTimeStamp;
+    FrameID mFrameID;
   };
   /**
    * Set aImages as the list of timestamped to display. The Images must have
@@ -316,6 +319,10 @@ public:
    * aImages must be non-empty. The first timestamp in the list may be
    * null but the others must not be, and the timestamps must increase.
    * Every element of aImages must have non-null mImage.
+   * mFrameID can be zero, in which case you won't get meaningful
+   * painted/dropped frame counts. Otherwise you should use a unique and
+   * increasing ID for each decoded and submitted frame (but it's OK to
+   * pass the same frame to SetCurrentImages).
    * 
    * The Image data must not be modified after this method is called!
    * Note that this must not be called if ENABLE_ASYNC has not been set.
@@ -354,8 +361,7 @@ public:
    * The Image data must not be modified after this method is called!
    * Note that this must not be called if ENABLE_ASYNC been set.
    *
-   * Implementations must call CurrentImageChanged() while holding
-   * mReentrantMonitor.
+   * You won't get meaningful painted/dropped counts when using this method.
    */
   void SetCurrentImageInTransaction(Image* aImage);
 
@@ -475,7 +481,8 @@ private:
   // Private destructor, to discourage deletion outside of Release():
   B2G_ACL_EXPORT ~ImageContainer();
 
-  void SetCurrentImageInternal(Image* aImage, const TimeStamp& aTimeStamp);
+  void SetCurrentImageInternal(Image* aImage, const TimeStamp& aTimeStamp,
+                               FrameID aFrameID);
 
   // This is called to ensure we have an active image, this may not be true
   // when we're storing image information in a RemoteImageData structure.
@@ -506,6 +513,7 @@ private:
   // See GetDroppedImageCount. Accessed only with mReentrantMonitor held.
   uint32_t mDroppedImageCount;
 
+  FrameID mCurrentImageFrameID;
   bool mCurrentImageComposited;
 
   // This is the image factory used by this container, layer managers using

image/RasterImage.cpp

@@ -907,7 +907,7 @@ RasterImage::UpdateImageContainer()
   mLastImageContainerDrawResult = result.first();
   nsAutoTArray<ImageContainer::NonOwningImage,1> imageList;
   imageList.AppendElement(
-      ImageContainer::NonOwningImage(result.second(), TimeStamp()));
+      ImageContainer::NonOwningImage(result.second()));
   container->SetCurrentImages(imageList);
 }