Bug 601470: Promote GetSourceDataSize & GetDecodedDataSize from RasterImage to Image superclass, and add implementations for VectorImage. r=joe a=joe
authorDaniel Holbert <dholbert@cs.stanford.edu>
Wed, 06 Oct 2010 08:37:12 -0700
changeset 54949 d262762e4f87ca99d5cae3a98bc777ec288dfb77
parent 54948 52b06ea6b602180185e091d7a6efba9d33ded008
child 54950 c6fa2ae426d4141d0e21e795b9cd8a8cac59db4f
push id16098
push userdholbert@mozilla.com
push dateWed, 06 Oct 2010 15:38:57 +0000
treeherdermozilla-central@d1518f80fce2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoe, joe
bugs601470
milestone2.0b7pre
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 601470: Promote GetSourceDataSize & GetDecodedDataSize from RasterImage to Image superclass, and add implementations for VectorImage. r=joe a=joe
modules/libpr0n/src/Image.cpp
modules/libpr0n/src/Image.h
modules/libpr0n/src/RasterImage.cpp
modules/libpr0n/src/RasterImage.h
modules/libpr0n/src/VectorImage.cpp
modules/libpr0n/src/VectorImage.h
modules/libpr0n/src/imgLoader.cpp
modules/libpr0n/test/mochitest/Makefile.in
modules/libpr0n/test/mochitest/lime100x100.svg
modules/libpr0n/test/mochitest/test_bug601470.html
--- a/modules/libpr0n/src/Image.cpp
+++ b/modules/libpr0n/src/Image.cpp
@@ -39,26 +39,36 @@
 
 namespace mozilla {
 namespace imagelib {
 
 // Constructor
 Image::Image(imgStatusTracker* aStatusTracker) :
   mAnimationConsumers(0),
   mInitialized(PR_FALSE),
-  mAnimating(PR_FALSE)
+  mAnimating(PR_FALSE),
+  mError(PR_FALSE)
 {
   if (aStatusTracker) {
     mStatusTracker = aStatusTracker;
     mStatusTracker->SetImage(this);
   } else {
     mStatusTracker = new imgStatusTracker(this);
   }
 }
 
+PRUint32
+Image::GetDataSize()
+{
+  if (mError)
+    return 0;
+  
+  return GetSourceDataSize() + GetDecodedDataSize();
+}
+
 // Translates a mimetype into a concrete decoder
 Image::eDecoderType
 Image::GetDecoderType(const char *aMimeType)
 {
   // By default we don't know
   eDecoderType rv = eDecoderType_unknown;
 
   // PNG
--- a/modules/libpr0n/src/Image.h
+++ b/modules/libpr0n/src/Image.h
@@ -88,17 +88,23 @@ public:
    * frame.
    */
   virtual void GetCurrentFrameRect(nsIntRect& aRect) = 0;
 
   /**
    * The size, in bytes, occupied by the significant data portions of the image.
    * This includes both compressed source data and decoded frames.
    */
-  virtual PRUint32 GetDataSize() = 0;
+  PRUint32 GetDataSize();
+
+  /**
+   * The components that make up GetDataSize().
+   */      
+  virtual PRUint32 GetDecodedDataSize() = 0;
+  virtual PRUint32 GetSourceDataSize() = 0;
 
   // Mimetype translation
   enum eDecoderType {
     eDecoderType_png     = 0,
     eDecoderType_gif     = 1,
     eDecoderType_jpeg    = 2,
     eDecoderType_bmp     = 3,
     eDecoderType_ico     = 4,
@@ -125,16 +131,17 @@ protected:
   virtual nsresult StartAnimation() = 0;
   virtual nsresult StopAnimation() = 0;
 
   // Member data shared by all implementations of this abstract class
   nsAutoPtr<imgStatusTracker> mStatusTracker;
   PRUint32                    mAnimationConsumers;
   PRPackedBool                mInitialized;   // Have we been initalized?
   PRPackedBool                mAnimating;
+  PRPackedBool                mError;         // Error handling
 
   /**
    * Extended by child classes, if they have additional
    * conditions for being able to animate
    */
   virtual PRBool ShouldAnimate() { return mAnimationConsumers > 0; }
 };
 
--- a/modules/libpr0n/src/RasterImage.cpp
+++ b/modules/libpr0n/src/RasterImage.cpp
@@ -193,17 +193,16 @@ RasterImage::RasterImage(imgStatusTracke
   mDecodeOnDraw(PR_FALSE),
   mMultipart(PR_FALSE),
   mDiscardable(PR_FALSE),
   mHasSourceData(PR_FALSE),
   mDecoded(PR_FALSE),
   mHasBeenDecoded(PR_FALSE),
   mWorkerPending(PR_FALSE),
   mInDecoder(PR_FALSE),
-  mError(PR_FALSE),
   mAnimationFinished(PR_FALSE)
 {
   // Set up the discard tracker node.
   mDiscardTrackerNode.curr = this;
   mDiscardTrackerNode.prev = mDiscardTrackerNode.next = nsnull;
 
   // Statistics
   num_containers++;
@@ -674,52 +673,36 @@ RasterImage::GetFrame(PRUint32 aWhichFra
   }
 
   *_retval = framesurf.forget().get();
 
   return rv;
 }
 
 PRUint32
-RasterImage::GetDataSize()
-{
-  if (mError)
-    return 0;
-
-  // Start with 0
-  PRUint32 size = 0;
-
-  // Account for any compressed source data
-  size += GetSourceDataSize();
-  NS_ABORT_IF_FALSE(StoringSourceData() || (size == 0),
-                    "Non-zero source data size when we aren't storing it?");
-
-  // Account for any uncompressed frames
-  size += GetDecodedDataSize();
-
-  return size;
-}
-
-PRUint32
 RasterImage::GetDecodedDataSize()
 {
   PRUint32 val = 0;
   for (PRUint32 i = 0; i < mFrames.Length(); ++i) {
     imgFrame *frame = mFrames.SafeElementAt(i, nsnull);
     NS_ABORT_IF_FALSE(frame, "Null frame in frame array!");
     val += frame->EstimateMemoryUsed();
   }
 
   return val;
 }
 
 PRUint32
 RasterImage::GetSourceDataSize()
 {
-  return mSourceData.Length();
+  PRUint32 sourceDataSize = mSourceData.Length();
+  
+  NS_ABORT_IF_FALSE(StoringSourceData() || (sourceDataSize == 0),
+                    "Non-zero source data size when we aren't storing it?");
+  return sourceDataSize;
 }
 
 void
 RasterImage::DeleteImgFrame(PRUint32 framenum)
 {
   NS_ABORT_IF_FALSE(framenum < mFrames.Length(), "Deleting invalid frame!");
 
   delete mFrames[framenum];
--- a/modules/libpr0n/src/RasterImage.h
+++ b/modules/libpr0n/src/RasterImage.h
@@ -175,17 +175,16 @@ public:
   virtual PRUint16 GetType() { return imgIContainer::TYPE_RASTER; }
 
   // Methods inherited from Image
   nsresult Init(imgIDecoderObserver* aObserver,
                 const char* aMimeType,
                 const char* aURIString,
                 PRUint32 aFlags);
   void     GetCurrentFrameRect(nsIntRect& aRect);
-  PRUint32 GetDataSize();
 
   // Raster-specific methods
   static NS_METHOD WriteToRasterImage(nsIInputStream* aIn, void* aClosure,
                                       const char* aFromRawSegment,
                                       PRUint32 aToOffset, PRUint32 aCount,
                                       PRUint32* aWriteCount);
 
   /* The index of the current frame that would be drawn if the image was to be
@@ -490,18 +489,16 @@ private: // data
   // Do we have the frames in decoded form?
   PRPackedBool               mDecoded:1;
   PRPackedBool               mHasBeenDecoded:1;
 
   // Helpers for decoder
   PRPackedBool               mWorkerPending:1;
   PRPackedBool               mInDecoder:1;
 
-  PRPackedBool               mError:1;  // Error handling
-
   // Whether the animation can stop, due to running out
   // of frames, or no more owning request
   PRPackedBool               mAnimationFinished:1;
 
   // Decoding
   nsresult WantDecodedFrames();
   nsresult SyncDecode();
   nsresult InitDecoder(bool aDoSizeDecode);
--- a/modules/libpr0n/src/VectorImage.cpp
+++ b/modules/libpr0n/src/VectorImage.cpp
@@ -209,18 +209,17 @@ NS_IMPL_ISUPPORTS3(VectorImage,
 VectorImage::VectorImage(imgStatusTracker* aStatusTracker) :
   Image(aStatusTracker), // invoke superclass's constructor
   mRestrictedRegion(0, 0, 0, 0),
   mLastRenderedSize(0, 0),
   mAnimationMode(kNormalAnimMode),
   mIsInitialized(PR_FALSE),
   mIsFullyLoaded(PR_FALSE),
   mHaveAnimations(PR_FALSE),
-  mHaveRestrictedRegion(PR_FALSE),
-  mError(PR_FALSE)
+  mHaveRestrictedRegion(PR_FALSE)
 {
 }
 
 VectorImage::~VectorImage()
 {
 }
 
 //------------------------------------------------------------------------------
@@ -250,23 +249,31 @@ VectorImage::Init(imgIDecoderObserver* a
 
 void
 VectorImage::GetCurrentFrameRect(nsIntRect& aRect)
 {
   aRect = nsIntRect::GetMaxSizedIntRect();
 }
 
 PRUint32
-VectorImage::GetDataSize()
+VectorImage::GetDecodedDataSize()
 {
-  // XXXdholbert "sizeof(*this)" is, of course, quite an underestimate.  This
-  // needs to be smarter -- see bug 590790.
+  // XXXdholbert TODO: return num bytes used by helper SVG doc. (bug 590790)
   return sizeof(*this);
 }
 
+PRUint32
+VectorImage::GetSourceDataSize()
+{
+  // We're not storing the source data -- we just feed that directly to
+  // our helper SVG document as we receive it, for it to parse.
+  // So 0 is an appropriate return value here.
+  return 0;
+}
+
 nsresult
 VectorImage::StartAnimation()
 {
   if (mError)
     return NS_ERROR_FAILURE;
 
   if (mAnimationMode == kDontAnimMode ||
       !mIsFullyLoaded || !mHaveAnimations) {
--- a/modules/libpr0n/src/VectorImage.h
+++ b/modules/libpr0n/src/VectorImage.h
@@ -67,17 +67,18 @@ public:
   PRUint16 GetType() { return imgIContainer::TYPE_VECTOR; }
 
   // Methods inherited from Image
   nsresult Init(imgIDecoderObserver* aObserver,
                 const char* aMimeType,
                 const char* aURIString,
                 PRUint32 aFlags);
   void GetCurrentFrameRect(nsIntRect& aRect);
-  PRUint32 GetDataSize();
+  PRUint32 GetDecodedDataSize();
+  PRUint32 GetSourceDataSize();
 
   // Callback for SVGRootRenderingObserver
   void InvalidateObserver();
 
 protected:
   virtual nsresult StartAnimation();
   virtual nsresult StopAnimation();
 
@@ -101,16 +102,14 @@ private:
   PRUint16       mAnimationMode;          // Are we allowed to animate?
 
   PRPackedBool   mIsInitialized:1;        // Have we been initalized?
   PRPackedBool   mIsFullyLoaded:1;        // Has OnStopRequest been called?
   PRPackedBool   mHaveAnimations:1;       // Is our SVG content SMIL-animated?
                                           // (Only set after mIsFullyLoaded.)
   PRPackedBool   mHaveRestrictedRegion:1; // Are we a restricted-region clone
                                           // created via ExtractFrame?
-
-  PRPackedBool   mError:1;                // Error handling
 };
 
 } // namespace imagelib
 } // namespace mozilla
 
 #endif // mozilla_imagelib_VectorImage_h_
--- a/modules/libpr0n/src/imgLoader.cpp
+++ b/modules/libpr0n/src/imgLoader.cpp
@@ -220,17 +220,17 @@ public:
       if (entry->HasNoProxies())
         return PL_DHASH_NEXT;
     } else {
       if (!entry->HasNoProxies())
         return PL_DHASH_NEXT;
     }
 
     nsRefPtr<imgRequest> req = entry->GetRequest();
-    RasterImage *image = static_cast<RasterImage*>(req->mImage.get());
+    Image *image = static_cast<Image*>(req->mImage.get());
     if (!image)
       return PL_DHASH_NEXT;
 
     if (rtype & RAW_BIT) {
       arg->value += image->GetSourceDataSize();
     } else {
       arg->value += image->GetDecodedDataSize();
     }
--- a/modules/libpr0n/test/mochitest/Makefile.in
+++ b/modules/libpr0n/test/mochitest/Makefile.in
@@ -40,16 +40,17 @@ topsrcdir	= @top_srcdir@
 srcdir		= @srcdir@
 VPATH		= @srcdir@
 relativesrcdir  = modules/libpr0n/test/mochitest
 
 include $(DEPTH)/config/autoconf.mk
 include $(topsrcdir)/config/rules.mk
 
 _TEST_FILES =   imgutils.js \
+                lime100x100.svg \
                 test_bug399925.html \
                 bug399925.gif \
                 schrep.png \
                 bug468160.sjs \
                 test_bug468160.html \
                 red.png \
                 test_bug466586.html \
                 big.png \
@@ -66,16 +67,17 @@ include $(topsrcdir)/config/rules.mk
                 bug496292-2.sjs \
                 test_bug512435.html \
                 damon.jpg \
                 shaver.png \
                 test_bug497665.html \
                 bug497665-iframe.html \
                 bug497665.sjs \
                 test_bug553982.html \
+                test_bug601470.html \
 		$(NULL)
 
 # Tests disabled due to intermittent orange
 # test_bug435296.html disabled - See bug 578591
 # test_bug478398.html disabled - See bug 579139
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
copy from layout/reftests/svg/image/lime100x100.svg
copy to modules/libpr0n/test/mochitest/lime100x100.svg
new file mode 100644
--- /dev/null
+++ b/modules/libpr0n/test/mochitest/test_bug601470.html
@@ -0,0 +1,47 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=601470
+-->
+<head>
+  <title>Test for Bug 601470</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=601470">Mozilla Bug 601470</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  <img src="lime100x100.svg" style="width: 100px; height: 100px;">
+  <img src="damon.jpg"       style="width: 100px; height: 100px;">
+</div>
+<pre id="test">
+<script type="application/javascript">
+/** Test for Bug 601470 **/
+
+SimpleTest.waitForExplicitFinish();
+
+window.onload = function() {
+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+  var mgr = Components
+    .classes["@mozilla.org/memory-reporter-manager;1"]
+    .getService(Components.interfaces.nsIMemoryReporterManager);
+
+  var e = mgr.enumerateReporters();
+  var memoryCounter = 0;
+  while (e.hasMoreElements()) {
+    var mr =
+      e.getNext().QueryInterface(Components.interfaces.nsIMemoryReporter);
+    memoryCounter += mr.memoryUsed;
+  }
+  ok(memoryCounter > 0, "we should be using a nonzero amount of memory");
+  ok(true, "yay, didn't crash!");
+
+  SimpleTest.finish();
+}
+
+</script>
+</pre>
+</body>
+</html>