[Mac] Fix bugs in resizing the browser actions area next to the omnibox.

[Mac] Fix bugs in resizing the browser actions area next to the omnibox.

- Consistently bounds-check the new width of the BrowserActionsContainerView
  during dragging. Fixes crbug/681004.

- Don't trust -[NSEvent deltaX/deltaY], it's inaccurate over time:
  http://stackoverflow.com/q/30324935/84745

- Get rid of kMinimumContainerWidth and another magic number (3.0) in
  browser_actions_controller.mm, which were both slightly wrong. This fixes the
  1px jump when you let go of the resizing handle at the minimum width.

- Refactor and de-dupe. This fixes animation when you stop dragging in between
  icons. The new size was getting applied twice, which aborted the animation.

BUG=681004
TEST=On a Mac, install several extensions.
- Try resizing the browser actions area.
- Try resizing it quickly (by moving the mouse back and forth quickly) to make
  sure you can't make it bigger than it needs to be for the number of icons,
  and can't make it so small that the omnibox overlaps the wrench menu.
- Make sure that, when you let go of the mouse button with the browser actions
  area fully open or closed, nothing changes (e.g. the omnibox doesn't grow or
  shrink by a pixel).
- Make sure that, when you resize the browser actions area and let go with only
  some extension icons visible, it smoothly animates to a size that doesn't
  leave any icon partially visible.

Review-Url: https://codereview.chromium.org/2629723005
Cr-Commit-Position: refs/heads/master@{#444847}
Committed: https://chromium.googlesource.com/chromium/src/+/ae80f9c27eb768d8a53f780c16af4d024e454659

[Sidney San Martín, 2017-01-13 22:06:04]

The CQ bit was checked by sdy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 22:06:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sidney San Martín, 2017-01-13 22:09:23]

The CQ bit was unchecked by sdy@chromium.org

[Sidney San Martín, 2017-01-13 22:11:09]

Patchset #2 (id:20001) has been deleted

[Sidney San Martín, 2017-01-13 22:14:11]

sdy@chromium.org changed reviewers:
+ rsesek@chromium.org

[Sidney San Martín, 2017-01-13 22:14:25]

The CQ bit was checked by sdy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 22:14:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Robert Sesek, 2017-01-13 22:53:59]

LGTM % comment

https://codereview.chromium.org/2629723005/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
(right):

https://codereview.chromium.org/2629723005/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm:12:
#include "chrome/browser/ui/layout_constants.h"
I can't find any other uses of layout_constants.h in Cocoa code, so I'm not sure
it's the right thing to be using here. But it looks like it may have been in the
past? (E.g., https://codereview.chromium.org/1770573002).

+shrike for his opinion

[Robert Sesek, 2017-01-13 22:54:22]

rsesek@chromium.org changed reviewers:
+ shrike@chromium.org

[Robert Sesek, 2017-01-13 22:54:23]

+shrike for real

[Sidney San Martín, 2017-01-13 23:03:29]

https://codereview.chromium.org/2629723005/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm
(right):

https://codereview.chromium.org/2629723005/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_container_view.mm:12:
#include "chrome/browser/ui/layout_constants.h"
On 2017/01/13 22:53:59, Robert Sesek wrote:
> I can't find any other uses of layout_constants.h in Cocoa code, so I'm not
sure
> it's the right thing to be using here. But it looks like it may have been in
the
> past? (E.g., https://codereview.chromium.org/1770573002).
> 
> +shrike for his opinion

I may have a better option, hang tight.

[commit-bot: I haz the power, 2017-01-14 00:02:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-14 00:02:17]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Sidney San Martín, 2017-01-19 06:15:03]

The CQ bit was checked by sdy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 06:15:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sidney San Martín, 2017-01-19 06:55:34]

Description was changed from

==========
[Mac] Fix bugs in resizing the browser actions area next to the omnibox.

- Consistently use newWidth, which is bounds checked, and delete dX,
  which wasn't. This fixes crbug/681004.

- Don't trust -[NSEvent deltaX/deltaY], it isn't accurate over time:
  http://stackoverflow.com/q/30324935/84745

- Delete kMinimumContainerWidth. TOOLBAR_STANDARD_SPACING, which is 1px
  bigger, was used in other places as the container's minimum width, so
  this fixes the 1px jump when you let go of the resizing handle at the
  minimum width.

BUG=681004
==========

to

==========
[Mac] Fix bugs in resizing the browser actions area next to the omnibox.

- Consistently bounds-check the new width of the BrowserActionsContainerView
  during dragging. Fixes crbug/681004.

- Don't trust -[NSEvent deltaX/deltaY], it's inaccurate over time:
  http://stackoverflow.com/q/30324935/84745

- Get rid of kMinimumContainerWidth and another magic number (3.0) in
  browser_actions_controller.mm, which were both slightly wrong. This fixes the
  1px jump when you let go of the resizing handle at the minimum width.

- Refactor and de-dupe. This fixes animation when you stop dragging in between
  icons. The new size was getting applied twice, which aborted the animation.

BUG=681004
TEST=On a Mac, install several extensions.
- Try resizing the browser actions area.
- Try resizing it quickly (by moving the mouse back and forth quickly) to make
  sure you can't make it bigger than it needs to be for the number of icons,
  and can't make it so small that the omnibox overlaps the wrench menu.
