Touchable Chrome: Fix profile avatar layout

- Adjust the alignment of the avatar icon in Touchable Chrome
  to follow the specs.
- Draw a circular 1px stroke around the avatar with the same
  color as that of the tab separator stroke.
- Refactor and gather some common code.

BUG=805267

Change-Id: I2be3181232cd43724f71b899de7905869b003360
Reviewed-on: https://chromium-review.googlesource.com/956809
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542183}

chrome/browser/ui/views/frame/browser_non_client_frame_view.cc

@@ -58,6 +58,11 @@ BrowserNonClientFrameView::~BrowserNonClientFrameView() {
   }
 }
 
+// static
+int BrowserNonClientFrameView::GetAvatarIconPadding() {
+  return ui::MaterialDesignController::IsTouchOptimizedUiEnabled() ? 8 : 4;
+}
+
 void BrowserNonClientFrameView::OnBrowserViewInitViewsComplete() {
   UpdateMinimumSize();
 }
@@ -90,6 +95,12 @@ void BrowserNonClientFrameView::UpdateClientArea() {}
 
 void BrowserNonClientFrameView::UpdateMinimumSize() {}
 
+int BrowserNonClientFrameView::GetTabStripLeftInset() const {
+  return profile_indicator_icon()
+             ? 2 * GetAvatarIconPadding() + GetIncognitoAvatarIcon().width()
+             : 4;
+}
+
 void BrowserNonClientFrameView::ChildPreferredSizeChanged(views::View* child) {
   if (child == GetProfileSwitcherView()) {
     // Perform a re-layout if the avatar button has changed, since that can
@@ -200,9 +211,12 @@ void BrowserNonClientFrameView::UpdateProfileIcons() {
   gfx::Image icon;
   if (is_incognito) {
     icon = gfx::Image(GetIncognitoAvatarIcon());
+    profile_indicator_icon_->set_stroke_color(SK_ColorTRANSPARENT);
   } else {
 #if defined(OS_CHROMEOS)
     AvatarMenu::GetImageForMenuButton(profile->GetPath(), &icon);
+    // Draw a stroke around the profile icon only for the avatar.
+    profile_indicator_icon_->set_stroke_color(GetToolbarTopSeparatorColor());
 #else
     NOTREACHED();
 #endif
@@ -219,12 +233,12 @@ void BrowserNonClientFrameView::LayoutIncognitoButton() {
 #endif
   gfx::ImageSkia incognito_icon = GetIncognitoAvatarIcon();
   int avatar_bottom = GetTopInset(false) + browser_view()->GetTabStripHeight() -
-                      kAvatarIconPadding;
+                      GetAvatarIconPadding();
   int avatar_y = avatar_bottom - incognito_icon.height();
   int avatar_height = incognito_icon.height();
+  gfx::Rect avatar_bounds(GetAvatarIconPadding(), avatar_y,
+                          incognito_icon.width(), avatar_height);
 
-  gfx::Rect avatar_bounds(kAvatarIconPadding, avatar_y, incognito_icon.width(),
-                          avatar_height);
   profile_indicator_icon()->SetBoundsRect(avatar_bounds);
   profile_indicator_icon()->SetVisible(true);
 }

chrome/browser/ui/views/frame/browser_non_client_frame_view.h

@@ -18,12 +18,12 @@ class BrowserView;
 class BrowserNonClientFrameView : public views::NonClientFrameView,
                                   public ProfileAttributesStorage::Observer {
  public:
-  // The padding on the left, right, and bottom of the avatar icon.
-  static constexpr int kAvatarIconPadding = 4;
-
   BrowserNonClientFrameView(BrowserFrame* frame, BrowserView* browser_view);
   ~BrowserNonClientFrameView() override;
 
+  // Returns the padding on the left, right, and bottom of the avatar icon.
+  static int GetAvatarIconPadding();
+
   BrowserView* browser_view() const { return browser_view_; }
   BrowserFrame* frame() const { return frame_; }
 
@@ -77,6 +77,12 @@ class BrowserNonClientFrameView : public views::NonClientFrameView,
   // Provided for mus to update the minimum window size property.
   virtual void UpdateMinimumSize();
 
+  // Distance between the leading edge of the NonClientFrameView and the tab
+  // strip.
+  // TODO: Consider refactoring and unifying tabstrip bounds calculations.
+  // https://crbug.com/820485.
+  virtual int GetTabStripLeftInset() const;
+
   // Overriden from views::View.
   void ChildPreferredSizeChanged(views::View* child) override;
   void VisibilityChanged(views::View* starting_from, bool is_visible) override;

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc

@@ -352,7 +352,7 @@ void BrowserNonClientFrameViewAsh::Layout() {
   frame_header_->SetHeaderHeightForPainting(painted_height);
 
   if (profile_indicator_icon())
-    LayoutProfileIndicatorIcon();
+    LayoutIncognitoButton();
   BrowserNonClientFrameView::Layout();
   const bool immersive =
       browser_view()->immersive_mode_controller()->IsEnabled();
@@ -499,14 +499,6 @@ AvatarButtonStyle BrowserNonClientFrameViewAsh::GetAvatarButtonStyle() const {
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserNonClientFrameViewAsh, private:
 
-int BrowserNonClientFrameViewAsh::GetTabStripLeftInset() const {
-  const int avatar_right =
-      profile_indicator_icon()
-          ? (kAvatarIconPadding + GetIncognitoAvatarIcon().width())
-          : 0;
-  return avatar_right + kAvatarIconPadding;
-}
-
 int BrowserNonClientFrameViewAsh::GetTabStripRightInset() const {
   return kTabstripRightSpacing +
       caption_button_container_->GetPreferredSize().width();
@@ -519,21 +511,6 @@ bool BrowserNonClientFrameViewAsh::UsePackagedAppHeaderStyle() const {
          browser->is_app();
 }
 
-void BrowserNonClientFrameViewAsh::LayoutProfileIndicatorIcon() {
-  DCHECK(profile_indicator_icon());
-
-  const gfx::ImageSkia incognito_icon = GetIncognitoAvatarIcon();
-  const int avatar_bottom = GetTopInset(false) +
-                            browser_view()->GetTabStripHeight() -
-                            kAvatarIconPadding;
-  int avatar_y = avatar_bottom - incognito_icon.height();
-
-  const int avatar_height = avatar_bottom - avatar_y;
-  profile_indicator_icon()->SetBounds(kAvatarIconPadding, avatar_y,
-                                      incognito_icon.width(), avatar_height);
-  profile_indicator_icon()->SetVisible(true);
-}
-
 bool BrowserNonClientFrameViewAsh::ShouldPaint() const {
   // We need to paint when the top-of-window views are revealed in immersive
   // fullscreen.

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h

@@ -108,9 +108,6 @@ class BrowserNonClientFrameViewAsh : public BrowserNonClientFrameView,
 
   friend class BrowserFrameHeaderAsh;
 
-  // Distance between the left edge of the NonClientFrameView and the tab strip.
-  int GetTabStripLeftInset() const;
-
   // Distance between the right edge of the NonClientFrameView and the tab
   // strip.
   int GetTabStripRightInset() const;
@@ -120,8 +117,6 @@ class BrowserNonClientFrameViewAsh : public BrowserNonClientFrameView,
   // scheme than browser windows.
   bool UsePackagedAppHeaderStyle() const;
 
-  void LayoutProfileIndicatorIcon();
-
   // Returns true if there is anything to paint. Some fullscreen windows do not
   // need their frames painted.
   bool ShouldPaint() const;

chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h

@@ -21,6 +21,7 @@ class BrowserNonClientFrameViewMac : public BrowserNonClientFrameView {
   int GetTopInset(bool restored) const override;
   int GetThemeBackgroundXInset() const override;
   void UpdateThrobber(bool running) override;
+  int GetTabStripLeftInset() const override;
 
   // views::NonClientFrameView:
   gfx::Rect GetBoundsForClientView() const override;

chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm

@@ -42,7 +42,7 @@ gfx::Rect BrowserNonClientFrameViewMac::GetBoundsForTabStrip(
   DCHECK(tabstrip);
   gfx::Rect bounds = gfx::Rect(0, kTabstripTopInset, width(),
                                tabstrip->GetPreferredSize().height());
-  bounds.Inset(kTabstripLeftInset, 0, GetTabStripRightInset(), 0);
+  bounds.Inset(GetTabStripLeftInset(), 0, GetTabStripRightInset(), 0);
   return bounds;
 }
 
@@ -56,7 +56,8 @@ int BrowserNonClientFrameViewMac::GetTabStripRightInset() const {
   if (profile_switcher_view) {
     inset += profile_switcher_view->GetPreferredSize().width();
   } else if (profile_indicator_icon()) {
-    inset += profile_indicator_icon()->bounds().width() + kAvatarIconPadding;
+    inset +=
+        profile_indicator_icon()->bounds().width() + GetAvatarIconPadding();
   }
   return inset;
 }
@@ -68,6 +69,10 @@ int BrowserNonClientFrameViewMac::GetThemeBackgroundXInset() const {
 void BrowserNonClientFrameViewMac::UpdateThrobber(bool running) {
 }
 
+int BrowserNonClientFrameViewMac::GetTabStripLeftInset() const {
+  return kTabstripLeftInset;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserNonClientFrameViewMac, views::NonClientFrameView implementation:
 

chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.cc

@@ -225,6 +225,11 @@ void BrowserNonClientFrameViewMus::UpdateMinimumSize() {
   }
 }
 
+int BrowserNonClientFrameViewMus::GetTabStripLeftInset() const {
+  return BrowserNonClientFrameView::GetTabStripLeftInset() +
+         frame_values().normal_insets.left();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // views::NonClientFrameView:
 
@@ -380,15 +385,6 @@ void BrowserNonClientFrameViewMus::TabStripDeleted(TabStrip* tab_strip) {
   tab_strip_ = nullptr;
 }
 
-int BrowserNonClientFrameViewMus::GetTabStripLeftInset() const {
-  const int avatar_right =
-      profile_indicator_icon()
-          ? (kAvatarIconPadding + GetIncognitoAvatarIcon().width())
-          : 0;
-  return avatar_right + kAvatarIconPadding +
-         frame_values().normal_insets.left();
-}
-
 int BrowserNonClientFrameViewMus::GetTabStripRightInset() const {
   const int frame_right_insets = frame_values().normal_insets.right() +
                                  frame_values().max_title_bar_button_width;

chrome/browser/ui/views/frame/browser_non_client_frame_view_mus.h

@@ -38,6 +38,7 @@ class BrowserNonClientFrameViewMus : public BrowserNonClientFrameView,
   void UpdateThrobber(bool running) override;
   void UpdateClientArea() override;
   void UpdateMinimumSize() override;
+  int GetTabStripLeftInset() const override;
 
   // views::NonClientFrameView:
   gfx::Rect GetBoundsForClientView() const override;
@@ -70,9 +71,6 @@ class BrowserNonClientFrameViewMus : public BrowserNonClientFrameView,
   void TabStripMaxXChanged(TabStrip* tab_strip) override;
   void TabStripDeleted(TabStrip* tab_strip) override;
 
-  // Distance between the left edge of the NonClientFrameView and the tab strip.
-  int GetTabStripLeftInset() const;
-
   // Distance between the right edge of the NonClientFrameView and the tab
   // strip.
   int GetTabStripRightInset() const;

chrome/browser/ui/views/frame/glass_browser_frame_view.cc

@@ -139,7 +139,7 @@ GlassBrowserFrameView::~GlassBrowserFrameView() {
 
 gfx::Rect GlassBrowserFrameView::GetBoundsForTabStrip(
     views::View* tabstrip) const {
-  const int x = incognito_bounds_.right() + kAvatarIconPadding;
+  const int x = GetTabStripLeftInset();
   int end_x = width() - ClientBorderThickness(false);
   if (!CaptionButtonsOnLeadingEdge()) {
     end_x = std::min(MinimizeButtonX(), end_x) -
@@ -218,6 +218,10 @@ gfx::Size GlassBrowserFrameView::GetMinimumSize() const {
   return min_size;
 }
 
+int GlassBrowserFrameView::GetTabStripLeftInset() const {
+  return incognito_bounds_.right() + GetAvatarIconPadding();
+}
+
 void GlassBrowserFrameView::OnBrowserViewInitViewsComplete() {
   if (browser_view()->tabstrip()) {
     DCHECK(!tab_strip_observer_.IsObserving(browser_view()->tabstrip()));
@@ -754,9 +758,9 @@ void GlassBrowserFrameView::LayoutIncognitoIcon() {
                                   : 0);
   }
   const int bottom = GetTopInset(false) + browser_view()->GetTabStripHeight() -
-                     kAvatarIconPadding;
+                     GetAvatarIconPadding();
   incognito_bounds_.SetRect(
-      x + (profile_indicator_icon() ? kAvatarIconPadding : 0),
+      x + (profile_indicator_icon() ? GetAvatarIconPadding() : 0),
       bottom - size.height(), profile_indicator_icon() ? size.width() : 0,
       size.height());
   if (profile_indicator_icon())

chrome/browser/ui/views/frame/glass_browser_frame_view.h

@@ -36,6 +36,7 @@ class GlassBrowserFrameView : public BrowserNonClientFrameView,
   int GetThemeBackgroundXInset() const override;
   void UpdateThrobber(bool running) override;
   gfx::Size GetMinimumSize() const override;
+  int GetTabStripLeftInset() const override;
   void OnBrowserViewInitViewsComplete() override;
 
   // views::NonClientFrameView:

chrome/browser/ui/views/frame/opaque_browser_frame_view.cc

@@ -173,6 +173,10 @@ gfx::Size OpaqueBrowserFrameView::GetMinimumSize() const {
   return layout_->GetMinimumSize(width());
 }
 
+int OpaqueBrowserFrameView::GetTabStripLeftInset() const {
+  return layout_->GetTabStripLeftInset();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // OpaqueBrowserFrameView, views::NonClientFrameView implementation:
 

chrome/browser/ui/views/frame/opaque_browser_frame_view.h

@@ -55,6 +55,7 @@ class OpaqueBrowserFrameView : public BrowserNonClientFrameView,
   int GetThemeBackgroundXInset() const override;
   void UpdateThrobber(bool running) override;
   gfx::Size GetMinimumSize() const override;
+  int GetTabStripLeftInset() const override;
 
   // views::NonClientFrameView:
   gfx::Rect GetBoundsForClientView() const override;

chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc

@@ -104,7 +104,7 @@ void OpaqueBrowserFrameViewLayout::SetButtonOrdering(
 gfx::Rect OpaqueBrowserFrameViewLayout::GetBoundsForTabStrip(
     const gfx::Size& tabstrip_preferred_size,
     int available_width) const {
-  int x = leading_button_start_ + OpaqueBrowserFrameView::kAvatarIconPadding;
+  const int x = GetTabStripLeftInset();
   available_width -= x + NewTabCaptionSpacing() + trailing_button_start_;
   return gfx::Rect(x, GetTabStripInsetsTop(false), std::max(0, available_width),
                    tabstrip_preferred_size.height());
@@ -132,6 +132,10 @@ gfx::Size OpaqueBrowserFrameViewLayout::GetMinimumSize(
   return min_size;
 }
 
+int OpaqueBrowserFrameViewLayout::GetTabStripLeftInset() const {
+  return leading_button_start_ + OpaqueBrowserFrameView::GetAvatarIconPadding();
+}
+
 gfx::Rect OpaqueBrowserFrameViewLayout::GetWindowBoundsForClientBounds(
     const gfx::Rect& client_bounds) const {
   int top_height = NonClientTopHeight(false);
@@ -428,7 +432,7 @@ void OpaqueBrowserFrameViewLayout::LayoutIncognitoIcon(views::View* host) {
   trailing_button_start_ = std::max(trailing_button_start_, min_button_width);
 
   if (incognito_icon_) {
-    constexpr int pad = OpaqueBrowserFrameView::kAvatarIconPadding;
+    const int pad = OpaqueBrowserFrameView::GetAvatarIconPadding();
     const gfx::Size size(delegate_->GetIncognitoAvatarIcon().size());
     const int incognito_width = pad + size.width();
     int x;

chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.h

@@ -55,6 +55,9 @@ class OpaqueBrowserFrameViewLayout : public views::LayoutManager {
 
   gfx::Size GetMinimumSize(int available_width) const;
 
+  // Distance between the left edge of the NonClientFrameView and the tab strip.
+  int GetTabStripLeftInset() const;
+
   // Returns the bounds of the window required to display the content area at
   // the specified bounds.
   gfx::Rect GetWindowBoundsForClientBounds(

chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc

@@ -241,7 +241,7 @@ class OpaqueBrowserFrameViewLayoutTest : public views::ViewsTestBase {
                      : -GetLayoutSize(NEW_TAB_BUTTON, delegate_->IsIncognito())
                             .width());
     }
-    int tabstrip_x = OpaqueBrowserFrameView::kAvatarIconPadding;
+    int tabstrip_x = OpaqueBrowserFrameView::GetAvatarIconPadding();
     if (show_caption_buttons && caption_buttons_on_left) {
       int right_of_close =
           maximized ? kMaximizedExtraCloseWidth : OBFVL::kFrameBorderThickness;

chrome/browser/ui/views/profiles/profile_indicator_icon.cc

@@ -7,7 +7,7 @@
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "ui/gfx/canvas.h"
 
-ProfileIndicatorIcon::ProfileIndicatorIcon() : old_height_(0) {
+ProfileIndicatorIcon::ProfileIndicatorIcon() {
   // In RTL mode, the incognito icon should be looking the opposite direction.
   EnableCanvasFlippingForRTLUI(true);
 }
@@ -45,6 +45,22 @@ void ProfileIndicatorIcon::OnPaint(gfx::Canvas* canvas) {
   canvas->DrawImageInt(modified_icon_, 0, 0, modified_icon_.width(),
                        modified_icon_.height(), dst_x, dst_y, dst_width,
                        dst_height, false);
+
+  if (stroke_color_ == SK_ColorTRANSPARENT)
+    return;
+
+  // Draw a 1px circular stroke (regardless of DSF) around the avatar icon.
+  const gfx::PointF center((dst_x + dst_width) / 2.0f,
+                           (dst_y + dst_height) / 2.0f);
+  cc::PaintFlags paint_flags;
+  paint_flags.setAntiAlias(true);
+  paint_flags.setStyle(cc::PaintFlags::kStroke_Style);
+  paint_flags.setColor(stroke_color_);
+  paint_flags.setStrokeWidth(1.0f / canvas->image_scale());
+  // Reduce the radius by 0.5f such that the circle overlaps with the edge of
+  // the image.
+  const float radius = (dst_width + dst_height) / 4.0f;
+  canvas->DrawCircle(center, radius - 0.5f, paint_flags);
 }
 
 void ProfileIndicatorIcon::SetIcon(const gfx::Image& icon) {

chrome/browser/ui/views/profiles/profile_indicator_icon.h

@@ -27,17 +27,22 @@ class ProfileIndicatorIcon : public views::View {
   ProfileIndicatorIcon();
   ~ProfileIndicatorIcon() override;
 
-  // views::MenuButton:
+  // views::View:
   void OnPaint(gfx::Canvas* canvas) override;
 
   // Sets the image for the avatar button. Rectangular images, as opposed
   // to Chrome avatar icons, will be resized and modified for the title bar.
   void SetIcon(const gfx::Image& icon);
 
+  // Sets the color to use for drawing a circular stroke around the icon image.
+  // Use SK_ColorTRANSPARENT not to draw any stroke.
+  void set_stroke_color(SkColor color) { stroke_color_ = color; }
+
  private:
   gfx::Image base_icon_;
   gfx::ImageSkia modified_icon_;
-  int old_height_;
+  int old_height_ = 0;
+  SkColor stroke_color_ = SK_ColorTRANSPARENT;
   DISALLOW_COPY_AND_ASSIGN(ProfileIndicatorIcon);
 };