Bug 580669 - Mac Menu icon crash with enabled image discarding.r=joe

modules/libpr0n/public/imgIContainer.idl

@@ -106,7 +106,9 @@ interface imgIContainer : nsISupports
    * FLAG_NONE: Lack of flags
    *
    * FLAG_SYNC_DECODE: Forces synchronous/non-progressive decode of all
-   * available data before the call returns.
+   * available data before the call returns. It is an error to pass this flag
+   * from a call stack that originates in a decoder (ie, from a decoder
+   * observer event).
    */
 
   const long FLAG_NONE            = 0x0;

modules/libpr0n/src/imgContainer.cpp

@@ -289,6 +289,10 @@ NS_IMETHODIMP imgContainer::ExtractFrame(PRUint32 aWhichFrame,
   if (mError)
     return NS_ERROR_FAILURE;
 
+  // Disallowed in the API
+  if (mInDecoder && (aFlags & imgIContainer::FLAG_SYNC_DECODE))
+    return NS_ERROR_FAILURE;
+
   // Make a new container. This should switch to another class with bug 505959.
   nsRefPtr<imgContainer> img(new imgContainer());
   NS_ENSURE_TRUE(img, NS_ERROR_OUT_OF_MEMORY);
@@ -531,6 +535,10 @@ NS_IMETHODIMP imgContainer::CopyFrame(PRUint32 aWhichFrame,
   if (mError)
     return NS_ERROR_FAILURE;
 
+  // Disallowed in the API
+  if (mInDecoder && (aFlags & imgIContainer::FLAG_SYNC_DECODE))
+    return NS_ERROR_FAILURE;
+
   nsresult rv;
 
   // If requested, synchronously flush any data we have lying around to the decoder
@@ -584,6 +592,10 @@ NS_IMETHODIMP imgContainer::GetFrame(PRUint32 aWhichFrame,
   if (mError)
     return NS_ERROR_FAILURE;
 
+  // Disallowed in the API
+  if (mInDecoder && (aFlags & imgIContainer::FLAG_SYNC_DECODE))
+    return NS_ERROR_FAILURE;
+
   nsresult rv = NS_OK;
 
   // If the caller requested a synchronous decode, do it
@@ -2334,12 +2346,9 @@ imgContainer::SyncDecode()
 
   // We really have no good way of forcing a synchronous decode if we're being
   // called in a re-entrant manner (ie, from an event listener fired by a
-  // decoder), because the decoding machinery is already tied up. The best we
-  // can do is assert that it doesn't happen for debug builds (if it does, we
-  // need to rethink the layout code that does it), and fail for release builds.
+  // decoder), because the decoding machinery is already tied up. We thus explicitly
+  // disallow this type of call in the API, and check for it in API methods.
   NS_ABORT_IF_FALSE(!mInDecoder, "Yikes, forcing sync in reentrant call!");
-  if (mInDecoder)
-    return NS_ERROR_FAILURE;
 
   // If we have a header-only decoder open, shut it down
   if (mDecoder && (mDecoderFlags & imgIDecoder::DECODER_FLAG_HEADERONLY)) {
@@ -2386,6 +2395,10 @@ NS_IMETHODIMP imgContainer::Draw(gfxContext *aContext,
   if (mError)
     return NS_ERROR_FAILURE;
 
+  // Disallowed in the API
+  if (mInDecoder && (aFlags & imgIContainer::FLAG_SYNC_DECODE))
+    return NS_ERROR_FAILURE;
+
   NS_ENSURE_ARG_POINTER(aContext);
 
   // If a synchronous draw is requested, flush anything that might be sitting around

widget/src/cocoa/nsMenuItemIconX.mm

@@ -443,7 +443,7 @@ nsMenuItemIconX::OnStopFrame(imgIRequest*    aRequest,
   
   nsRefPtr<gfxImageSurface> frame;
   nsresult rv = imageContainer->CopyFrame(  imgIContainer::FRAME_CURRENT,
-                                            imgIContainer::FLAG_SYNC_DECODE,
+                                            imgIContainer::FLAG_NONE,
                                             getter_AddRefs(frame));
   if (NS_FAILED(rv) || !frame) {
     [mNativeMenuItem setImage:nil];