Use light images in colored app frames, when the frame color is dark.

apps/ui/views/app_window_frame_view.cc

 // Copyright 2014 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 "apps/ui/views/app_window_frame_view.h"
 
 #include "apps/ui/native_app_window.h"
 #include "base/strings/utf_string_conversions.h"
 #include "extensions/common/draggable_region.h"
 #include "grit/theme_resources.h"
 #include "grit/ui_strings.h"  // Accessibility names
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkRegion.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/color_utils.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/path.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 
 namespace {
 
@@ skipping 37 matching lines @@
         views::CustomButton::STATE_NORMAL,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_CLOSE).ToImageSkia());
     close_button_->SetImage(
         views::CustomButton::STATE_HOVERED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_CLOSE_H).ToImageSkia());
     close_button_->SetImage(
         views::CustomButton::STATE_PRESSED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_CLOSE_P).ToImageSkia());
     close_button_->SetAccessibleName(
         l10n_util::GetStringUTF16(IDS_APP_ACCNAME_CLOSE));
     AddChildView(close_button_);

[Matt Giuca, 2014/05/09 08:33:11]

// STATE_NORMAL images are set in SetButtonImagesForFrame, not here.

(or similar)

[benwells, 2014/05/09 09:06:05]

Done.

     maximize_button_ = new views::ImageButton(this);
     maximize_button_->SetImage(
-        views::CustomButton::STATE_NORMAL,
-        rb.GetNativeImageNamed(IDR_APP_WINDOW_MAXIMIZE).ToImageSkia());
-    maximize_button_->SetImage(
         views::CustomButton::STATE_HOVERED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_MAXIMIZE_H).ToImageSkia());
     maximize_button_->SetImage(
         views::CustomButton::STATE_PRESSED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_MAXIMIZE_P).ToImageSkia());
     maximize_button_->SetImage(
         views::CustomButton::STATE_DISABLED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_MAXIMIZE_D).ToImageSkia());
     maximize_button_->SetAccessibleName(
         l10n_util::GetStringUTF16(IDS_APP_ACCNAME_MAXIMIZE));
     AddChildView(maximize_button_);
     restore_button_ = new views::ImageButton(this);
     restore_button_->SetImage(
-        views::CustomButton::STATE_NORMAL,
-        rb.GetNativeImageNamed(IDR_APP_WINDOW_RESTORE).ToImageSkia());
-    restore_button_->SetImage(
         views::CustomButton::STATE_HOVERED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_RESTORE_H).ToImageSkia());
     restore_button_->SetImage(
         views::CustomButton::STATE_PRESSED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_RESTORE_P).ToImageSkia());
     restore_button_->SetAccessibleName(
         l10n_util::GetStringUTF16(IDS_APP_ACCNAME_RESTORE));
     AddChildView(restore_button_);
     minimize_button_ = new views::ImageButton(this);
     minimize_button_->SetImage(
-        views::CustomButton::STATE_NORMAL,
-        rb.GetNativeImageNamed(IDR_APP_WINDOW_MINIMIZE).ToImageSkia());
-    minimize_button_->SetImage(
         views::CustomButton::STATE_HOVERED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_MINIMIZE_H).ToImageSkia());
     minimize_button_->SetImage(
         views::CustomButton::STATE_PRESSED,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_MINIMIZE_P).ToImageSkia());
     minimize_button_->SetAccessibleName(
         l10n_util::GetStringUTF16(IDS_APP_ACCNAME_MINIMIZE));
     AddChildView(minimize_button_);
+
+    SetButtonImagesForFrame();
   }
 }
 
 void AppWindowFrameView::SetResizeSizes(int resize_inside_bounds_size,
                                         int resize_outside_bounds_size,
                                         int resize_area_corner_size) {
   resize_inside_bounds_size_ = resize_inside_bounds_size;
   resize_outside_bounds_size_ = resize_outside_bounds_size;
   resize_area_corner_size_ = resize_area_corner_size;
 }
