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 331090 5005b1604f269e2204260bdff4b26a92564e42f6
parent 331089 37ee5a42ddb506b6ecdf4d0119e96003db734a1d
child 331091 329a66b8e67c1c9367fbd496752e0488723aa79b
push id1146
push userCallek@gmail.com
push dateMon, 25 Jul 2016 16:35:44 +0000
treeherdermozilla-release@a55778f9cd5a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs1222596
milestone48.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 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.
image/RasterImage.cpp
image/imgFrame.cpp
image/imgFrame.h
--- 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)) {
     result.DrawableRef()->WaitUntilFinished();
   }
 
+  // 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.
+  if (aFlags & (FLAG_SYNC_DECODE | FLAG_SYNC_DECODE_IF_FAST) &&
+    result.DrawableRef()->IsAborted()) {
+    return DrawableFrameRef();
+  }
+
   return Move(result.DrawableRef());
 }
 
 uint32_t
 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.
   mMonitor.NotifyAll();
 }
 
 bool
+imgFrame::IsAborted() const
+{
+  MonitorAutoLock lock(mMonitor);
+  return mAborted;
+}
+
+bool
 imgFrame::IsFinished() const
 {
   MonitorAutoLock lock(mMonitor);
   return mFinished;
 }
 
 void
 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.
    *