Bug 1222596. If RasterImage::LookupFrame does (some) sync decoding and encouters an error we don't want to return the surface with an error. r=seth
authorTimothy Nikkel <tnikkel@gmail.com>
Fri, 01 Apr 2016 12:44:17 -0500
changeset 291288 5005b1604f269e2204260bdff4b26a92564e42f6
parent 291287 37ee5a42ddb506b6ecdf4d0119e96003db734a1d
child 291289 329a66b8e67c1c9367fbd496752e0488723aa79b
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
Bug 1222596. If RasterImage::LookupFrame does (some) sync decoding and encouters an error we don't want to return the surface with an error. r=seth If the sync decoding the LookupFrame does encounters an error it will set mError on the RasterImage, which LookupFrame callers check before calling LookupFrame. But they've called LookupFrame before the error was encountered, so we check if the frame has had Abort called on it to determine if we should return it at all. We only does this if one of the sync decode flags was passed in because IsAborted needs to get the imgFrame's monitor, so we don't want to block consumers that haven't asked for decoding.
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -363,16 +363,25 @@ RasterImage::LookupFrame(uint32_t aFrame
   // Sync decoding guarantees that we got the frame, but if it's owned by an
   // async decoder that's currently running, the contents of the frame may not
   // be available yet. Make sure we get everything.
   if (mHasSourceData && (aFlags & FLAG_SYNC_DECODE)) {
+  // If we could have done some decoding in this function we need to check if
+  // that decoding encountered an error and hence aborted the surface. We want
+  // to avoid calling IsAborted if we weren't passed any sync decode flag because
+  // IsAborted acquires the monitor for the imgFrame.
+    result.DrawableRef()->IsAborted()) {
+    return DrawableFrameRef();
+  }
   return Move(result.DrawableRef());
 RasterImage::GetCurrentFrameIndex() const
   if (mAnim) {
     return mAnim->GetCurrentAnimationFrameIndex();
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -993,16 +993,23 @@ imgFrame::Abort()
   mAborted = true;
   // Wake up anyone who's waiting.
+imgFrame::IsAborted() const
+  MonitorAutoLock lock(mMonitor);
+  return mAborted;
 imgFrame::IsFinished() const
   MonitorAutoLock lock(mMonitor);
   return mFinished;
 imgFrame::WaitUntilFinished() const
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -205,16 +205,21 @@ public:
    * completely decoded now, it never will be.
    * You must always call either Finish() or Abort() before releasing the last
    * RawAccessFrameRef pointing to an imgFrame.
   void Abort();
+   * Returns true if this imgFrame has been aborted.
+   */
+  bool IsAborted() const;
+  /**
    * Returns true if this imgFrame is completely decoded.
   bool IsFinished() const;
    * Blocks until this imgFrame is either completely decoded, or is marked as
    * aborted.