Bug 1177615 - Rip everything related to FLAG_DECODE_STARTED out of ImageLib. r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Mon, 06 Jul 2015 17:11:14 -0700
changeset 283271 20748e1e2276b67bc7f9551127cbd62de7a3d03d
parent 283270 c2fdd6543cfa04b22a27540eb745149f23b52a75
child 283272 551085502cbf5718a46538b3ff200fa0cbd258cc
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1177615
milestone42.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 1177615 - Rip everything related to FLAG_DECODE_STARTED out of ImageLib. r=tn
image/Decoder.cpp
image/IProgressObserver.h
image/MultipartImage.cpp
image/MultipartImage.h
image/ProgressTracker.cpp
image/ProgressTracker.h
image/RasterImage.cpp
image/RasterImage.h
image/VectorImage.cpp
image/imgIRequest.idl
image/imgRequest.cpp
image/imgRequestProxy.cpp
image/imgRequestProxy.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -78,20 +78,16 @@ Decoder::~Decoder()
  */
 
 void
 Decoder::Init()
 {
   // No re-initializing
   MOZ_ASSERT(!mInitialized, "Can't re-initialize a decoder!");
 
-  if (!IsSizeDecode()) {
-      mProgress |= FLAG_DECODE_STARTED;
-  }
-
   // Implementation-specific initialization
   InitInternal();
 
   mInitialized = true;
 }
 
 // Initializes a decoder whose image and observer is already being used by a
 // parent decoder
--- a/image/IProgressObserver.h
+++ b/image/IProgressObserver.h
@@ -40,17 +40,16 @@ public:
   virtual void OnLoadComplete(bool aLastPart) = 0;
 
   // imgIOnloadBlocker methods:
   virtual void BlockOnload() = 0;
   virtual void UnblockOnload() = 0;
 
   // Other, internal-only methods:
   virtual void SetHasImage() = 0;
-  virtual void OnStartDecode() = 0;
   virtual bool NotificationsDeferred() const = 0;
   virtual void SetNotificationsDeferred(bool aDeferNotifications) = 0;
 
 protected:
   virtual ~IProgressObserver() { }
 };
 
 } // namespace image
--- a/image/MultipartImage.cpp
+++ b/image/MultipartImage.cpp
@@ -71,17 +71,16 @@ public:
       FinishObserving();
     }
   }
 
   // Other notifications are ignored.
   virtual void BlockOnload() override { }
   virtual void UnblockOnload() override { }
   virtual void SetHasImage() override { }
