Update avatar button to MD

chrome/browser/ui/views/frame/minimize_button_metrics_win.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 "chrome/browser/ui/views/frame/minimize_button_metrics_win.h"
 
 #include "base/logging.h"
 #include "base/i18n/rtl.h"
 #include "base/win/windows_version.h"
 #include "dwmapi.h"
 #include "ui/base/win/shell.h"
 #include "ui/display/win/dpi.h"
 #include "ui/display/win/screen_win.h"
 #include "ui/gfx/geometry/point.h"
 
 namespace {
 
 // These constants were determined by manually adding various offsets
 // until the identity switcher was placed at the same location as before.
 // When a new or updated OS version is released, a new constant may need
 // to be added to this list and GetDefaultButtonBoundsOffset() is updated.
 const int kWin7ButtonBoundsPositionOffset = 1;
 const int kWin8ButtonBoundsPositionOffset = 10;
 const int kWin10ButtonBoundsPositionOffset = 6;
 const int kInvalidOffset = static_cast<int>(0x80000000);
 
+// This constant was determined manually by testing on windows 10.
+const int kWin10ButtonHeight = 30;
+const int kInvalidHeight = static_cast<int>(0x80000000);
+
 using base::win::GetVersion;
 using display::win::ScreenWin;
 
 int GetDefaultButtonBoundsOffset() {
   if (GetVersion() >= base::win::VERSION_WIN10)
     return kWin10ButtonBoundsPositionOffset;
   if (GetVersion() >= base::win::VERSION_WIN8)
     return kWin8ButtonBoundsPositionOffset;
   return kWin7ButtonBoundsPositionOffset;
 }
 
+int GetDefaultButtonHeight() {
+  return kWin10ButtonHeight;
+}
+
 }  // namespace
 
 // static
 int MinimizeButtonMetrics::last_cached_minimize_button_x_delta_ = 0;
 
 // static
 int MinimizeButtonMetrics::button_bounds_position_offset_ = kInvalidOffset;
 
+// static
+int MinimizeButtonMetrics::button_bounds_height_ = kInvalidHeight;
+
 MinimizeButtonMetrics::MinimizeButtonMetrics()
     : hwnd_(nullptr),
       cached_minimize_button_x_delta_(last_cached_minimize_button_x_delta_),
       was_activated_(false) {
 }
 
 MinimizeButtonMetrics::~MinimizeButtonMetrics() {
 }
 
 void MinimizeButtonMetrics::Init(HWND hwnd) {
@@ skipping 60 matching lines @@
   if (!dwm_button_pos) {
     // Fallback to using the message for the titlebar info only if the above
     // code fails. It can fail if DWM is disabled globally or only for the
     // given HWND. The WM_GETTITLEBARINFOEX message can fail if we are not
     // active/visible. By fail we get a location of 0; the return status
     // code is always the same and similarly the state never seems to change
     // (titlebar_info.rgstate).
     TITLEBARINFOEX titlebar_info = {0};
     titlebar_info.cbSize = sizeof(TITLEBARINFOEX);
     SendMessage(hwnd_, WM_GETTITLEBARINFOEX, 0,
-                reinterpret_cast<WPARAM>(&titlebar_info));
+                reinterpret_cast<LPARAM>(&titlebar_info));
 
     // Under DWM WM_GETTITLEBARINFOEX won't return the right thing until after
     // WM_NCACTIVATE (maybe it returns classic values?). In an attempt to
     // return a consistant value we cache the last value across instances and
     // use it until we get the activate.
     if (titlebar_info.rgrect[2].left == titlebar_info.rgrect[2].right ||
         (titlebar_info.rgstate[2] &
          (STATE_SYSTEM_INVISIBLE | STATE_SYSTEM_OFFSCREEN |
           STATE_SYSTEM_UNAVAILABLE)))
       return 0;
@@ skipping 45 matching lines @@
     cached_minimize_button_x_delta_ = minimize_button_offset;
   } else {
     RECT client_rect = {0};
     GetClientRect(hwnd_, &client_rect);
     cached_minimize_button_x_delta_ =
         client_rect.right - minimize_button_offset;
   }
   last_cached_minimize_button_x_delta_ = cached_minimize_button_x_delta_;
   return minimize_button_offset;
 }
+
+int MinimizeButtonMetrics::GetMinimizeButtonHeight() const {
+  // We want the "tall" avatar button to be the same height as the caption

[msarda, 2017/04/21 09:43:28]

Who is "we"?

Also, it is not clear why the comment in this file references the avatar button
(what is the avatar button in this context?). Remove the reference to the avatar
button in this comment, but keep the rest of the comment as it is really good.

[emx, 2017/04/24 16:23:10]

Done.

+  // buttons [http://crbug.com/635699] but it's very hard to get this right for
+  // all resolutions/DPI settings. GetSystemMetricsForDpi is probaly the real
+  // solution [http://crbug.com/668278]. GetSystemMetrics provides a good
+  // approximation for now.
+
+  // Note that the following methods do not return the correct result with DPI
+  // scaling settings other than 100%:
+  //   DwmGetWindowAttribute(hwnd_, DWMWA_CAPTION_BUTTON_BOUNDS, ...)
+  //   SendMessage(hwnd_, WM_GETTITLEBARINFOEX, ...)
+  //   display::win::ScreenWin::GetSystemMetricsInDIP(SM_CYSIZE)
+  //   display::win::ScreenWin::GetSystemMetricsForHwnd(hwnd_, SM_CYSIZE)
+
+  if (button_bounds_height_ == kInvalidHeight) {
+    int button_height_from_sysmetrics = GetSystemMetrics(SM_CYSIZE);
+    if (button_height_from_sysmetrics == 0) {
+      LOG(ERROR) << "GetSystemMetrics(SM_CYSIZE) error " << ::GetLastError();

[msarda, 2017/04/21 09:43:28]

If the user is in this case, then I think this line will write a log of logs
that are in general useless for the regular end user. I would argue that we
should only log this in debug, using DLOG.

[emx, 2017/04/24 16:23:10]

Done.

+      return GetDefaultButtonHeight();

[msarda, 2017/04/21 09:43:28]

From the name of the method, it looks like this function is supposed to return
the minimize button height for all Windows versions. However, in the
implementation, the default value is only correct for Windows 10.

I see 2 options:
* Change the method name to be Windows10 specific. And a DCHECK to check that
the OS version is at least 10 when this method is called.
* Have the right defaults for other versions. If 30 is the right default for all
windows versions, then change the constant name to kWinButtonHeight.

[emx, 2017/04/24 16:23:10]

Done.

+    }
+    button_bounds_height_ = button_height_from_sysmetrics;
+  }
+
+  return button_bounds_height_;
+}