Stop nsMenuItemIconX object from possibly outliving its nsMenuObjectX owner. b=499600 r=josh

widget/src/cocoa/nsMenuItemIconX.h

@@ -79,13 +79,19 @@ public:
   // icon.  The request may not complete until after LoadIcon returns.
   nsresult LoadIcon(nsIURI* aIconURI);
 
+  // Unless we take precautions, we may outlive the object that created us
+  // (mMenuObject, which owns our native menu item (mNativeMenuItem)).
+  // Destroy() should be called from mMenuObject's destructor to prevent
+  // this from happening.  See bug 499600.
+  void Destroy();
+
 protected:
   nsCOMPtr<nsIContent>  mContent;
   nsCOMPtr<imgIRequest> mIconRequest;
-  nsMenuObjectX*        mMenuObject;
+  nsMenuObjectX*        mMenuObject; // [weak]
   PRPackedBool          mLoadedIcon;
   PRPackedBool          mSetIcon;
-  NSMenuItem*           mNativeMenuItem;
+  NSMenuItem*           mNativeMenuItem; // [weak]
 };
 
 #endif // nsMenuItemIconX_h_

widget/src/cocoa/nsMenuItemIconX.mm

@@ -97,6 +97,19 @@ nsMenuItemIconX::~nsMenuItemIconX()
     mIconRequest->CancelAndForgetObserver(NS_BINDING_ABORTED);
 }
 
+// Called from mMenuObjectX's destructor, to prevent us from outliving it
+// (as might otherwise happen if calls to our imgIDecoderObserver methods
+// are still outstanding).  mMenuObjectX owns our nNativeMenuItem.
+void nsMenuItemIconX::Destroy()
+{
+  if (mIconRequest) {
+    mIconRequest->CancelAndForgetObserver(NS_BINDING_ABORTED);
+    mIconRequest = nsnull;
+  }
+  mMenuObject = nsnull;
+  mNativeMenuItem = nil;
+}
+
 nsresult
 nsMenuItemIconX::SetupIcon()
 {
@@ -126,6 +139,9 @@ nsMenuItemIconX::SetupIcon()
 nsresult
 nsMenuItemIconX::GetIconURI(nsIURI** aIconURI)
 {
+  if (!mMenuObject)
+    return NS_ERROR_FAILURE;
+
   // Mac native menu items support having both a checkmark and an icon
   // simultaneously, but this is unheard of in the cross-platform toolkit,
   // seemingly because the win32 theme is unable to cope with both at once.
@@ -320,6 +336,8 @@ nsMenuItemIconX::OnStopFrame(imgIRequest*    aRequest,
   if (mLoadedIcon)
     return NS_OK;
 
+  if (!mNativeMenuItem) return NS_ERROR_FAILURE;
+
   nsCOMPtr<gfxIImageFrame> frame = aFrame;
   nsCOMPtr<nsIImage> image = do_GetInterface(frame);
   if (!image) return NS_ERROR_FAILURE;

widget/src/cocoa/nsMenuItemX.mm

@@ -72,6 +72,10 @@ nsMenuItemX::~nsMenuItemX()
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
+  // Prevent the icon object from outliving us.
+  if (mIcon)
+    mIcon->Destroy();
+
   // autorelease the native menu item so that anything else happening to this
   // object happens before the native menu item actually dies
   [mNativeMenuItem autorelease];

widget/src/cocoa/nsMenuX.mm

@@ -117,6 +117,10 @@ nsMenuX::~nsMenuX()
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
+  // Prevent the icon object from outliving us.
+  if (mIcon)
+    mIcon->Destroy();
+
   RemoveAll();
 
   [mNativeMenu setDelegate:nil];