Linux Aura Task Manager Frame Buttons Misaligned

ui/views/window/custom_frame_view.cc

 // Copyright (c) 2012 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 "ui/views/window/custom_frame_view.h"
 
 #include <algorithm>
 
 #include "base/strings/utf_string_conversions.h"
 #include "grit/ui_resources.h"
 #include "grit/ui_strings.h"
 #include "ui/base/hit_test.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/path.h"
 #include "ui/views/color_constants.h"
 #include "ui/views/controls/button/image_button.h"
+
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+#include "ui/views/linux_ui/linux_ui.h"
+#endif
+
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/native_widget_aura.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/views/window/client_view.h"
 #include "ui/views/window/frame_background.h"
 #include "ui/views/window/window_resources.h"
 #include "ui/views/window/window_shape.h"
 
 namespace views {
@@ skipping 42 matching lines @@
 // CustomFrameView, public:
 
 CustomFrameView::CustomFrameView()
     : frame_(NULL),
       window_icon_(NULL),
       minimize_button_(NULL),
       maximize_button_(NULL),
       restore_button_(NULL),
       close_button_(NULL),
       should_show_maximize_button_(false),
-      frame_background_(new FrameBackground()) {
+      frame_background_(new FrameBackground()),
+      minimum_title_bar_x_(0),
+      maximum_title_bar_x_(INT_MAX) {
+  trailing_buttons_.push_back(views::FRAME_BUTTON_MINIMIZE);
+  trailing_buttons_.push_back(views::FRAME_BUTTON_MAXIMIZE);
+  trailing_buttons_.push_back(views::FRAME_BUTTON_CLOSE);
 }
 
 CustomFrameView::~CustomFrameView() {
 }
 
 void CustomFrameView::Init(Widget* frame) {
   frame_ = frame;
 
-  close_button_ = new ImageButton(this);
-  close_button_->SetAccessibleName(
-      l10n_util::GetStringUTF16(IDS_APP_ACCNAME_CLOSE));
-
-  // Close button images will be set in LayoutWindowControls().
-  AddChildView(close_button_);
+  close_button_ = InitWindowCaptionButton(IDS_APP_ACCNAME_CLOSE,
+      IDR_CLOSE, IDR_CLOSE_H, IDR_CLOSE_P);
 
   minimize_button_ = InitWindowCaptionButton(IDS_APP_ACCNAME_MINIMIZE,
       IDR_MINIMIZE, IDR_MINIMIZE_H, IDR_MINIMIZE_P);
 
   maximize_button_ = InitWindowCaptionButton(IDS_APP_ACCNAME_MAXIMIZE,
       IDR_MAXIMIZE, IDR_MAXIMIZE_H, IDR_MAXIMIZE_P);
 
   restore_button_ = InitWindowCaptionButton(IDS_APP_ACCNAME_RESTORE,
       IDR_RESTORE, IDR_RESTORE_H, IDR_RESTORE_P);
 
   should_show_maximize_button_ = frame_->widget_delegate()->CanMaximize();
 
   if (frame_->widget_delegate()->ShouldShowWindowIcon()) {
     window_icon_ = new ImageButton(this);
     AddChildView(window_icon_);
   }
+
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+  views::LinuxUI* ui = views::LinuxUI::instance();
+  if (ui)
+    ui->AddWindowButtonOrderObserver(this);
+#endif
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // CustomFrameView, NonClientFrameView implementation:
 
 gfx::Rect CustomFrameView::GetBoundsForClientView() const {
   return client_view_bounds_;
 }
 
 gfx::Rect CustomFrameView::GetWindowBoundsForClientBounds(
@@ skipping 123 matching lines @@
     frame_->Close();
   else if (sender == minimize_button_)
     frame_->Minimize();
   else if (sender == maximize_button_)
     frame_->Maximize();
   else if (sender == restore_button_)
     frame_->Restore();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
+// CustomFrameView, WindowButtonOrderObserver:
+
+void CustomFrameView::OnWindowButtonOrderingChange(
+    const std::vector<views::FrameButton>& leading_buttons,
+    const std::vector<views::FrameButton>& trailing_buttons) {
+  leading_buttons_ = leading_buttons;
+  trailing_buttons_ = trailing_buttons;
+}
+
+///////////////////////////////////////////////////////////////////////////////
 // CustomFrameView, private:
 
 int CustomFrameView::FrameBorderThickness() const {
   return frame_->IsMaximized() ? 0 : kFrameBorderThickness;
 }
 
 int CustomFrameView::NonClientBorderThickness() const {
   // In maximized mode, we don't show a client edge.
   return FrameBorderThickness() +
       (ShouldShowClientEdge() ? kClientEdgeThickness : 0);
 }
 
 int CustomFrameView::NonClientTopBorderHeight() const {
   return std::max(FrameBorderThickness() + IconSize(),
                   CaptionButtonY() + kCaptionButtonHeightWithPadding) +
       TitlebarBottomThickness();
 }
 
 int CustomFrameView::CaptionButtonY() const {
   // Maximized buttons start at window top so that even if their images aren't
   // drawn flush with the screen edge, they still obey Fitts' Law.
-  return frame_->IsMaximized() ? FrameBorderThickness() : kFrameShadowThickness;
+  return frame_->IsMaximized() ? FrameBorderThickness() : kFrameBorderThickness;
 }
 
 int CustomFrameView::TitlebarBottomThickness() const {
   return kTitlebarTopAndBottomEdgeThickness +
       (ShouldShowClientEdge() ? kClientEdgeThickness : 0);
 }
 
 int CustomFrameView::IconSize() const {
 #if defined(OS_WIN)
   // This metric scales up if either the titlebar height or the titlebar font
@@ skipping 17 matching lines @@
   // from below the 3D edge.
   int unavailable_px_at_top = frame_->IsMaximized() ?
       frame_thickness : kTitlebarTopAndBottomEdgeThickness;
   // When the icon is shorter than the minimum space we reserve for the caption
   // button, we vertically center it.  We want to bias rounding to put extra
   // space above the icon, since the 3D edge (+ client edge, for restored
   // windows) below looks (to the eye) more like additional space than does the
   // 3D edge (or nothing at all, for maximized windows) above; hence the +1.
   int y = unavailable_px_at_top + (NonClientTopBorderHeight() -
       unavailable_px_at_top - size - TitlebarBottomThickness() + 1) / 2;
-  return gfx::Rect(frame_thickness + kIconLeftSpacing, y, size, size);
+  return gfx::Rect(frame_thickness + kIconLeftSpacing + minimum_title_bar_x_,
+                   y, size, size);
 }
 
 bool CustomFrameView::ShouldShowTitleBarAndBorder() const {
   if (frame_->IsFullscreen())
     return false;
 
   if (ViewsDelegate::views_delegate) {
     return !ViewsDelegate::views_delegate->WindowManagerProvidesTitleBar(
                 frame_->IsMaximized());
   }
@@ skipping 132 matching lines @@
 
 SkColor CustomFrameView::GetFrameColor() const {
   return frame_->IsActive() ? kDefaultColorFrame : kDefaultColorFrameInactive;
 }
 
 const gfx::ImageSkia* CustomFrameView::GetFrameImage() const {
   return ui::ResourceBundle::GetSharedInstance().GetImageNamed(
       frame_->IsActive() ? IDR_FRAME : IDR_FRAME_INACTIVE).ToImageSkia();
 }
 
+void CustomFrameView::LayoutButton(ImageButton* button, int x, int y,
+    bool leading) {

[flackr, 2014/04/22 18:36:45]

I don't think this wrapping conforms to the style guide:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi....

Should be wrapped to open bracket or all arguments wrapped.

[jonross, 2014/04/23 18:53:53]

Done.

+  button->SetVisible(true);
+  button->SetImageAlignment(ImageButton::ALIGN_LEFT,
+                            ImageButton::ALIGN_BOTTOM);
+  gfx::Size button_size = button->GetPreferredSize();
+
+  if (!leading)
+    x -= button_size.width();
+
+  button->SetBounds(x, y, button_size.width(), button_size.height());
+}
+
 void CustomFrameView::LayoutWindowControls() {
-  close_button_->SetImageAlignment(ImageButton::ALIGN_LEFT,
-                                   ImageButton::ALIGN_BOTTOM);
+  minimum_title_bar_x_ = 0;
+  maximum_title_bar_x_ = width();
   int caption_y = CaptionButtonY();
   bool is_maximized = frame_->IsMaximized();
   // There should always be the same number of non-shadow pixels visible to the
-  // side of the caption buttons.  In maximized mode we extend the rightmost
-  // button to the screen corner to obey Fitts' Law.
-  int right_extra_width = is_maximized ?
+  // side of the caption buttons.  In maximized mode we extend the edge button
+  // to the screen corner to obey Fitts' Law.
+  int extra_width = is_maximized ?
       (kFrameBorderThickness - kFrameShadowThickness) : 0;
-  gfx::Size close_button_size = close_button_->GetPreferredSize();
-  close_button_->SetBounds(width() - FrameBorderThickness() -
-      right_extra_width - close_button_size.width(), caption_y,
-      close_button_size.width() + right_extra_width,
-      close_button_size.height());
+  int next_button_x = FrameBorderThickness() + extra_width;
 
-  // When the window is restored, we show a maximized button; otherwise, we show
-  // a restore button.
   bool is_restored = !is_maximized && !frame_->IsMinimized();
   ImageButton* invisible_button = is_restored ? restore_button_
                                               : maximize_button_;
   invisible_button->SetVisible(false);
 
-  ImageButton* visible_button = is_restored ? maximize_button_
-                                            : restore_button_;
-  FramePartImage normal_part, hot_part, pushed_part;
-  int next_button_x;
-  if (should_show_maximize_button_) {
-    visible_button->SetVisible(true);
-    visible_button->SetImageAlignment(ImageButton::ALIGN_LEFT,
-                                      ImageButton::ALIGN_BOTTOM);
-    gfx::Size visible_button_size = visible_button->GetPreferredSize();
-    visible_button->SetBounds(close_button_->x() - visible_button_size.width(),
-                              caption_y, visible_button_size.width(),
-                              visible_button_size.height());
-    next_button_x = visible_button->x();
-  } else {
-    visible_button->SetVisible(false);
-    next_button_x = close_button_->x();
+  ImageButton* button = NULL;
+  for (std::vector<views::FrameButton>::const_iterator it =
+           leading_buttons_.begin(); it != leading_buttons_.end(); ++it) {
+    switch (*it) {
+      case views::FRAME_BUTTON_MINIMIZE: {
+        button = minimize_button_;

[flackr, 2014/04/22 18:36:45]

What if the button order specifies multiple of the same button? Can this happen?

[jonross, 2014/04/23 18:53:53]

Currently the last specified instance will determine the position of the button.
The offsets then used for the title bar text will be incorrect. However only
three buttons will ever be shown.

The logic for loading the config is:  GConfListener::ParseAndStoreButtonValue

It provides no guarantees that the config file doesn't specify duplicates.
However other handling code assumes that there are none.

+        break;
+      }
+      case views::FRAME_BUTTON_MAXIMIZE: {
+        button = is_restored ? maximize_button_ : restore_button_;
+        if (!should_show_maximize_button_) {
+          button->SetVisible(false);
+          continue;
+        }
+        break;
+      }
+      case views::FRAME_BUTTON_CLOSE: {
+        button = close_button_;
+        break;
+      }
+    }
+    LayoutButton(button, next_button_x, caption_y, true);
+    next_button_x += button->width();
+    minimum_title_bar_x_ = next_button_x;
   }
 
-  minimize_button_->SetVisible(true);
-  minimize_button_->SetImageAlignment(ImageButton::ALIGN_LEFT,
-                                      ImageButton::ALIGN_BOTTOM);
-  gfx::Size minimize_button_size = minimize_button_->GetPreferredSize();
-  minimize_button_->SetBounds(
-      next_button_x - minimize_button_size.width(), caption_y,
-      minimize_button_size.width(),
-      minimize_button_size.height());
-
-  normal_part = IDR_CLOSE;
-  hot_part = IDR_CLOSE_H;
-  pushed_part = IDR_CLOSE_P;
-
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-
-  close_button_->SetImage(CustomButton::STATE_NORMAL,
-                          rb.GetImageNamed(normal_part).ToImageSkia());
-  close_button_->SetImage(CustomButton::STATE_HOVERED,
-                          rb.GetImageNamed(hot_part).ToImageSkia());
-  close_button_->SetImage(CustomButton::STATE_PRESSED,
-                          rb.GetImageNamed(pushed_part).ToImageSkia());
+  // Trailing buttions are laid out in a RTL fashion
+  next_button_x = width() - FrameBorderThickness() - extra_width;
+  for (std::vector<views::FrameButton>::const_reverse_iterator it =
+           trailing_buttons_.rbegin(); it != trailing_buttons_.rend(); ++it) {
+    switch (*it) {

[flackr, 2014/04/22 18:36:45]

This duplication should be avoided. Maybe a common function to get the button?

[jonross, 2014/04/23 18:53:53]

Done.

+      case views::FRAME_BUTTON_MINIMIZE: {
+        button = minimize_button_;
+        break;
+      }
+      case views::FRAME_BUTTON_MAXIMIZE: {
+        button = is_restored ? maximize_button_ : restore_button_;
+        if (!should_show_maximize_button_) {
+          button->SetVisible(false);
+          continue;
+        }
+        break;
+      }
+      case views::FRAME_BUTTON_CLOSE: {
+        button = close_button_;
+        break;
+      }
+    }
+    LayoutButton(button, next_button_x, caption_y, false);
+    next_button_x = button->x();
+    maximum_title_bar_x_ = next_button_x;
+  }
 }
 
 void CustomFrameView::LayoutTitleBar() {
   // The window title position is calculated based on the icon position, even
   // when there is no icon.
   gfx::Rect icon_bounds(IconBounds());
   bool show_window_icon = window_icon_ != NULL;
   if (show_window_icon)
     window_icon_->SetBoundsRect(icon_bounds);
 
   // The offset between the window left edge and the title text.
   int title_x = show_window_icon ? icon_bounds.right() + kTitleIconOffsetX
                                  : icon_bounds.x();
   int title_height = GetTitleFontList().GetHeight();
   // We bias the title position so that when the difference between the icon and
   // title heights is odd, the extra pixel of the title is above the vertical
   // midline rather than below.  This compensates for how the icon is already
   // biased downwards (see IconBounds()) and helps prevent descenders on the
   // title from overlapping the 3D edge at the bottom of the titlebar.
   title_bounds_.SetRect(title_x,
       icon_bounds.y() + ((icon_bounds.height() - title_height - 1) / 2),
-      std::max(0, minimize_button_->x() - kTitleCaptionSpacing -
+      std::max(0, maximum_title_bar_x_ - kTitleCaptionSpacing -

[flackr, 2014/04/22 18:36:45]

Seems like it's very important that this is called after LayoutWindowControls,
is this guaranteed? This layout order dependency should be documented in the
header.

[jonross, 2014/04/23 18:53:53]

Not guaranteed by API. But it is private, and currently only called from Layout
where we force the order. I've added comments to the docs about this.

       title_x), title_height);
 }
 
 void CustomFrameView::LayoutClientView() {
   if (!ShouldShowTitleBarAndBorder()) {
     client_view_bounds_ = bounds();
     return;
   }
 
   int top_height = NonClientTopBorderHeight();
@@ skipping 15 matching lines @@
                    rb.GetImageNamed(normal_image_id).ToImageSkia());
   button->SetImage(CustomButton::STATE_HOVERED,
                    rb.GetImageNamed(hot_image_id).ToImageSkia());
   button->SetImage(CustomButton::STATE_PRESSED,
                    rb.GetImageNamed(pushed_image_id).ToImageSkia());
   AddChildView(button);
   return button;
 }
 
 }  // namespace views