Dynamically compute tab/frame separator color.
For reference, this color is overlaid atop the frame/toolbar when outlining tabs
and for outlines/shadows when drawing the new tab button. It's either white or
black with a varying (but usually 0x40) alpha value.
Computing this is surprisingly complicated, beause we're trying to contrast with
two different colors simultaneously (tab and frame), and we also want to set our
magic numbers so as to achieve the colors from the design specs in the default
theme.
After quite a bit of thought, I elected to use a mechanism that defaults to
moving the frame away from the tab luminance; that is, if the tab is "brighter"
than the frame, we make the separator darken the frame, and if the tab is
darker, we try to lighten the frame. If the frame is already so dark or light
that the result has too low of contrast with the frame color, we reverse
direction. ("Too low" here is a lot lower than I'd like, but any higher and
we'd end up using light separators for the default theme in incognito mode,
which I think looks good but Sebastien dislikes.) In the case where we reversed
direction, we have to worry that the result of all this won't contrast enough
with the tab; in that case, we push up the alpha value so the result contrasts
enough with the tab as well. This last computation is the most expensive
because of how I chose to make it behave, but I think the behavior I selected
(too complicated to explain here, see code/comments for details) will feel more
consistent than any of the simpler methods I considered.
Because computing the separator color can be expensive (every call to
GetRelativeLuminance() can involve floating-point exponentiation among other
things, and in the worst case the color computation computes the desired color
via a 7-iteration loop), I elected to cache the computed value in a map. This
might be a case of premature optimization, but in debugging it looked like this
color could be requested frequently, and I didn't want to risk performance
problems.
Even this choice presented options. I used a simple map that I never prune
entries from; at worst, we'll add up to 2 entries per distinct theme the user
switches to while running, which didn't seem too bad. I considered instead
using base::MRUCache, which would let me cap the size. I also considered just
keeping a couple member structs containing the relevant information for the
normal and incognito color schemes, but this generally seemed like it ended up
as more code than the other routes. None of these options seems wildly better
or worse than the others; I'm willing to change course in the face of violent
opinion :)
Finally, because the alpha value of the separator can now vary, I converted the
code in tab_strip.cc that set it to fixed values to instead use scaling
multipliers. These will compute the same values as before when the separator
has its default (0x40) alpha, but in the case where we've computed some larger
alpha, scaling proportionally seemed like the best thing to do. I used a
saturated_cast in one place where I wasn't sure the result was guaranteed to
stay <= 255.
BUG=585470
TEST=See bug comment 0
Review URL: https://codereview.chromium.org/1785613004
Cr-Commit-Position: refs/heads/master@{#381617}
5 files changed