Some more pre-material cleanups

chrome/browser/ui/layout_constants.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/layout_constants.h"
 
 #include "base/logging.h"
 #include "build/build_config.h"
 #include "ui/base/material_design/material_design_controller.h"
 
 int GetLayoutConstant(LayoutConstant constant) {
-  const int kFindBarVerticalOffset[] = {1, 6, 6};
-  const int kLocationBarBorderThickness[] = {2, 1, 1};
-  const int kLocationBarBubbleFontVerticalPadding[] = {1, 2, 4};
-  const int kLocationBarBubbleHorizontalPadding[] = {1, 0, 0};
-  const int kLocationBarBubbleVerticalPadding[] = {1, 3, 3};
-  const int kLocationBarBubbleAnchorVerticalInset[] = {5, 6, 8};
-  const int kLocationBarHeight[] = {27, 28, 32};
-  const int kLocationBarHorizontalPadding[] = {3, 6, 6};
-  const int kLocationBarVerticalPadding[] = {0, 1, 1};
-  const int kOmniboxFontPixelSize[] = {16, 14, 14};
-  const int kTabFaviconTitleSpacing[] = {4, 6, 6};
-  const int kTabHeight[] = {29, 29, 33};
-  const int kTabPinnedContentWidth[] = {25, 23, 23};
+  const int kFindBarVerticalOffset[] = {6, 6};
+  const int kLocationBarBorderThickness[] = {1, 1};
+  const int kLocationBarBubbleFontVerticalPadding[] = {2, 4};

[Evan Stade, 2016/10/24 19:09:40]

what do we think should happen to the layout constants here, especially the ones
that are the same in normal and hybrid? Or the ones like kFindBarVerticalOffset
which are only used in one place, i.e. browser_view_layout.cc

[Peter Kasting, 2016/10/24 23:49:56]

Things that are only used in one place, whether the same or different between
normal and hybrid, I would define in that file.

Things that are the same between normal and hybrid, but used in multiple places,
I would define publicly in some relevant class as a public constant.

Not sure how many things are "different between normal and hybrid, and used in
multiple places".  For now I'd leave those here.

[Evan Stade, 2016/10/25 16:22:57]

this plan soudns fine, but for now (for the purposes of smallish, atomic
changes) I intend to leave everything here

[Peter Kasting, 2016/10/27 22:16:37]

I'm cool with that.  You've been cleaning all the rest of the MD stuff up, so I
assume you will take care of it eventually.

+  const int kLocationBarBubbleHorizontalPadding[] = {0, 0};

[Evan Stade, 2016/10/24 19:09:41]

this one definitely seems like it should just die

[Peter Kasting, 2016/10/24 23:49:56]

Yep.

[Evan Stade, 2016/10/25 16:22:56]

Done.

+  const int kLocationBarBubbleVerticalPadding[] = {3, 3};
+  const int kLocationBarBubbleAnchorVerticalInset[] = {6, 8};
+  const int kLocationBarHeight[] = {28, 32};
+  const int kLocationBarHorizontalPadding[] = {6, 6};
+  const int kLocationBarVerticalPadding[] = {1, 1};
+  const int kOmniboxFontPixelSize[] = {14, 14};
+  const int kTabFaviconTitleSpacing[] = {6, 6};
+  const int kTabHeight[] = {29, 33};
+  const int kTabPinnedContentWidth[] = {23, 23};
 #if defined(OS_MACOSX)

[Evan Stade, 2016/10/24 19:09:40]

the mac/non mac distinction is no longer important

[Peter Kasting, 2016/10/24 23:49:56]

Yup, rip out

[Evan Stade, 2016/10/25 16:22:57]

Done.

-  const int kTabstripNewTabButtonOverlap[] = {8, 5, 6};
-  const int kTabstripTabOverlap[] = {19, 16, 16};
+  const int kTabstripNewTabButtonOverlap[] = {5, 6};
+  const int kTabstripTabOverlap[] = {16, 16};
 #else
-  const int kTabstripNewTabButtonOverlap[] = {11, 5, 6};
-  const int kTabstripTabOverlap[] = {26, 16, 16};
+  const int kTabstripNewTabButtonOverlap[] = {5, 6};
+  const int kTabstripTabOverlap[] = {16, 16};
 #endif
-  const int kToolbarStandardSpacing[] = {3, 4, 8};
-  const int kToolbarElementPadding[] = {0, 0, 8};
-  const int kToolbarLocationBarRightPadding[] = {0, 4, 8};
+  const int kToolbarStandardSpacing[] = {4, 8};
+  const int kToolbarElementPadding[] = {0, 8};
+  const int kToolbarLocationBarRightPadding[] = {4, 8};

[Evan Stade, 2016/10/24 19:09:40]

perhaps this can/should be combined with ToolbarStandardSpacing now?

[Peter Kasting, 2016/10/24 23:49:56]

Probably.  In general I'd try to combine constants when it makes sense to do so.

[Peter Kasting, 2016/10/27 22:16:37]

Do later?

 
   const int mode = ui::MaterialDesignController::GetMode();
   switch (constant) {
     case FIND_BAR_TOOLBAR_OVERLAP:
       return kFindBarVerticalOffset[mode];
     case LOCATION_BAR_BORDER_THICKNESS:
       return kLocationBarBorderThickness[mode];
     case LOCATION_BAR_BUBBLE_FONT_VERTICAL_PADDING:
       return kLocationBarBubbleFontVerticalPadding[mode];
     case LOCATION_BAR_BUBBLE_HORIZONTAL_PADDING:
@@ skipping 27 matching lines @@
     case TOOLBAR_LOCATION_BAR_RIGHT_PADDING:
       return kToolbarLocationBarRightPadding[mode];
     case TOOLBAR_STANDARD_SPACING:
       return kToolbarStandardSpacing[mode];
   }
   NOTREACHED();
   return 0;
 }
 
 gfx::Insets GetLayoutInsets(LayoutInset inset) {
-  const int kAvatarLeftPadding[] = {2, 4, 4};
-  const int kAvatarRightPadding[] = {-6, 4, 4};
-  const int kAvatarBottomPadding[] = {2, 4, 4};
-  const int kOmniboxDropdownIconPadding[] = {2, 4, 8};
-  const int kOmniboxDropdownPadding[] = {3, 4, 4};
-  const int kOmniboxDropdownTextPadding[] = {3, 3, 3};
-  const int kTabBottomPadding[] = {2, 1, 1};
-  const int kTabHorizontalPadding[] = {20, 16, 18};
-  const int kTabTopPadding[] = {4, 1, 1};
-  const int kToolbarBottomPadding[] = {5, 5, 5};
-  const int kToolbarButtonPadding[] = {2, 6, 6};
-  const int kToolbarLeftPadding[] = {3, 4, 8};
-  const int kToolbarRightPadding[] = {2, 4, 8};
-  const int kToolbarTopPadding[] = {5, 4, 4};
+  const int kAvatarLeftPadding[] = {4, 4};
+  const int kAvatarRightPadding[] = {4, 4};
+  const int kAvatarBottomPadding[] = {4, 4};
+  const int kOmniboxDropdownIconPadding[] = {4, 8};
+  const int kOmniboxDropdownPadding[] = {4, 4};
+  const int kOmniboxDropdownTextPadding[] = {3, 3};
+  const int kTabBottomPadding[] = {1, 1};
+  const int kTabHorizontalPadding[] = {16, 18};
+  const int kTabTopPadding[] = {1, 1};
+  const int kToolbarBottomPadding[] = {5, 5};
+  const int kToolbarButtonPadding[] = {6, 6};
+  const int kToolbarLeftPadding[] = {4, 8};
+  const int kToolbarRightPadding[] = {4, 8};
+  const int kToolbarTopPadding[] = {4, 4};
 
   const int mode = ui::MaterialDesignController::GetMode();
   switch (inset) {
     case AVATAR_ICON:
       return gfx::Insets(0, kAvatarLeftPadding[mode],
                          kAvatarBottomPadding[mode], kAvatarRightPadding[mode]);
     case OMNIBOX_DROPDOWN:
       return gfx::Insets(kOmniboxDropdownPadding[mode], 0);
     case OMNIBOX_DROPDOWN_ICON:
       return gfx::Insets(kOmniboxDropdownIconPadding[mode], 0);
     case OMNIBOX_DROPDOWN_TEXT:
       return gfx::Insets(kOmniboxDropdownTextPadding[mode], 0);
     case TAB:
       return gfx::Insets(kTabTopPadding[mode], kTabHorizontalPadding[mode],
                          kTabBottomPadding[mode], kTabHorizontalPadding[mode]);
     case TOOLBAR:
       return gfx::Insets(kToolbarTopPadding[mode], kToolbarLeftPadding[mode],
                          kToolbarBottomPadding[mode],
                          kToolbarRightPadding[mode]);
     case TOOLBAR_BUTTON:
       return gfx::Insets(kToolbarButtonPadding[mode]);
   }
   NOTREACHED();
   return gfx::Insets();
 }
 
 gfx::Size GetLayoutSize(LayoutSize size) {
-  const int kNewTabButtonWidth[] = {34, 36, 39};
-  const int kNewTabButtonHeight[] = {18, 18, 21};
+  const int kNewTabButtonWidth[] = {36, 39};
+  const int kNewTabButtonHeight[] = {18, 21};
 
   const int mode = ui::MaterialDesignController::GetMode();
   switch (size) {
     case NEW_TAB_BUTTON:
       return gfx::Size(kNewTabButtonWidth[mode], kNewTabButtonHeight[mode]);
   }
   NOTREACHED();
   return gfx::Size();
 }