839029 - default button in dialog window is continuously repainted even when disabled

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Jonathan Kew [:jfkthame]]

In dialog windows on OS X, the blue "default" button (if any) pulsates slightly, which entails continuously repainting it.

However, it turns out that we continuously repaint the button -even if it is disabled-, in which case it appears "flat" grey and does not visually pulsate. That's unnecessary, and it means that leaving such a dialog on screen, even when the window is inactive and/or the button disabled, will add significantly to Firefox's cpu and battery usage.

STR:
* In about:config, set nglayout.debug.paint_flashing = true
* Open Preferences, go to the Content panel, click the Exceptions button to the right of Block Pop-up Windows.

Note that although the "Allow" button in the dialog is disabled as long as no text is entered in the Address of Website field, it is being continuously repainted.

This persists even if another window or a different application is brought to the foreground.

This causes Firefox to hover around 3.5-4% CPU usage according to Activity Monitor on my MacBook, whereas without such a dialog open, it spends much of its time at 0.5-1%.

Fortunately, we don't have all that many windows with "default" buttons, but still, this seems like an inefficiency we ought to avoid. In addition to Firefox's built-in dialogs such as the Exceptions ones in Preferences, the same behavior applies in dialogs presented by add-ons.

Comment 1

[André Reinald]

Attachment: Avoid refreshing disabled or inactive default button

With thanks to Markus for his hint on how to best fix this. My first patch was much worse.

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 721140 [details] [diff] [review]
Avoid refreshing disabled or inactive default button

Looks good to me.
I'm not sure about the current review policies in widget:cocoa, so I'm requesting review from Josh, too.

Comment 3

[Josh Aas]

Comment on attachment 721140 [details] [diff] [review]
Avoid refreshing disabled or inactive default button

Review of attachment 721140 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Current review policy for Cocoa widgets is that a review is required from an owner or peer. Since mstange is a peer, my review is technically unnecessary here.

Comment 4

[Markus Stange [:mstange]]

OK, thanks

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1557651f08a

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b1557651f08a