[Webkit-unassigned] [Bug 26691] Cleanup: Move focusRingColor to RenderTheme

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26691


eric at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31945|review?                     |review-
               Flag|                            |




------- Comment #4 from eric at webkit.org  2009-06-26 13:53 PDT -------
(From update of attachment 31945)
Style error:
 175 Color RenderThemeChromiumMac::focusRingColor() const {

I worry that we're no longer caching the focus ring color.
 425             color = convertNSColorToColor([NSColor
keyboardFocusIndicatorColor]);
could be expensive to call every time if it goes down the "allocate a bitmap"
case.

Also, are you sure that keyboardFocusIndicatorColor will respect focus ring
changes?  i.e. have you tested changing the focus ring color from System
Prefernces while Safari is open and verified that the focus ring colors still
update?  (Actually they probably don't update until they're redrawn.)

I take it nsColor(color) is no longer used?

In general this looks fine.  A PLT run would tell us if the focus ring color
caching has caused a regression.

r- for the possible performance regression.  We need to investigate that
keyboardFocusIndicatorColor properly updates on focus color change in the
Appearance control panel.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.