Bug 840841 - Remove mObserver from RasterImage and VectorImage. r=joe
authorSeth Fowler <seth@mozilla.com>
Tue, 12 Feb 2013 19:00:03 -0800
changeset 131930 78d86cf0671be8416574174edcc0cd4c57fb6312
parent 131929 b3bdec87d9635451601869f77b0ceba400d6515f
child 131931 c68f8131db6a0232140561c3d45f20913f540bea
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoe
bugs840841
milestone21.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 840841 - Remove mObserver from RasterImage and VectorImage. r=joe
image/src/Image.h
image/src/ImageFactory.cpp
image/src/RasterImage.cpp
image/src/RasterImage.h
image/src/VectorImage.cpp
image/src/VectorImage.h
image/src/imgDecoderObserver.h
--- a/image/src/Image.h
+++ b/image/src/Image.h
@@ -49,22 +49,20 @@ public:
   static const uint32_t INIT_FLAG_NONE           = 0x0;
   static const uint32_t INIT_FLAG_DISCARDABLE    = 0x1;
   static const uint32_t INIT_FLAG_DECODE_ON_DRAW = 0x2;
   static const uint32_t INIT_FLAG_MULTIPART      = 0x4;
 
   /**
    * Creates a new image container.
    *
-   * @param aObserver Observer to send decoder and animation notifications to.
    * @param aMimeType The mimetype of the image.
    * @param aFlags Initialization flags of the INIT_FLAG_* variety.
    */