@@ skipping 169 matching lines @@
   if (ShouldPaintAsActive()) {
     close_button_->SetImage(
         views::CustomButton::STATE_NORMAL,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_CLOSE).ToImageSkia());
   } else {
     close_button_->SetImage(
         views::CustomButton::STATE_NORMAL,
         rb.GetNativeImageNamed(IDR_APP_WINDOW_CLOSE_U).ToImageSkia());
   }
 
-  // TODO(jeremya): different look for inactive?
+  SetButtonImagesForFrame();
+  // TODO(benwells): different look for inactive by default.
   SkPaint paint;
   paint.setAntiAlias(false);
   paint.setStyle(SkPaint::kFill_Style);
   if (widget_->IsActive())
     paint.setColor(active_frame_color_);
   else
     paint.setColor(inactive_frame_color_);
   gfx::Path path;
   path.moveTo(0, 0);
   path.lineTo(width(), 0);
@@ skipping 41 matching lines @@
   if (sender == close_button_)
     widget_->Close();
   else if (sender == maximize_button_)
     widget_->Maximize();
   else if (sender == restore_button_)
     widget_->Restore();
   else if (sender == minimize_button_)
     widget_->Minimize();
 }
 
+void AppWindowFrameView::SetButtonImagesForFrame() {
+  DCHECK(draw_frame_);
+
+  unsigned char frame_luma =
+      widget_->IsActive()
+          ? color_utils::GetLuminanceForColor(active_frame_color_)

[Matt Giuca, 2014/05/09 08:33:11]

Optional: Can I suggest a small refactor: add a method GetFrameColor that
returns widget_->IsActive() ? active_frame_color_ : innactive_frame_color_, and
use it here?

If not: Can you simplify this a bit:
unsigned char frame_luma = color_utils::GetLuminanceForColor(
    widget_->IsActive() ? active_frame_color_ :
    inactive_frame_color_);
(+ClangFormat)

[benwells, 2014/05/09 09:06:05]

Done.

+          : color_utils::GetLuminanceForColor(inactive_frame_color_);
+
+  // If the frame is dark, we should use the light images so they have
+  // some contrast.
+  const unsigned char kLuminanceThreshold = 100;
+  bool use_light = frame_luma < kLuminanceThreshold;
+
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();

[Matt Giuca, 2014/05/09 08:33:11]

const

(I looked up GetSharedInstance and I'm shocked that it returns a non-const
reference... but you shouldn't need non-const here.)

[benwells, 2014/05/09 09:06:05]

Yeah resource bundle isn't very consty. Needs to be non-const.

+  if (use_light) {
+    maximize_button_->SetImage(
+        views::CustomButton::STATE_NORMAL,
+        rb.GetNativeImageNamed(IDR_APP_WINDOW_MAXIMIZE_L).ToImageSkia());
+    restore_button_->SetImage(
+        views::CustomButton::STATE_NORMAL,
+        rb.GetNativeImageNamed(IDR_APP_WINDOW_RESTORE_L).ToImageSkia());
+    minimize_button_->SetImage(
+        views::CustomButton::STATE_NORMAL,
+        rb.GetNativeImageNamed(IDR_APP_WINDOW_MINIMIZE_L).ToImageSkia());
+  } else {
+    maximize_button_->SetImage(
+        views::CustomButton::STATE_NORMAL,
+        rb.GetNativeImageNamed(IDR_APP_WINDOW_MAXIMIZE).ToImageSkia());
+    restore_button_->SetImage(
+        views::CustomButton::STATE_NORMAL,
+        rb.GetNativeImageNamed(IDR_APP_WINDOW_RESTORE).ToImageSkia());
+    minimize_button_->SetImage(
+        views::CustomButton::STATE_NORMAL,
+        rb.GetNativeImageNamed(IDR_APP_WINDOW_MINIMIZE).ToImageSkia());
+  }
+}
+
 }  // namespace apps