Add GetCaptionButtonHeightInDIPs() method

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"
@@ skipping 34 matching lines @@
 
 MinimizeButtonMetrics::MinimizeButtonMetrics()
     : hwnd_(nullptr),
       cached_minimize_button_x_delta_(last_cached_minimize_button_x_delta_),
       was_activated_(false) {
 }
 
 MinimizeButtonMetrics::~MinimizeButtonMetrics() {
 }
 
+// static
+int MinimizeButtonMetrics::GetCaptionButtonHeight() {
+  // Caption button height is not affected by DPI settings.

[Peter Kasting, 2017/04/26 17:19:52]

robliao says that in his testing, this isn't correct.  He gets the same result
MSDN suggests: doubled values at double DPI.

His suggestion is that if you saw something different, you likely did one of two
things:
(1) Tested in a sandbox program -- by default Windows assumes programs are not
"DPI-aware" and virtualizes the results of GetSystemMetrics().  You have to tell
windows you are DPI-aware.  Chrome does this.
(2) Failed to log out and log in again after changing DPI, in which case
GetSystemMetrics() gives you the wrong results.

Given this, I think you really do want GetSystemMetricsInDIP().

+  // GetSystemMetrics(SM_CYSIZE) returns the same value regardless of the DPI
+  // settings of any monitor (28 on Windows 10). This is what needs to be

[Peter Kasting, 2017/04/26 17:19:52]

Rob also notes he gets 22 here, not 28.  I would just avoid specifying a
particular height.

+  // passed to a View, which then takes care of the scaling.

[Peter Kasting, 2017/04/26 17:19:52]

Nit: No need for the last sentence here

+
+  // A View drawn to this height may look ~1px off at some DPI settings.

[Peter Kasting, 2017/04/26 17:19:52]

I think "~1" is inaccurate here.  Assuming the conversion rounds, we could look
off by up to (DSF/2) px.  So if we ever get up to e.g. DSF=4, we could be off by
2 px.

I would just say "TODO(...): Due to conversion inaccuracy, returning the height
in DIPs may result in slight height differences with the native buttons. 
http://crbug.com/..."

Then I would file a separate bug on this specific issue and block 668278 on it,
and link that above.

+  // http://crbug.com/668278 tracks pixel-precise positioning for this.
+  // (According to robliao@ and bsep@ the solution is probably
+  // GetSystemMetricsForDpi.)
+
+  // Note that the following methods all return scaled values, which is not
+  // what the caller expects:
+  //   DwmGetWindowAttribute(hwnd_, DWMWA_CAPTION_BUTTON_BOUNDS, ...)
+  //   SendMessage(hwnd_, WM_GETTITLEBARINFOEX, ...)
+  //   display::win::ScreenWin::GetSystemMetricsForHwnd(hwnd_, SM_CYSIZE)
+  //     these all return (height * scale factor) e.g. 56 at 200%
+  //   display::win::ScreenWin::GetSystemMetricsInDIP(SM_CYSIZE)
+  //     returns (height / by primary monitor scale factor) e.g. 22 for 125%

[Peter Kasting, 2017/04/26 17:19:52]

I would omit this comment entirely, as your first one covers it by saying
SM_CYSIZE is not affected by DPI.

+
+  return GetSystemMetrics(SM_CYSIZE);
+}
+
 void MinimizeButtonMetrics::Init(HWND hwnd) {
   DCHECK(!hwnd_);
   hwnd_ = hwnd;
 }
 
 void MinimizeButtonMetrics::OnHWNDActivated() {
   was_activated_ = true;
   // NOTE: we don't cache here as it seems only after the activate is the value
   // correct.
 }
@@ skipping 51 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;
 }