- Make sure that, when you let go of the mouse button with the browser actions
  area fully open or closed, nothing changes (e.g. the omnibox doesn't grow or
  shrink by a pixel).
- Make sure that, when you resize the browser actions area and let go with only
  some extension icons visible, it smoothly animates to a size that doesn't
  leave any icon partially visible.
==========

[commit-bot: I haz the power, 2017-01-19 07:03:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 07:03:18]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Sidney San Martín, 2017-01-19 08:00:05]

The CQ bit was checked by sdy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 08:00:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sidney San Martín, 2017-01-19 08:04:01]

This new patch set does some refactoring (much more could be done) which gets
rid of the need for the include. As a bonus, it fixes the animation when you let
go with an icon partially visible. Take a look when you have a chance — it's
bigger, but removes much more than it adds.

[commit-bot: I haz the power, 2017-01-19 08:32:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 08:32:23]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2017-01-19 20:55:42]

This does seem simpler. LGTM % two nits

https://codereview.chromium.org/2629723005/diff/80001/chrome/browser/ui/cocoa...
File
chrome/browser/ui/cocoa/extensions/browser_actions_container_view_unittest.mm
(right):

https://codereview.chromium.org/2629723005/diff/80001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_container_view_unittest.mm:34:
[view_ setMinWidth: kMinimumContainerWidth];
nit: no space after :

https://codereview.chromium.org/2629723005/diff/80001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/browser_actions_controller.h (right):

https://codereview.chromium.org/2629723005/diff/80001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_controller.h:64: 
nit: no blank line after

[Sidney San Martín, 2017-01-19 21:04:36]

The CQ bit was checked by sdy@chromium.org

[Sidney San Martín, 2017-01-19 21:04:37]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/2629723005/#ps100001
(title: "Format")

[commit-bot: I haz the power, 2017-01-19 21:05:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 21:29:38]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484859876637500,
"parent_rev": "1c7af89ea2d8e6e5e31be9d1f7bfa6c306584f5d", "commit_rev":
"ae80f9c27eb768d8a53f780c16af4d024e454659"}

[commit-bot: I haz the power, 2017-01-19 21:30:14]

Description was changed from

==========
[Mac] Fix bugs in resizing the browser actions area next to the omnibox.

- Consistently bounds-check the new width of the BrowserActionsContainerView
  during dragging. Fixes crbug/681004.

- Don't trust -[NSEvent deltaX/deltaY], it's inaccurate over time:
  http://stackoverflow.com/q/30324935/84745

- Get rid of kMinimumContainerWidth and another magic number (3.0) in
  browser_actions_controller.mm, which were both slightly wrong. This fixes the
  1px jump when you let go of the resizing handle at the minimum width.

- Refactor and de-dupe. This fixes animation when you stop dragging in between
  icons. The new size was getting applied twice, which aborted the animation.

BUG=681004
TEST=On a Mac, install several extensions.
- Try resizing the browser actions area.
- Try resizing it quickly (by moving the mouse back and forth quickly) to make
  sure you can't make it bigger than it needs to be for the number of icons,
  and can't make it so small that the omnibox overlaps the wrench menu.
- Make sure that, when you let go of the mouse button with the browser actions
  area fully open or closed, nothing changes (e.g. the omnibox doesn't grow or
  shrink by a pixel).
- Make sure that, when you resize the browser actions area and let go with only
  some extension icons visible, it smoothly animates to a size that doesn't
  leave any icon partially visible.
==========

to

==========
[Mac] Fix bugs in resizing the browser actions area next to the omnibox.

- Consistently bounds-check the new width of the BrowserActionsContainerView
  during dragging. Fixes crbug/681004.

- Don't trust -[NSEvent deltaX/deltaY], it's inaccurate over time:
  http://stackoverflow.com/q/30324935/84745

- Get rid of kMinimumContainerWidth and another magic number (3.0) in
  browser_actions_controller.mm, which were both slightly wrong. This fixes the
  1px jump when you let go of the resizing handle at the minimum width.

- Refactor and de-dupe. This fixes animation when you stop dragging in between
  icons. The new size was getting applied twice, which aborted the animation.

BUG=681004
TEST=On a Mac, install several extensions.
- Try resizing the browser actions area.
- Try resizing it quickly (by moving the mouse back and forth quickly) to make
  sure you can't make it bigger than it needs to be for the number of icons,
  and can't make it so small that the omnibox overlaps the wrench menu.
- Make sure that, when you let go of the mouse button with the browser actions
  area fully open or closed, nothing changes (e.g. the omnibox doesn't grow or
  shrink by a pixel).
- Make sure that, when you resize the browser actions area and let go with only
  some extension icons visible, it smoothly animates to a size that doesn't
  leave any icon partially visible.

Review-Url: https://codereview.chromium.org/2629723005
Cr-Commit-Position: refs/heads/master@{#444847}
Committed:
https://chromium.googlesource.com/chromium/src/+/ae80f9c27eb768d8a53f780c16af...
==========

[commit-bot: I haz the power, 2017-01-19 21:30:16]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/ae80f9c27eb768d8a53f780c16af...