-  virtual nsresult Init(imgDecoderObserver* aObserver,
-                        const char* aMimeType,
+  virtual nsresult Init(const char* aMimeType,
                         uint32_t aFlags) = 0;
 
   virtual imgStatusTracker& GetStatusTracker() = 0;
 
   /**
    * The rectangle defining the location and size of the given frame.
    */
   virtual nsIntRect FrameRect(uint32_t aWhichFrame) = 0;
--- a/image/src/ImageFactory.cpp
+++ b/image/src/ImageFactory.cpp
@@ -117,17 +117,17 @@ BadImage(nsRefPtr<T>& image)
 
 /* static */ already_AddRefed<Image>
 ImageFactory::CreateAnonymousImage(const nsCString& aMimeType)
 {
   nsresult rv;
 
   nsRefPtr<RasterImage> newImage = new RasterImage();
 
-  rv = newImage->Init(nullptr, aMimeType.get(), Image::INIT_FLAG_NONE);
+  rv = newImage->Init(aMimeType.get(), Image::INIT_FLAG_NONE);
   NS_ENSURE_SUCCESS(rv, BadImage(newImage));
 
   return newImage.forget();
 }
 
 /* static */ already_AddRefed<Image>
 ImageFactory::CreateRasterImage(nsIRequest* aRequest,
                                 imgStatusTracker* aStatusTracker,
@@ -135,18 +135,17 @@ ImageFactory::CreateRasterImage(nsIReque
                                 nsIURI* aURI,
                                 uint32_t aImageFlags,
                                 uint32_t aInnerWindowId)
 {
   nsresult rv;
 
   nsRefPtr<RasterImage> newImage = new RasterImage(aStatusTracker, aURI);
 
-  rv = newImage->Init(aStatusTracker->GetDecoderObserver(),
-                      aMimeType.get(), aImageFlags);
+  rv = newImage->Init(aMimeType.get(), aImageFlags);
   NS_ENSURE_SUCCESS(rv, BadImage(newImage));
 
   newImage->SetInnerWindowID(aInnerWindowId);
 
   // Use content-length as a size hint for http channels.
   nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(aRequest));
   if (httpChannel) {
     nsAutoCString contentLength;
@@ -184,18 +183,17 @@ ImageFactory::CreateVectorImage(nsIReque
                                 nsIURI* aURI,
                                 uint32_t aImageFlags,
                                 uint32_t aInnerWindowId)
 {
   nsresult rv;
 
   nsRefPtr<VectorImage> newImage = new VectorImage(aStatusTracker, aURI);
 
-  rv = newImage->Init(aStatusTracker->GetDecoderObserver(),
-                      aMimeType.get(), aImageFlags);
+  rv = newImage->Init(aMimeType.get(), aImageFlags);
   NS_ENSURE_SUCCESS(rv, BadImage(newImage));
 
   newImage->SetInnerWindowID(aInnerWindowId);
 
   rv = newImage->OnStartRequest(aRequest, nullptr);
   NS_ENSURE_SUCCESS(rv, BadImage(newImage));
 
   return newImage.forget();
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -448,18 +448,17 @@ RasterImage::Initialize()
   InitPrefCaches();
 
   // Create our singletons now, so we don't have to worry about what thread
   // they're created on.
   DecodeWorker::Singleton();
 }
 
 nsresult
-RasterImage::Init(imgDecoderObserver *aObserver,
-                  const char* aMimeType,
+RasterImage::Init(const char* aMimeType,
                   uint32_t aFlags)
 {
   // We don't support re-initialization
   if (mInitialized)
     return NS_ERROR_ILLEGAL_VALUE;
 
   // Not sure an error can happen before init, but be safe
   if (mError)
@@ -470,19 +469,16 @@ RasterImage::Init(imgDecoderObserver *aO
   // We must be non-discardable and non-decode-on-draw for
   // multipart channels
   NS_ABORT_IF_FALSE(!(aFlags & INIT_FLAG_MULTIPART) ||
                     (!(aFlags & INIT_FLAG_DISCARDABLE) &&
                      !(aFlags & INIT_FLAG_DECODE_ON_DRAW)),
                     "Can't be discardable or decode-on-draw for multipart");
 
   // Store initialization data
-  if (aObserver) {
-    mObserver = aObserver->asWeakPtr();
-  }
   mSourceDataMimeType.Assign(aMimeType);
   mDiscardable = !!(aFlags & INIT_FLAG_DISCARDABLE);
   mDecodeOnDraw = !!(aFlags & INIT_FLAG_DECODE_ON_DRAW);
   mMultipart = !!(aFlags & INIT_FLAG_MULTIPART);
 
   // Statistics
   if (mDiscardable) {
     num_discardable_containers++;
@@ -647,31 +643,26 @@ RasterImage::RequestRefresh(const mozill
     // then we need to break out of this loop & wait for the frame(s)
     // to finish downloading
     if (!frameAdvanced && (currentFrameEndTime == oldFrameEndTime)) {
       break;
     }
   }
 
   if (frameAdvanced) {
-    if (!mObserver) {
-      NS_ERROR("Refreshing image after its imgRequest is gone");
-      StopAnimation();
-      return;
-    }
-
     // Notify listeners that our frame has actually changed, but do this only
     // once for all frames that we've now passed (if AdvanceFrame() was called
     // more than once).
     #ifdef DEBUG
       mFramesNotified++;
     #endif
 
     UpdateImageContainer();
-    mObserver->FrameChanged(&dirtyRect);
+    if (mStatusTracker)
+      mStatusTracker->GetDecoderObserver()->FrameChanged(&dirtyRect);
   }
 }
 
 //******************************************************************************
 /* [noscript] imgIContainer extractFrame(uint32_t aWhichFrame,
  *                                       [const] in nsIntRect aRegion,
  *                                       in uint32_t aFlags); */
 NS_IMETHODIMP
@@ -695,17 +686,17 @@ RasterImage::ExtractFrame(uint32_t aWhic
     return NS_ERROR_FAILURE;
 
   // Make a new container. This should switch to another class with bug 505959.
   nsRefPtr<RasterImage> img(new RasterImage());
 
   // We don't actually have a mimetype in this case. The empty string tells the
   // init routine not to try to instantiate a decoder. This should be fixed in
   // bug 505959.
-  img->Init(nullptr, "", INIT_FLAG_NONE);
+  img->Init("", INIT_FLAG_NONE);
   img->SetSize(aRegion.width, aRegion.height);
   img->mDecoded = true; // Also, we need to mark the image as decoded
   img->mHasBeenDecoded = true;
   img->mFrameDecodeFlags = aFlags & DECODE_FLAGS_MASK;
 
   if (!ApplyDecodeFlags(aFlags))
     return NS_ERROR_NOT_AVAILABLE;
   
@@ -1673,18 +1664,18 @@ RasterImage::ResetAnimation()
   mAnim->lastCompositedFrameIndex = -1;
   mAnim->currentAnimationFrameIndex = 0;
   UpdateImageContainer();
 
   // Note - We probably want to kick off a redecode somewhere around here when
   // we fix bug 500402.
 
   // Update display if we were animating before
-  if (mAnimating && mObserver)
-    mObserver->FrameChanged(&(mAnim->firstFrameRefreshArea));
+  if (mAnimating && mStatusTracker)
+    mStatusTracker->GetDecoderObserver()->FrameChanged(&(mAnim->firstFrameRefreshArea));
 
   if (ShouldAnimate()) {
     StartAnimation();
     // The animation may not have been running before, if mAnimationFinished
     // was false (before we changed it to true in this function). So, mark the
     // animation as running.
     mAnimating = true;
   }
@@ -2480,18 +2471,18 @@ RasterImage::Discard(bool force)
   // Clear our downscaled frame.
   mScaleResult.status = SCALE_INVALID;
   mScaleResult.frame = nullptr;
 
   // Flag that we no longer have decoded frames for this image
   mDecoded = false;
 
   // Notify that we discarded
-  if (mObserver)
-    mObserver->OnDiscard();
+  if (mStatusTracker)
+    mStatusTracker->GetDecoderObserver()->OnDiscard();
 
   if (force)
     DiscardTracker::Remove(&mDiscardTrackerNode);
 
   // Log
   PR_LOG(GetCompressedImageAccountingLog(), PR_LOG_DEBUG,
          ("CompressedImageAccounting: discarded uncompressed image "
           "data from RasterImage %p (%s) - %d frames (cached count: %d); "
@@ -2552,38 +2543,40 @@ RasterImage::InitDecoder(bool aDoSizeDec
   // Since we're not decoded, we should not have a discard timer active
   NS_ABORT_IF_FALSE(!DiscardingActive(), "Discard Timer active in InitDecoder()!");
 
   // Figure out which decoder we want
   eDecoderType type = GetDecoderType(mSourceDataMimeType.get());
   CONTAINER_ENSURE_TRUE(type != eDecoderType_unknown, NS_IMAGELIB_ERROR_NO_DECODER);
 
   // Instantiate the appropriate decoder
+  imgDecoderObserver* observer = mStatusTracker ? mStatusTracker->GetDecoderObserver()
+                                                : nullptr;
   switch (type) {
     case eDecoderType_png:
-      mDecoder = new nsPNGDecoder(*this, mObserver);
+      mDecoder = new nsPNGDecoder(*this, observer);
       break;
     case eDecoderType_gif:
-      mDecoder = new nsGIFDecoder2(*this, mObserver);
+      mDecoder = new nsGIFDecoder2(*this, observer);
       break;
     case eDecoderType_jpeg:
       // If we have all the data we don't want to waste cpu time doing
       // a progressive decode
-      mDecoder = new nsJPEGDecoder(*this, mObserver,
+      mDecoder = new nsJPEGDecoder(*this, observer,
                                    mHasBeenDecoded ? Decoder::SEQUENTIAL :
                                                      Decoder::PROGRESSIVE);
       break;
     case eDecoderType_bmp:
-      mDecoder = new nsBMPDecoder(*this, mObserver);
+      mDecoder = new nsBMPDecoder(*this, observer);
       break;
     case eDecoderType_ico:
-      mDecoder = new nsICODecoder(*this, mObserver);
+      mDecoder = new nsICODecoder(*this, observer);
       break;
     case eDecoderType_icon:
-      mDecoder = new nsIconDecoder(*this, mObserver);
+      mDecoder = new nsIconDecoder(*this, observer);
       break;
     default:
       NS_ABORT_IF_FALSE(0, "Shouldn't get here!");
   }
 
   // Initialize the decoder
   mDecoder->SetSizeDecode(aDoSizeDecode);
   mDecoder->SetDecodeFlags(mFrameDecodeFlags);
@@ -2934,21 +2927,20 @@ void
 RasterImage::ScalingDone(ScaleRequest* request, ScaleStatus status)
 {
   MOZ_ASSERT(status == SCALE_DONE || status == SCALE_INVALID);
   MOZ_ASSERT(request);
 
   if (status == SCALE_DONE) {
     MOZ_ASSERT(request->done);
 
-    RefPtr<imgDecoderObserver> observer(mObserver);
-    if (observer) {
+    if (mStatusTracker) {
       imgFrame *scaledFrame = request->dstFrame.get();
       scaledFrame->ImageUpdated(scaledFrame->GetRect());
-      observer->FrameChanged(&request->srcRect);
+      mStatusTracker->GetDecoderObserver()->FrameChanged(&request->srcRect);
     }
 
     mScaleResult.status = SCALE_DONE;
     mScaleResult.frame = request->dstFrame;
     mScaleResult.scale = request->scale;
   } else {
     mScaleResult.status = SCALE_INVALID;
     mScaleResult.frame = nullptr;
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -113,19 +113,18 @@ class nsIInputStream;
  * prevCompositingFrame.  When DoComposite gets called to do Frame 4, we
  * copy prevCompositingFrame back, and then draw Frame 4 on top.
  *
  * @par
  * The mAnim structure has members only needed for animated images, so
  * it's not allocated until the second frame is added.
  *
  * @note
- * mAnimationMode, mLoopCount and mObserver are not in the mAnim structure
- * because the first two have public setters and the observer we only get
- * in Init().
+ * mAnimationMode and mLoopCount are not in the mAnim structure because
+ * they have public setters.
  */
 
 class ScaleRequest;
 
 namespace mozilla {
 namespace layers {
 class LayerManager;
 class ImageContainer;
@@ -152,18 +151,17 @@ public:
 
   // (no public constructor - use ImageFactory)
   virtual ~RasterImage();
 
   virtual nsresult StartAnimation();
   virtual nsresult StopAnimation();
 
   // Methods inherited from Image
-  nsresult Init(imgDecoderObserver* aObserver,
-                const char* aMimeType,
+  nsresult Init(const char* aMimeType,
                 uint32_t aFlags);
   virtual nsIntRect FrameRect(uint32_t aWhichFrame) MOZ_OVERRIDE;
 
   // Raster-specific methods
   static NS_METHOD WriteToRasterImage(nsIInputStream* aIn, void* aClosure,
                                       const char* aFromRawSegment,
                                       uint32_t aToOffset, uint32_t aCount,
                                       uint32_t* aWriteCount);
@@ -639,18 +637,16 @@ private: // data
   // IMPORTANT: if you use mAnim in a method, call EnsureImageIsDecoded() first to ensure
   // that the frames actually exist (they may have been discarded to save memory, or
   // we maybe decoding on draw).
   RasterImage::Anim*        mAnim;
   
   //! # loops remaining before animation stops (-1 no stop)
   int32_t                    mLoopCount;
   
-  mozilla::WeakPtr<imgDecoderObserver> mObserver;
-
   // Discard members
   uint32_t                   mLockCount;
   DiscardTracker::Node       mDiscardTrackerNode;
 
   // Source data members
   FallibleTArray<char>       mSourceData;
   nsCString                  mSourceDataMimeType;
 
--- a/image/src/VectorImage.cpp
+++ b/image/src/VectorImage.cpp
@@ -186,35 +186,29 @@ VectorImage::VectorImage(imgStatusTracke
 VectorImage::~VectorImage()
 {
 }
 
 //------------------------------------------------------------------------------
 // Methods inherited from Image.h
 
 nsresult
-VectorImage::Init(imgDecoderObserver* aObserver,
-                  const char* aMimeType,
+VectorImage::Init(const char* aMimeType,
                   uint32_t aFlags)
 {
   // We don't support re-initialization
   if (mIsInitialized)
     return NS_ERROR_ILLEGAL_VALUE;
 
   NS_ABORT_IF_FALSE(!mIsFullyLoaded && !mHaveAnimations &&
                     !mHaveRestrictedRegion && !mError,
                     "Flags unexpectedly set before initialization");
-
-  if (aObserver) {
-    mObserver = aObserver->asWeakPtr();
-  }
   NS_ABORT_IF_FALSE(!strcmp(aMimeType, IMAGE_SVG_XML), "Unexpected mimetype");
 
   mIsInitialized = true;
-
   return NS_OK;
 }
 
 nsIntRect
 VectorImage::FrameRect(uint32_t aWhichFrame)
 {
   return nsIntRect::GetMaxSizedIntRect();
 }
@@ -709,19 +703,19 @@ VectorImage::OnStopRequest(nsIRequest* a
 
   mIsFullyLoaded = true;
   mHaveAnimations = mSVGDocumentWrapper->IsAnimated();
 
   // Start listening to our image for rendering updates
   mRenderingObserver = new SVGRootRenderingObserver(mSVGDocumentWrapper, this);
 
   // Tell *our* observers that we're done loading
-  RefPtr<imgDecoderObserver> observer(mObserver);
-  if (observer) {
+  if (mStatusTracker) {
     // NOTE: This signals that width/height are available.
+    imgDecoderObserver* observer = mStatusTracker->GetDecoderObserver();
     observer->OnStartContainer();
 
     observer->FrameChanged(&nsIntRect::GetMaxSizedIntRect());
     observer->OnStopFrame();
     observer->OnStopDecode(NS_OK);
   }
   EvaluateAnimation();
 
@@ -748,17 +742,17 @@ VectorImage::OnDataAvailable(nsIRequest*
 }
 
 // --------------------------
 // Invalidation helper method
 
 void
 VectorImage::InvalidateObserver()
 {
-  RefPtr<imgDecoderObserver> observer(mObserver);
-  if (observer) {
+  if (mStatusTracker) {
+    imgDecoderObserver* observer = mStatusTracker->GetDecoderObserver();
     observer->FrameChanged(&nsIntRect::GetMaxSizedIntRect());
     observer->OnStopFrame();
   }
 }
 
 } // namespace image
 } // namespace mozilla
--- a/image/src/VectorImage.h
+++ b/image/src/VectorImage.h
@@ -32,18 +32,17 @@ public:
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_IMGICONTAINER
 
   // (no public constructor - use ImageFactory)
   virtual ~VectorImage();
 
   // Methods inherited from Image
-  nsresult Init(imgDecoderObserver* aObserver,
-                const char* aMimeType,
+  nsresult Init(const char* aMimeType,
                 uint32_t aFlags);
   virtual nsIntRect FrameRect(uint32_t aWhichFrame) MOZ_OVERRIDE;
 
   virtual size_t HeapSizeOfSourceWithComputedFallback(nsMallocSizeOfFun aMallocSizeOf) const;
   virtual size_t HeapSizeOfDecodedWithComputedFallback(nsMallocSizeOfFun aMallocSizeOf) const;
   virtual size_t NonHeapSizeOfDecoded() const;
   virtual size_t OutOfProcessSizeOfDecoded() const;
 
@@ -63,17 +62,16 @@ public:
 protected:
   VectorImage(imgStatusTracker* aStatusTracker = nullptr, nsIURI* aURI = nullptr);
 
   virtual nsresult StartAnimation();
   virtual nsresult StopAnimation();
   virtual bool     ShouldAnimate();
 
 private:
-  WeakPtr<imgDecoderObserver>        mObserver;
   nsRefPtr<SVGDocumentWrapper>       mSVGDocumentWrapper;
   nsRefPtr<SVGRootRenderingObserver> mRenderingObserver;
 
   nsIntRect      mRestrictedRegion;       // If we were created by
                                           // ExtractFrame, this is the region
                                           // that we're restricted to using.
                                           // Otherwise, this is ignored.
 
--- a/image/src/imgDecoderObserver.h
+++ b/image/src/imgDecoderObserver.h
@@ -27,18 +27,17 @@
  * relied upon.
  *
  * Decode notifications may or may not be synchronous, depending on the
  * situation. If imgIDecoder::FLAG_SYNC_DECODE is passed to a function that
  * triggers a decode, all notifications that can be generated from the currently
  * loaded data fire before the call returns. If FLAG_SYNC_DECODE is not passed,
  * all, some, or none of the notifications may fire before the call returns.
  */
-class imgDecoderObserver : public mozilla::RefCounted<imgDecoderObserver>,
-                           public mozilla::SupportsWeakPtr<imgDecoderObserver>
+class imgDecoderObserver : public mozilla::RefCounted<imgDecoderObserver>
 {
 public:
   virtual ~imgDecoderObserver() = 0;
 
   /**
    * Load notification.
    *
    * called at the same time that nsIRequestObserver::onStartRequest would be