Attachment 516418 Details for Bug 580531

[patch] Patch: Push ImageContainer subclass' locks up into a superclass monitor.

580531-layers-monitor.patch (text/plain), 14.52 KB, created by Chris Pearce [:cpearce (Not reading bugmail)]

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Pearce [:cpearce (Not reading bugmail)]

Size: 14.52 KB

patch

obsolete

># HG changeset patch
># User Chris Pearce <chris@pearce.org.nz>
># Parent c793b4f8731893bc5b3e9a36a15ff4aec2cc43c6
>Bug 580531 - Push ImageContainer subclass' locks up into a superclass monitor. r=?
>
>diff --git a/gfx/layers/ImageLayers.h b/gfx/layers/ImageLayers.h
>--- a/gfx/layers/ImageLayers.h
>+++ b/gfx/layers/ImageLayers.h
>@@ -38,16 +38,17 @@
> #ifndef GFX_IMAGELAYER_H
> #define GFX_IMAGELAYER_H
> 
> #include "Layers.h"
> 
> #include "gfxPattern.h"
> #include "nsThreadUtils.h"
> #include "nsCoreAnimationSupport.h"
>+#include "mozilla/Monitor.h"
> 
> namespace mozilla {
> namespace layers {
> 
> enum StereoMode {
>   STEREO_MODE_MONO,
>   STEREO_MODE_LEFT_RIGHT,
>   STEREO_MODE_RIGHT_LEFT,
>@@ -126,17 +127,17 @@ protected:
>  * (because layers can only be used on the main thread) and we want to
>  * be able to set the current Image from any thread, to facilitate
>  * video playback without involving the main thread, for example.
>  */
> class THEBES_API ImageContainer {
>   THEBES_INLINE_DECL_THREADSAFE_REFCOUNTING(ImageContainer)
> 
> public:
>-  ImageContainer() {}
>+  ImageContainer() : mMonitor("ImageContainer") {}
>   virtual ~ImageContainer() {}
> 
>   /**
>    * Create an Image in one of the given formats.
>    * Picks the "best" format from the list and creates an Image of that
>    * format.
>    * Returns null if this backend does not support any of the formats.
>    */
>@@ -207,19 +208,21 @@ public:
>   /**
>    * Get the layer manager type this image container was created with,
>    * presumably its users might want to do something special if types do not
>    * match.
>    */
>   virtual LayerManager::LayersBackend GetBackendType() = 0;
> 
> protected:
>+  typedef mozilla::Monitor Monitor;
>   LayerManager* mManager;
>+  Monitor mMonitor;
> 
>-  ImageContainer(LayerManager* aManager) : mManager(aManager) {}
>+  ImageContainer(LayerManager* aManager) : mManager(aManager), mMonitor("ImageContainer")  {}
> };
> 
> /**
>  * A Layer which renders an Image.
>  */
> class THEBES_API ImageLayer : public Layer {
> public:
>   /**
>diff --git a/gfx/layers/basic/BasicImages.cpp b/gfx/layers/basic/BasicImages.cpp
>--- a/gfx/layers/basic/BasicImages.cpp
>+++ b/gfx/layers/basic/BasicImages.cpp
>@@ -287,33 +287,32 @@ BasicPlanarYCbCrImage::GetAsSurface()
>  * for the image objects. We use a Monitor to synchronize access to
>  * mImage.
>  */
> class BasicImageContainer : public ImageContainer {
> public:
>   typedef gfxASurface::gfxImageFormat gfxImageFormat;
> 
>   BasicImageContainer() :
>-    ImageContainer(nsnull), mMonitor("BasicImageContainer"),
>+    ImageContainer(nsnull),
>     mScaleHint(-1, -1),
>     mOffscreenFormat(gfxASurface::ImageFormatUnknown)
>   {}
>   virtual already_AddRefed<Image> CreateImage(const Image::Format* aFormats,
>                                               PRUint32 aNumFormats);
>   virtual void SetCurrentImage(Image* aImage);
>   virtual already_AddRefed<Image> GetCurrentImage();
>   virtual already_AddRefed<gfxASurface> GetCurrentAsSurface(gfxIntSize* aSize);
>   virtual gfxIntSize GetCurrentSize();
>   virtual PRBool SetLayerManager(LayerManager *aManager);
>   virtual void SetScaleHint(const gfxIntSize& aScaleHint);
>   void SetOffscreenFormat(gfxImageFormat aFormat) { mOffscreenFormat = aFormat; }
>   virtual LayerManager::LayersBackend GetBackendType() { return LayerManager::LAYERS_BASIC; }
> 
> protected:
>-  Monitor mMonitor;
>   nsRefPtr<Image> mImage;
>   gfxIntSize mScaleHint;
>   gfxImageFormat mOffscreenFormat;
> };
> 
> /**
>  * Returns true if aFormat is in the given format array.
>  */
>diff --git a/gfx/layers/d3d10/ImageLayerD3D10.cpp b/gfx/layers/d3d10/ImageLayerD3D10.cpp
>--- a/gfx/layers/d3d10/ImageLayerD3D10.cpp
>+++ b/gfx/layers/d3d10/ImageLayerD3D10.cpp
>@@ -40,18 +40,16 @@
> #include "gfxD2DSurface.h"
> #include "gfxWindowsSurface.h"
> #include "yuv_convert.h"
> #include "../d3d9/Nv3DVUtils.h"
> 
> namespace mozilla {
> namespace layers {
> 
>-using mozilla::MutexAutoLock;
>-
> static already_AddRefed<ID3D10Texture2D>
> SurfaceToTexture(ID3D10Device *aDevice,
>                  gfxASurface *aSurface,
>                  const gfxIntSize &aSize)
> {
>   if (aSurface && aSurface->GetType() == gfxASurface::SurfaceTypeD2D) {
>     void *data = aSurface->GetData(&gKeyD3D10Texture);
>     if (data) {
>@@ -96,17 +94,16 @@ SurfaceToTexture(ID3D10Device *aDevice,
>   }
> 
>   return texture.forget();
> }
> 
> ImageContainerD3D10::ImageContainerD3D10(ID3D10Device1 *aDevice)
>   : ImageContainer(nsnull)
>   , mDevice(aDevice)
>-  , mActiveImageLock("mozilla.layers.ImageContainerD3D10.mActiveImageLock")
> {
> }
> 
> already_AddRefed<Image>
> ImageContainerD3D10::CreateImage(const Image::Format *aFormats,
>                                  PRUint32 aNumFormats)
> {
>   if (!aNumFormats) {
>@@ -119,34 +116,34 @@ ImageContainerD3D10::CreateImage(const I
>     img = new CairoImageD3D10(mDevice);
>   }
>   return img.forget();
> }
> 
> void
> ImageContainerD3D10::SetCurrentImage(Image *aImage)
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
> 
>   mActiveImage = aImage;
> }
> 
> already_AddRefed<Image>
> ImageContainerD3D10::GetCurrentImage()
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
> 
>   nsRefPtr<Image> retval = mActiveImage;
>   return retval.forget();
> }
> 
> already_AddRefed<gfxASurface>
> ImageContainerD3D10::GetCurrentAsSurface(gfxIntSize *aSize)
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
>   if (!mActiveImage) {
>     return nsnull;
>   }
> 
>   if (mActiveImage->GetFormat() == Image::PLANAR_YCBCR) {
>     PlanarYCbCrImageD3D10 *yuvImage =
>       static_cast<PlanarYCbCrImageD3D10*>(mActiveImage.get());
>     if (yuvImage->HasData()) {
>@@ -159,17 +156,17 @@ ImageContainerD3D10::GetCurrentAsSurface
>   }
> 
>   return static_cast<ImageD3D10*>(mActiveImage->GetImplData())->GetAsSurface();
> }
> 
> gfxIntSize
> ImageContainerD3D10::GetCurrentSize()
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
>   if (!mActiveImage) {
>     return gfxIntSize(0,0);
>   }
>   if (mActiveImage->GetFormat() == Image::PLANAR_YCBCR) {
>     PlanarYCbCrImageD3D10 *yuvImage =
>       static_cast<PlanarYCbCrImageD3D10*>(mActiveImage.get());
>     if (!yuvImage->HasData()) {
>       return gfxIntSize(0,0);
>diff --git a/gfx/layers/d3d10/ImageLayerD3D10.h b/gfx/layers/d3d10/ImageLayerD3D10.h
>--- a/gfx/layers/d3d10/ImageLayerD3D10.h
>+++ b/gfx/layers/d3d10/ImageLayerD3D10.h
>@@ -36,17 +36,16 @@
>  * ***** END LICENSE BLOCK ***** */
> 
> #ifndef GFX_IMAGELAYERD3D10_H
> #define GFX_IMAGELAYERD3D10_H
> 
> #include "LayerManagerD3D10.h"
> #include "ImageLayers.h"
> #include "yuv_convert.h"
>-#include "mozilla/Mutex.h"
> 
> namespace mozilla {
> namespace layers {
> 
> class THEBES_API ImageContainerD3D10 : public ImageContainer
> {
> public:
>   ImageContainerD3D10(ID3D10Device1 *aDevice);
>@@ -66,22 +65,18 @@ public:
>   virtual PRBool SetLayerManager(LayerManager *aManager);
> 
>   virtual LayerManager::LayersBackend GetBackendType() { return LayerManager::LAYERS_D3D10; }
> 
>   ID3D10Device1 *device() { return mDevice; }
>   void SetDevice(ID3D10Device1 *aDevice) { mDevice = aDevice; }
> 
> private:
>-  typedef mozilla::Mutex Mutex;
>-
>   nsRefPtr<Image> mActiveImage;
>   nsRefPtr<ID3D10Device1> mDevice;
>-
>-  Mutex mActiveImageLock;
> };
> 
> class THEBES_API ImageLayerD3D10 : public ImageLayer,
>                                    public LayerD3D10
> {
> public:
>   ImageLayerD3D10(LayerManagerD3D10 *aManager)
>     : ImageLayer(aManager, NULL)
>diff --git a/gfx/layers/d3d9/ImageLayerD3D9.cpp b/gfx/layers/d3d9/ImageLayerD3D9.cpp
>--- a/gfx/layers/d3d9/ImageLayerD3D9.cpp
>+++ b/gfx/layers/d3d9/ImageLayerD3D9.cpp
>@@ -41,18 +41,16 @@
> #include "nsIServiceManager.h" 
> #include "nsIConsoleService.h" 
> #include "nsPrintfCString.h" 
> #include "Nv3DVUtils.h"
> 
> namespace mozilla {
> namespace layers {
> 
>-using mozilla::MutexAutoLock;
>-
> static already_AddRefed<IDirect3DTexture9>
> SurfaceToTexture(IDirect3DDevice9 *aDevice,
>                  gfxASurface *aSurface,
>                  const gfxIntSize &aSize)
> {
> 
>   nsRefPtr<gfxImageSurface> imageSurface = aSurface->GetAsImageSurface();
> 
>@@ -131,17 +129,16 @@ SurfaceToTexture(IDirect3DDevice9 *aDevi
>   }
> 
>   return texture.forget();
> }
> 
> ImageContainerD3D9::ImageContainerD3D9(IDirect3DDevice9 *aDevice)
>   : ImageContainer(nsnull)
>   , mDevice(aDevice)
>-  , mActiveImageLock("mozilla.layers.ImageContainerD3D9.mActiveImageLock")
> {
> }
> 
> already_AddRefed<Image>
> ImageContainerD3D9::CreateImage(const Image::Format *aFormats,
>                                PRUint32 aNumFormats)
> {
>   if (!aNumFormats) {
>@@ -154,34 +151,34 @@ ImageContainerD3D9::CreateImage(const Im
>     img = new CairoImageD3D9(mDevice);
>   }
>   return img.forget();
> }
> 
> void
> ImageContainerD3D9::SetCurrentImage(Image *aImage)
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
> 
>   mActiveImage = aImage;
> }
> 
> already_AddRefed<Image>
> ImageContainerD3D9::GetCurrentImage()
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
> 
>   nsRefPtr<Image> retval = mActiveImage;
>   return retval.forget();
> }
> 
> already_AddRefed<gfxASurface>
> ImageContainerD3D9::GetCurrentAsSurface(gfxIntSize *aSize)
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
>   if (!mActiveImage) {
>     return nsnull;
>   }
> 
>   if (mActiveImage->GetFormat() == Image::PLANAR_YCBCR) {
>     PlanarYCbCrImageD3D9 *yuvImage =
>       static_cast<PlanarYCbCrImageD3D9*>(mActiveImage.get());
>     if (yuvImage->HasData()) {
>@@ -194,17 +191,17 @@ ImageContainerD3D9::GetCurrentAsSurface(
>   }
> 
>   return static_cast<ImageD3D9*>(mActiveImage->GetImplData())->GetAsSurface();
> }
> 
> gfxIntSize
> ImageContainerD3D9::GetCurrentSize()
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
>   if (!mActiveImage) {
>     return gfxIntSize(0,0);
>   }
>   if (mActiveImage->GetFormat() == Image::PLANAR_YCBCR) {
>     PlanarYCbCrImageD3D9 *yuvImage =
>       static_cast<PlanarYCbCrImageD3D9*>(mActiveImage.get());
>     if (!yuvImage->HasData()) {
>       return gfxIntSize(0,0);
>diff --git a/gfx/layers/d3d9/ImageLayerD3D9.h b/gfx/layers/d3d9/ImageLayerD3D9.h
>--- a/gfx/layers/d3d9/ImageLayerD3D9.h
>+++ b/gfx/layers/d3d9/ImageLayerD3D9.h
>@@ -36,17 +36,16 @@
>  * ***** END LICENSE BLOCK ***** */
> 
> #ifndef GFX_IMAGELAYERD3D9_H
> #define GFX_IMAGELAYERD3D9_H
> 
> #include "LayerManagerD3D9.h"
> #include "ImageLayers.h"
> #include "yuv_convert.h"
>-#include "mozilla/Mutex.h"
> 
> namespace mozilla {
> namespace layers {
> 
> class THEBES_API ImageContainerD3D9 : public ImageContainer
> {
> public:
>   ImageContainerD3D9(IDirect3DDevice9 *aDevice);
>@@ -66,23 +65,19 @@ public:
>   virtual PRBool SetLayerManager(LayerManager *aManager);
> 
>   virtual LayerManager::LayersBackend GetBackendType() { return LayerManager::LAYERS_D3D9; }
> 
>   IDirect3DDevice9 *device() { return mDevice; }
>   void SetDevice(IDirect3DDevice9 *aDevice) { mDevice = aDevice; }
> 
> private:
>-  typedef mozilla::Mutex Mutex;
>-
>   nsRefPtr<Image> mActiveImage;
> 
>   nsRefPtr<IDirect3DDevice9> mDevice;
>-
>-  Mutex mActiveImageLock;
> };
> 
> class THEBES_API ImageLayerD3D9 : public ImageLayer,
>                                   public LayerD3D9
> {
> public:
>   ImageLayerD3D9(LayerManagerD3D9 *aManager)
>     : ImageLayer(aManager, NULL)
>diff --git a/gfx/layers/opengl/ImageLayerOGL.cpp b/gfx/layers/opengl/ImageLayerOGL.cpp
>--- a/gfx/layers/opengl/ImageLayerOGL.cpp
>+++ b/gfx/layers/opengl/ImageLayerOGL.cpp
>@@ -46,18 +46,16 @@
> #include "GLContextProvider.h"
> #include "MacIOSurfaceImageOGL.h"
> 
> using namespace mozilla::gl;
> 
> namespace mozilla {
> namespace layers {
> 
>-using mozilla::MutexAutoLock;
>-
> /**
>  * This is an event used to unref a GLContext on the main thread and
>  * optionally delete a texture associated with that context.
>  */
> class TextureDeleter : public nsRunnable {
> public:
>   TextureDeleter(already_AddRefed<GLContext> aContext,
>                  GLuint aTexture)
>@@ -187,17 +185,16 @@ RecycleBin::GetTexture(TextureType aType
>   PRUint32 last = mRecycledTextures[aType].Length() - 1;
>   aOutTexture->TakeFrom(&mRecycledTextures[aType].ElementAt(last));
>   mRecycledTextures[aType].RemoveElementAt(last);
> }
> 
> ImageContainerOGL::ImageContainerOGL(LayerManagerOGL *aManager)
>   : ImageContainer(aManager)
>   , mRecycleBin(new RecycleBin())
>-  , mActiveImageLock("mozilla.layers.ImageContainerOGL.mActiveImageLock")
> {
> }
> 
> ImageContainerOGL::~ImageContainerOGL()
> {
>   if (mManager) {
>     NS_ASSERTION(mManager->GetBackendType() == LayerManager::LAYERS_OPENGL, "Wrong layer manager got assigned to ImageContainerOGL!");
> 
>@@ -229,39 +226,39 @@ ImageContainerOGL::CreateImage(const Ima
> }
> 
> void
> ImageContainerOGL::SetCurrentImage(Image *aImage)
> {
>   nsRefPtr<Image> oldImage;
> 
>   {
>-    MutexAutoLock lock(mActiveImageLock);
>+    MonitorAutoEnter mon(mMonitor);
> 
>     oldImage = mActiveImage.forget();
>     mActiveImage = aImage;
>   }
> 
>   // Make sure oldImage is released outside the lock, so it can take our
>   // lock in RecycleBuffer
> }
> 
> already_AddRefed<Image>
> ImageContainerOGL::GetCurrentImage()
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
> 
>   nsRefPtr<Image> retval = mActiveImage;
>   return retval.forget();
> }
> 
> already_AddRefed<gfxASurface>
> ImageContainerOGL::GetCurrentAsSurface(gfxIntSize *aSize)
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
> 
>   if (!mActiveImage) {
>     *aSize = gfxIntSize(0,0);
>     return nsnull;
>   }
> 
>   GLContext *gl = nsnull;
>   // tex1 will be RGBA or Y, tex2 will Cb, tex3 will be Cr
>@@ -309,17 +306,17 @@ ImageContainerOGL::GetCurrentAsSurface(g
>   nsRefPtr<gfxImageSurface> s = gl->ReadTextureImage(tex1, size, LOCAL_GL_RGBA);
>   *aSize = size;
>   return s.forget();
> }
> 
> gfxIntSize
> ImageContainerOGL::GetCurrentSize()
> {
>-  MutexAutoLock lock(mActiveImageLock);
>+  MonitorAutoEnter mon(mMonitor);
>   if (!mActiveImage) {
>     return gfxIntSize(0,0);
>   }
> 
>   if (mActiveImage->GetFormat() == Image::PLANAR_YCBCR) {
>     PlanarYCbCrImageOGL *yuvImage =
>       static_cast<PlanarYCbCrImageOGL*>(mActiveImage.get());
>     if (!yuvImage->HasData()) {
>diff --git a/gfx/layers/opengl/ImageLayerOGL.h b/gfx/layers/opengl/ImageLayerOGL.h
>--- a/gfx/layers/opengl/ImageLayerOGL.h
>+++ b/gfx/layers/opengl/ImageLayerOGL.h
>@@ -154,23 +154,18 @@ public:
> 
>   virtual gfxIntSize GetCurrentSize();
> 
>   virtual PRBool SetLayerManager(LayerManager *aManager);
> 
>   virtual LayerManager::LayersBackend GetBackendType() { return LayerManager::LAYERS_OPENGL; }
> 
> private:
>-  typedef mozilla::Mutex Mutex;
> 
>   nsRefPtr<RecycleBin> mRecycleBin;
>-
>-  // This protects mActiveImage
>-  Mutex mActiveImageLock;
>-
>   nsRefPtr<Image> mActiveImage;
> };
> 
> class THEBES_API ImageLayerOGL : public ImageLayer,
>                                  public LayerOGL
> {
> public:
>   ImageLayerOGL(LayerManagerOGL *aManager)

Flags:

roc: review+

Actions: View | Diff | Review

Attachments on bug 580531: 458935 | 458937 | 465555 | 465556 | 468611 | 468612 | 468617 | 495942 | 496253 | 496254 | 496258 | 514667 | 515506 | 515799 | 516082 | 516083 | 516105 | 516139 | 516418 | 516419