-  virtual void OnStartDecode() override { }
   virtual bool NotificationsDeferred() const override { return false; }
   virtual void SetNotificationsDeferred(bool) override { }
 
 private:
   virtual ~NextPartObserver() { }
 
   void FinishObserving()
   {
@@ -310,22 +309,16 @@ MultipartImage::OnLoadComplete(bool aLas
 }
 
 void
 MultipartImage::SetHasImage()
 {
   mTracker->OnImageAvailable();
 }
 
-void
-MultipartImage::OnStartDecode()
-{
-  mTracker->SyncNotifyProgress(FLAG_DECODE_STARTED);
-}
-
 bool
 MultipartImage::NotificationsDeferred() const
 {
   return mDeferNotifications;
 }
 
 void
 MultipartImage::SetNotificationsDeferred(bool aDeferNotifications)
--- a/image/MultipartImage.h
+++ b/image/MultipartImage.h
@@ -53,17 +53,16 @@ public:
   virtual uint32_t GetAnimationConsumers() override { return 1; }
 #endif
 
   // Overridden IProgressObserver methods:
   virtual void Notify(int32_t aType,
                       const nsIntRect* aRect = nullptr) override;
   virtual void OnLoadComplete(bool aLastPart) override;
   virtual void SetHasImage() override;
-  virtual void OnStartDecode() override;
   virtual bool NotificationsDeferred() const override;
   virtual void SetNotificationsDeferred(bool aDeferNotifications) override;
 
   // We don't allow multipart images to block onload, so we override these
   // methods to do nothing.
   virtual void BlockOnload() override { }
   virtual void UnblockOnload() override { }
 
--- a/image/ProgressTracker.cpp
+++ b/image/ProgressTracker.cpp
@@ -25,37 +25,33 @@ namespace image {
 static void
 CheckProgressConsistency(Progress aProgress)
 {
   // Check preconditions for every progress bit.
 
   if (aProgress & FLAG_SIZE_AVAILABLE) {
     // No preconditions.
   }
-  if (aProgress & FLAG_DECODE_STARTED) {
+  if (aProgress & FLAG_DECODE_COMPLETE) {
     // No preconditions.
   }
-  if (aProgress & FLAG_DECODE_COMPLETE) {
-    MOZ_ASSERT(aProgress & FLAG_DECODE_STARTED);
-  }
   if (aProgress & FLAG_FRAME_COMPLETE) {
-    MOZ_ASSERT(aProgress & FLAG_DECODE_STARTED);
+    // No preconditions.
   }
   if (aProgress & FLAG_LOAD_COMPLETE) {
     // No preconditions.
   }
   if (aProgress & FLAG_ONLOAD_BLOCKED) {
     // No preconditions.
   }
   if (aProgress & FLAG_ONLOAD_UNBLOCKED) {
     MOZ_ASSERT(aProgress & FLAG_ONLOAD_BLOCKED);
     MOZ_ASSERT(aProgress & (FLAG_SIZE_AVAILABLE | FLAG_HAS_ERROR));
   }
   if (aProgress & FLAG_IS_ANIMATED) {
-    MOZ_ASSERT(aProgress & FLAG_DECODE_STARTED);
     MOZ_ASSERT(aProgress & FLAG_SIZE_AVAILABLE);
   }
   if (aProgress & FLAG_HAS_TRANSPARENCY) {
     MOZ_ASSERT(aProgress & FLAG_SIZE_AVAILABLE);
   }
   if (aProgress & FLAG_LAST_PART_COMPLETE) {
     MOZ_ASSERT(aProgress & FLAG_LOAD_COMPLETE);
   }
@@ -85,19 +81,16 @@ uint32_t
 ProgressTracker::GetImageStatus() const
 {
   uint32_t status = imgIRequest::STATUS_NONE;
 
   // Translate our current state to a set of imgIRequest::STATE_* flags.
   if (mProgress & FLAG_SIZE_AVAILABLE) {
     status |= imgIRequest::STATUS_SIZE_AVAILABLE;
   }
-  if (mProgress & FLAG_DECODE_STARTED) {
-    status |= imgIRequest::STATUS_DECODE_STARTED;
-  }
   if (mProgress & FLAG_DECODE_COMPLETE) {
     status |= imgIRequest::STATUS_DECODE_COMPLETE;
   }
   if (mProgress & FLAG_FRAME_COMPLETE) {
     status |= imgIRequest::STATUS_FRAME_COMPLETE;
   }
   if (mProgress & FLAG_LOAD_COMPLETE) {
     status |= imgIRequest::STATUS_LOAD_COMPLETE;
@@ -270,20 +263,16 @@ ProgressTracker::SyncNotifyInternal(Obse
   MOZ_ASSERT(NS_IsMainThread());
 
   typedef imgINotificationObserver I;
 
   if (aProgress & FLAG_SIZE_AVAILABLE) {
     NOTIFY_IMAGE_OBSERVERS(aObservers, Notify(I::SIZE_AVAILABLE));
   }
 
-  if (aProgress & FLAG_DECODE_STARTED) {
-    NOTIFY_IMAGE_OBSERVERS(aObservers, OnStartDecode());
-  }
-
   if (aProgress & FLAG_ONLOAD_BLOCKED) {
     NOTIFY_IMAGE_OBSERVERS(aObservers, BlockOnload());
   }
 
   if (aHasImage) {
     // OnFrameUpdate
     // If there's any content in this frame at all (always true for
     // vector images, true for raster images that have decoded at
--- a/image/ProgressTracker.h
+++ b/image/ProgressTracker.h
@@ -23,26 +23,25 @@ namespace image {
 
 class AsyncNotifyRunnable;
 class AsyncNotifyCurrentStateRunnable;
 class Image;
 
 // Image progress bitflags.
 enum {
   FLAG_SIZE_AVAILABLE     = 1u << 0,  // STATUS_SIZE_AVAILABLE
-  FLAG_DECODE_STARTED     = 1u << 1,  // STATUS_DECODE_STARTED
-  FLAG_DECODE_COMPLETE    = 1u << 2,  // STATUS_DECODE_COMPLETE
-  FLAG_FRAME_COMPLETE     = 1u << 3,  // STATUS_FRAME_COMPLETE
-  FLAG_LOAD_COMPLETE      = 1u << 4,  // STATUS_LOAD_COMPLETE
-  FLAG_ONLOAD_BLOCKED     = 1u << 5,
-  FLAG_ONLOAD_UNBLOCKED   = 1u << 6,
-  FLAG_IS_ANIMATED        = 1u << 7,  // STATUS_IS_ANIMATED
-  FLAG_HAS_TRANSPARENCY   = 1u << 8,  // STATUS_HAS_TRANSPARENCY
-  FLAG_LAST_PART_COMPLETE = 1u << 9,
-  FLAG_HAS_ERROR          = 1u << 10  // STATUS_ERROR
+  FLAG_DECODE_COMPLETE    = 1u << 1,  // STATUS_DECODE_COMPLETE
+  FLAG_FRAME_COMPLETE     = 1u << 2,  // STATUS_FRAME_COMPLETE
+  FLAG_LOAD_COMPLETE      = 1u << 3,  // STATUS_LOAD_COMPLETE
+  FLAG_ONLOAD_BLOCKED     = 1u << 4,
+  FLAG_ONLOAD_UNBLOCKED   = 1u << 5,
+  FLAG_IS_ANIMATED        = 1u << 6,  // STATUS_IS_ANIMATED
+  FLAG_HAS_TRANSPARENCY   = 1u << 7,  // STATUS_HAS_TRANSPARENCY
+  FLAG_LAST_PART_COMPLETE = 1u << 8,
+  FLAG_HAS_ERROR          = 1u << 9   // STATUS_ERROR
 };
 
 typedef uint32_t Progress;
 
 const uint32_t NoProgress = 0;
 
 inline Progress LoadCompleteProgress(bool aLastPart,
                                      bool aError,
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1233,61 +1233,38 @@ RasterImage::NotifyForLoadEvent(Progress
   MOZ_ASSERT(mHasSize || mError, "Need to know size before firing load event");
   MOZ_ASSERT(!mHasSize ||
              (mProgressTracker->GetProgress() & FLAG_SIZE_AVAILABLE),
              "Should have notified that the size is available if we have it");
 
   if (mDecodeOnlyOnDraw) {
     // For decode-only-on-draw images, we want to send notifications as if we've
     // already finished decoding. Otherwise some observers will never even try
-    // to draw. (We may have already sent some of these notifications from
-    // NotifyForDecodeOnlyOnDraw(), but ProgressTracker will ensure no duplicate
-    // notifications get sent.)
-    aProgress |= FLAG_DECODE_STARTED |
-                 FLAG_FRAME_COMPLETE |
-                 FLAG_DECODE_COMPLETE;
+    // to draw.
+    aProgress |= FLAG_FRAME_COMPLETE | FLAG_DECODE_COMPLETE;
   }
 
   // If we encountered an error, make sure we notify for that as well.
   if (mError) {
     aProgress |= FLAG_HAS_ERROR;
   }
 
   // Notify our listeners, which will fire this image's load event.
   NotifyProgress(aProgress);
 }
 
-void
-RasterImage::NotifyForDecodeOnlyOnDraw()
-{
-  if (!NS_IsMainThread()) {
-    nsCOMPtr<nsIRunnable> runnable =
-      NS_NewRunnableMethod(this, &RasterImage::NotifyForDecodeOnlyOnDraw);
-    NS_DispatchToMainThread(runnable);
-    return;
-  }
-
-  NotifyProgress(FLAG_DECODE_STARTED);
-}
-
 nsresult
 RasterImage::OnImageDataAvailable(nsIRequest*,
                                   nsISupports*,
                                   nsIInputStream* aInStr,
                                   uint64_t aOffset,
                                   uint32_t aCount)
 {
   nsresult rv;
 
-  if (MOZ_UNLIKELY(mDecodeOnlyOnDraw && aOffset == 0)) {
-    // If we're a decode-only-on-draw image, send notifications as if we've just
-    // started decoding.
-    NotifyForDecodeOnlyOnDraw();
-  }
-
   // WriteToSourceBuffer always consumes everything it gets if it doesn't run
   // out of memory.
   uint32_t bytesRead;
   rv = aInStr->ReadSegments(WriteToSourceBuffer, this, aCount, &bytesRead);
 
   MOZ_ASSERT(bytesRead == aCount || HasError() || NS_FAILED(rv),
     "WriteToSourceBuffer should consume everything if ReadSegments succeeds or "
     "the image must be in error!");
@@ -1630,24 +1607,16 @@ RasterImage::Decode(const Maybe<IntSize>
   }
 
   // Create a decoder.
   nsRefPtr<Decoder> decoder = CreateDecoder(aSize, aFlags);
   if (!decoder) {
     return NS_ERROR_FAILURE;
   }
 
-  if (aSize) {
-    // This isn't a size decode (which doesn't send any early notifications), so
-    // send out notifications right away.
-    NotifyProgress(decoder->TakeProgress(),
-                   decoder->TakeInvalidRect(),
-                   decoder->GetDecodeFlags());
-  }
-
   if (mHasSourceData) {
     // If we have all the data, we can sync decode if requested.
     if (aFlags & FLAG_SYNC_DECODE) {
       PROFILER_LABEL_PRINTF("DecodePool", "SyncDecodeIfPossible",
         js::ProfileEntry::Category::GRAPHICS, "%s", GetURIString().get());
       DecodePool::Singleton()->SyncDecodeIfPossible(decoder);
       return NS_OK;
     }
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -241,17 +241,16 @@ public:
                                         uint64_t aSourceOffset,
                                         uint32_t aCount) override;
   virtual nsresult OnImageDataComplete(nsIRequest* aRequest,
                                        nsISupports* aContext,
                                        nsresult aStatus,
                                        bool aLastPart) override;
 
   void NotifyForLoadEvent(Progress aProgress);
-  void NotifyForDecodeOnlyOnDraw();
 
   /**
    * A hint of the number of bytes of source data that the image contains. If
    * called early on, this can help reduce copying and reallocations by
    * appropriately preallocating the source data buffer.
    *
    * We take this approach rather than having the source data management code do
    * something more complicated (like chunklisting) because HTTP is by far the
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -1090,22 +1090,20 @@ VectorImage::OnStartRequest(nsIRequest* 
   mSVGDocumentWrapper = new SVGDocumentWrapper();
   nsresult rv = mSVGDocumentWrapper->OnStartRequest(aRequest, aCtxt);
   if (NS_FAILED(rv)) {
     mSVGDocumentWrapper = nullptr;
     mError = true;
     return rv;
   }
 
-  // Sending StartDecode will block page load until the document's ready.  (We
-  // unblock it by sending StopDecode in OnSVGDocumentLoaded or
-  // OnSVGDocumentError.)
+  // Block page load until the document's ready.  (We unblock it in
+  // OnSVGDocumentLoaded or OnSVGDocumentError.)
   if (mProgressTracker) {
-    mProgressTracker->SyncNotifyProgress(FLAG_DECODE_STARTED |
-                                         FLAG_ONLOAD_BLOCKED);
+    mProgressTracker->SyncNotifyProgress(FLAG_ONLOAD_BLOCKED);
   }
 
   // Create a listener to wait until the SVG document is fully loaded, which
   // will signal that this image is ready to render. Certain error conditions
   // will prevent us from ever getting this notification, so we also create a
   // listener that waits for parsing to complete and cancels the
   // SVGLoadEventListener if needed. The listeners are automatically attached
   // to the document by their constructors.
--- a/image/imgIRequest.idl
+++ b/image/imgIRequest.idl
@@ -14,17 +14,17 @@ interface nsIPrincipal;
 
 /**
  * imgIRequest interface
  *
  * @author Stuart Parmenter <stuart@mozilla.com>
  * @version 0.1
  * @see imagelib2
  */
-[scriptable, builtinclass, uuid(83a7708b-5c35-409f-bab3-7fc08be6a264)]
+[scriptable, builtinclass, uuid(4cb01f0a-ef94-4345-a8d7-1a93f15ff548)]
 interface imgIRequest : nsIRequest
 {
   /**
    * the image container...
    * @return the image object associated with the request.
    * @attention NEED DOCS
    */
   readonly attribute imgIContainer image;
@@ -42,38 +42,34 @@ interface imgIRequest : nsIRequest
    * and height of the image, and have thus called SetSize()
    * on the container.
    *
    * STATUS_LOAD_COMPLETE: The data has been fully loaded
    * to memory, but not necessarily fully decoded.
    *
    * STATUS_ERROR: An error occurred loading the image.
    *
-   * STATUS_DECODE_STARTED: The decoding process has begun, but not yet
-   * finished.
-   *
    * STATUS_FRAME_COMPLETE: The first frame has been
    * completely decoded.
    *
    * STATUS_DECODE_COMPLETE: The whole image has been decoded.
    *
    * STATUS_IS_ANIMATED: The image is animated.
    *
    * STATUS_HAS_TRANSPARENCY: The image is partially or completely transparent.
    */
   //@{
   const long STATUS_NONE             = 0x0;
   const long STATUS_SIZE_AVAILABLE   = 0x1;
   const long STATUS_LOAD_COMPLETE    = 0x2;
   const long STATUS_ERROR            = 0x4;
-  const long STATUS_DECODE_STARTED   = 0x8;
-  const long STATUS_FRAME_COMPLETE   = 0x10;
-  const long STATUS_DECODE_COMPLETE  = 0x20;
-  const long STATUS_IS_ANIMATED      = 0x40;
-  const long STATUS_HAS_TRANSPARENCY = 0x80;
+  const long STATUS_FRAME_COMPLETE   = 0x8;
+  const long STATUS_DECODE_COMPLETE  = 0x10;
+  const long STATUS_IS_ANIMATED      = 0x20;
+  const long STATUS_HAS_TRANSPARENCY = 0x40;
   //@}
 
   /**
    * Status flags of the STATUS_* variety.
    */
   readonly attribute unsigned long imageStatus;
 
   /*
--- a/image/imgRequest.cpp
+++ b/image/imgRequest.cpp
@@ -907,23 +907,25 @@ imgRequest::CheckListenerChain()
 /** nsIStreamListener methods **/
 
 struct NewPartResult final
 {
   explicit NewPartResult(Image* aExistingImage)
     : mImage(aExistingImage)
     , mIsFirstPart(!aExistingImage)
     , mSucceeded(false)
+    , mShouldResetCacheEntry(false)
   { }
 
   nsAutoCString mContentType;
   nsAutoCString mContentDisposition;
   nsRefPtr<Image> mImage;
   const bool mIsFirstPart;
   bool mSucceeded;
+  bool mShouldResetCacheEntry;
 };
 
 static NewPartResult
 PrepareForNewPart(nsIRequest* aRequest, nsIInputStream* aInStr, uint32_t aCount,
                   ImageURL* aURI, bool aIsMultipart, Image* aExistingImage,
                   ProgressTracker* aProgressTracker, uint32_t aInnerWindowId)
 {
   NewPartResult result(aExistingImage);
@@ -973,16 +975,19 @@ PrepareForNewPart(nsIRequest* aRequest, 
       // First part for a multipart channel. Create the MultipartImage wrapper.
       MOZ_ASSERT(aProgressTracker, "Shouldn't have given away tracker yet");
       result.mImage =
         ImageFactory::CreateMultipartImage(partImage, aProgressTracker);
     } else {
       // Transition to the new part.
       auto multipartImage = static_cast<MultipartImage*>(aExistingImage);
       multipartImage->BeginTransitionToPart(partImage);
+
+      // Reset our cache entry size so it doesn't keep growing without bound.
+      result.mShouldResetCacheEntry = true;
     }
   } else {
     MOZ_ASSERT(!aExistingImage, "New part for non-multipart channel?");
     MOZ_ASSERT(aProgressTracker, "Shouldn't have given away tracker yet");
 
     // Create an image using our progress tracker.
     result.mImage =
       ImageFactory::CreateImage(aRequest, aProgressTracker, result.mContentType,
@@ -1034,16 +1039,20 @@ imgRequest::FinishPreparingForNewPart(co
 
   if (aResult.mIsFirstPart) {
     // Notify listeners that we have an image.
     nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
     progressTracker->OnImageAvailable();
     MOZ_ASSERT(progressTracker->HasImage());
   }
 
+  if (aResult.mShouldResetCacheEntry) {
+    ResetCacheEntry();
+  }
+
   if (IsDecodeRequested()) {
     aResult.mImage->RequestDecode();
   }
 }
 
 NS_IMETHODIMP
 imgRequest::OnDataAvailable(nsIRequest* aRequest, nsISupports* aContext,
                             nsIInputStream* aInStr, uint64_t aOffset,
--- a/image/imgRequestProxy.cpp
+++ b/image/imgRequestProxy.cpp
@@ -799,31 +799,16 @@ imgRequestProxy::GetHasTransferredData(b
     *hasData = GetOwner()->HasTransferredData();
   } else {
     // The safe thing to do is to claim we have data
     *hasData = true;
   }
   return NS_OK;
 }
 
-void
-imgRequestProxy::OnStartDecode()
-{
-  // This notification is deliberately not propagated since there are no
-  // listeners who care about it.
-  if (GetOwner()) {
-    // In the case of streaming jpegs, it is possible to get multiple
-    // OnStartDecodes which indicates the beginning of a new decode.  The cache
-    // entry's size therefore needs to be reset to 0 here.  If we do not do
-    // this, the code in ProgressTrackerObserver::OnStopFrame will continue to
-    // increase the data size cumulatively.
-    GetOwner()->ResetCacheEntry();
-  }
-}
-
 static const char*
 NotificationTypeToString(int32_t aType)
 {
   switch(aType)
   {
     case imgINotificationObserver::SIZE_AVAILABLE: return "SIZE_AVAILABLE";
     case imgINotificationObserver::FRAME_UPDATE: return "FRAME_UPDATE";
     case imgINotificationObserver::FRAME_COMPLETE: return "FRAME_COMPLETE";
--- a/image/imgRequestProxy.h
+++ b/image/imgRequestProxy.h
@@ -102,17 +102,16 @@ public:
   virtual void OnLoadComplete(bool aLastPart) override;
 
   // imgIOnloadBlocker methods:
   virtual void BlockOnload() override;
   virtual void UnblockOnload() override;
 
   // Other, internal-only methods:
   virtual void SetHasImage() override;
-  virtual void OnStartDecode() override;
 
   // Whether we want notifications from ProgressTracker to be deferred until
   // an event it has scheduled has been fired.
   virtual bool NotificationsDeferred() const override
   {
     return mDeferNotifications;
   }
   virtual void SetNotificationsDeferred(bool aDeferNotifications) override