Fix three small bugs in BrowserActionsContainer

Fix three small bugs in BrowserActionsContainer

There are three subtle bugs that appear when using the
redesign that this fixes:
1. You can drag the container to be only 3 pixels wide,
   but then it expands back to 6 pixels wide.
2. Expanding the container enough to see an icon won't
   necessarily make the container "stick" with the icon
   visible.
3. The highlight painter should still be initialized for
   the main container with the redesign.

Also tweak the function signature of IconCountToWidth(),
and have it determine the need for a chevron.

Committed: https://crrev.com/e544ac94dcfcd3ef43f06d412a4a56183080de19
Cr-Commit-Position: refs/heads/master@{#297946}

[Devlin, 2014-10-02 21:14:14]

rdevlin.cronin@chromium.org changed reviewers:
+ pkasting@chromium.org

[Devlin, 2014-10-02 21:14:22]

Patchset #2 (id:20001) has been deleted

[Devlin, 2014-10-02 21:14:27]

Patchset #1 (id:1) has been deleted

[Devlin, 2014-10-02 21:44:14]

Peter, mind taking a quick look?

[Peter Kasting, 2014-10-02 21:49:57]

LGTM

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/toolbar/browser_actions_container.cc (right):

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:363: // Subtract
off the trailing padding.
Should we be doing this before we compare IconCountToWidth() to this value
above?

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:918: // overflow.
Nit: Put this comment above the conditional instead of inside.

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:952: chevron_ &&
icons < static_cast<int>(browser_action_views_.size());
Nit: I think casting icons to size_t is probably better, since we've already
ensured it's non-negative.

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:955: int icons_size
= icons == 0 ? 0 : ((icons * IconWidth(true)) - kItemSpacing);
Nit: Up to you, but I think the parens around the condition that the previous
code had are more readable.

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/toolbar/browser_actions_container.h (right):

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.h:312: // icons and
we're not using the redesign). For convenience, callers can set
These two edited comments appear to be the only references to "the redesign". 
Unless you intend to remove these references within the next week or two, it
would probably be better to be more clear about what you mean, stated in terms
of variables/methods/classes this file already refers to.

[Devlin, 2014-10-02 22:11:06]

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/toolbar/browser_actions_container.cc (right):

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:363: // Subtract
off the trailing padding.
On 2014/10/02 21:49:56, Peter Kasting wrote:
> Should we be doing this before we compare IconCountToWidth() to this value
> above?

Nope.  IconCountToWidth() will be the full width of the container, so if we
subtract it off there, then max_x will always be less than IconCountToWidth(-1)
(max_x is intended to be the max_x for an icon, not the max_x for the
container).

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:918: // overflow.
On 2014/10/02 21:49:56, Peter Kasting wrote:
> Nit: Put this comment above the conditional instead of inside.

Done.

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:952: chevron_ &&
icons < static_cast<int>(browser_action_views_.size());
On 2014/10/02 21:49:56, Peter Kasting wrote:
> Nit: I think casting icons to size_t is probably better, since we've already
> ensured it's non-negative.

Done.

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:955: int icons_size
= icons == 0 ? 0 : ((icons * IconWidth(true)) - kItemSpacing);
On 2014/10/02 21:49:56, Peter Kasting wrote:
> Nit: Up to you, but I think the parens around the condition that the previous
> code had are more readable.

Happy to defer to OWNER's preference. :) Done.

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/toolbar/browser_actions_container.h (right):

https://codereview.chromium.org/602613003/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/toolbar/browser_actions_container.h:312: // icons and
we're not using the redesign). For convenience, callers can set
On 2014/10/02 21:49:56, Peter Kasting wrote:
> These two edited comments appear to be the only references to "the redesign". 
> Unless you intend to remove these references within the next week or two, it
> would probably be better to be more clear about what you mean, stated in terms
> of variables/methods/classes this file already refers to.

Good point. Done.

[Devlin, 2014-10-02 22:11:14]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2014-10-02 22:13:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/602613003/60001

[commit-bot: I haz the power, 2014-10-02 23:24:47]

Committed patchset #2 (id:60001) as a34e3ab31c9bc2d4f02f05078d6d0ecaa1e6565b

[commit-bot: I haz the power, 2014-10-02 23:26:14]

Patchset 2 (id:??) landed as
https://crrev.com/e544ac94dcfcd3ef43f06d412a4a56183080de19
Cr-Commit-Position: refs/heads/master@{#297946}