402627 - "WARNING: Calling SetWindowTitlebarColor on window that isn't of the ToolbarWindow class."

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Jesse Ruderman]

This warning appears when I start Firefox:

WARNING: Calling SetWindowTitlebarColor on window that isn't of the ToolbarWindow class.: file /Users/jruderman/trunk/mozilla/widget/src/cocoa/nsCocoaWindow.mm, line 1089

I'm guessing this started happening with the landing of bug 402125.

Comment 1

[Colin Barrett [:cbarrett]]

Can you break on that line and backtrace? 

Comment 2

[Jesse Ruderman]

Attachment: stack trace

Comment 3

[Colin Barrett [:cbarrett]]

This is caused by someone trying to set the titlebar color of the hidden window.

They should probably stop doing that ;)

Comment 4

[Asaf Romano (gone)]

I think you better fail silently, otherwise we'd have to distinguish the hidden window any anything which might be opened as a sheet using pure css selectors...

Comment 5

[Colin Barrett [:cbarrett]]

Attachment: fix v1.0

If we're the hidden window, we should fail silently according to Mano.

Comment 6

[Josh Aas]

Comment on attachment 287616 [details] [diff] [review]
fix v1.0

+  // Fail silently for the hidden window.
+  if (SameCOMIdentity(GetHiddenWindowWidget(), (nsIWidget*)this))
+    return NS_ERROR_FAILURE;

I'm not sure raising an error here is failing silently, and in any case the comment itself is saying nothing new about obvious code so it isn't necessary. Why return an error instead of NS_OK? I'm not sure which is better, error or NS_OK.

Comment 7

[Colin Barrett [:cbarrett]]

Attachment: fix v1.1

This should be a little clearer.

Comment 8

[Josh Aas]

Comment on attachment 287701 [details] [diff] [review]
fix v1.1

+  nsIWidget* hiddenWindowWidgetNoCOMPtr = GetHiddenWindowWidget();
   return static_cast<nsCocoaWindow*>(hiddenWindowWidgetNoCOMPtr)->GetMenuBar();  

If GetHiddenWindowWidget return null this will crash.

Comment 9

[Colin Barrett [:cbarrett]]

Attachment: fix v1.2

Thanks, fixed.

Comment 10

[Josh Aas]

Comment on attachment 289100 [details] [diff] [review]
fix v1.2

>@@ -1521,6 +1533,7 @@ void patternDraw(void* aInfo, CGContextR
>     CGColorSpaceRelease(colorSpace);
>     CGFunctionRelease(function);
>     CGContextDrawShading(aContext, shading);
>+    CGShadingRelease(shading);

This won't apply and it has nothing to do with this bug.

Comment 11

[Håkan Waara]

Comment on attachment 289100 [details] [diff] [review]
fix v1.2

>+    // Don't output a warning for the hidden window.
>+    if (!SameCOMIdentity(GetHiddenWindowWidget(), (nsIWidget*)this))
>+      NS_WARNING("Calling SetWindowTitlebarColor on window that isn't of the ToolbarWindow class.");
>     return NS_ERROR_FAILURE;
>   }

Since NS_WARNING will only apply in a debug build, I think you should also make the if-statement only run on debug builds by putting it inside #if DEBUG.

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

NS_WARN_IF_FALSE

Comment 13

[Colin Barrett [:cbarrett]]

Attachment: fix v1.3

Should apply cleanly now that I'm not working from an old source tree. Using NS_WARN_IF_FALSE as well now.

Comment 14

[Colin Barrett [:cbarrett]]

Comment on attachment 290286 [details] [diff] [review]
fix v1.3

Want this to land for 1.9. It's silencing a spurious warning during start up, and helps make automated testing easier and real warnings stand out better.

Comment 15

[Colin Barrett [:cbarrett]]

Checking in widget/src/cocoa/nsCocoaWindow.mm;
/cvsroot/mozilla/widget/src/cocoa/nsCocoaWindow.mm,v  <--  nsCocoaWindow.mm
new revision: 1.120; previous revision: 1.119
done