bettes@ and sgabriel@, any thoughts on this? Should we fix this by setting the intra-button spacing to 4pts?

https://docs.google.com/presentation/d/1EO7TOpIMJ7QHjaTVw9St-q6naKwtXX2TwzMirG5EsKY/edit#slide=id.g3232c09376_6_1271 gives a spec for this, I think we should implement that on all platforms.

In short: 8 DIP between groups of buttons and something adjacent like the window edge or the omnibox; 4 DIP between buttons within a group (toolbar buttons, browser actions).

Here's what I measure today in 1x Refresh non-touch:

Distance between hover highlights for "forward" and "reload": 4 px
Distance between reload highlight and omnibox: 8 px
Distance between omnibox and 1st extension action: 10 px
Distance between two extension actions: 4 px
Distance between last extension action and separator: 10 px
Distance between separator and avatar: 8 px
Distance between avatar and menu: 4 px

So the distance between adjacent icons seems consistent at 4, but the distance between icons and "other stuff" (the omnibox, separators) varies between 8 and 10.

This may represent a bugmorph.

Assigning to pbos@ to investigate the spacing issues

AFAICT from the spec, the main issue is that the browser actions area has 10 DIP on its edges instead of 8.  Other stuff looks correct.

I did not check touch.  For all I know that may differ (it has in the past).

FWIW this is because browser actions have 2dp of insets *inside them* to make 4dp spacing between them (instead of explicit spacing). The separator is placed 8dp away from the RHS item (using the correct spacing constant), but these insets effectively add 2dp of spacing on either side = 10dp total.

I'd like for this code use 4dp of explicit spacing between them instead of insets, but the piece of code is fairly complex as there's interactions between ToolbarActionsBar separate from BrowserActionsContainer (because of Cocoa). Once MacViews is launched and the Cocoa version is removed, I think we can simplify this code and address the above case more easily.

Deleted: browseractions.png 8.1 KB

	browseractions.png 8.1 KB View Download

I'll spitball this as 71 unless anyone thinks it's more urgent than that as I hope we can clean up Cocoa by then.

Taking this back as it should be fixed by reducing the effective size of ToolbarActionView + adding element padding.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82848abbf09b0cf470a2f06a617fe681d87f7f8a

commit 82848abbf09b0cf470a2f06a617fe681d87f7f8a
Author: Peter Boström <pbos@chromium.org>
Date: Fri Jan 04 18:00:33 2019

Decrease extension button size + add spacing

This makes ToolbarActionView the same size as other toolbar buttons
instead of increasing its internal size with 4dp to accomodate for
padding between items.

This reduces the BrowserActionsContainer height by 4dp (matching its
current visual height) so it should no longer be able to push the
toolbar height. It also fixes some polish where the spacing left and
right of the browser actions were inadvertedly 10dp instead of 8dp.

Bug:  chromium:831393 ,  chromium:916241 ,  chromium:916462 
Change-Id: I63e5413ce187f173a5e35b40a3ec8f02a68fa09c
Reviewed-on: https://chromium-review.googlesource.com/c/1394870
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619999}
[modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/layout_constants.cc
[modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/toolbar/toolbar_actions_bar.cc
[modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc
[modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/views/toolbar/browser_actions_container.cc
[modify] https://crrev.com/82848abbf09b0cf470a2f06a617fe681d87f7f8a/chrome/browser/ui/views/toolbar/browser_actions_container.h