Exclude caption button bounds from non-client frame hit testing

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

Index: chrome/browser/ui/views/frame/glass_browser_frame_view.cc
diff --git a/chrome/browser/ui/views/frame/glass_browser_frame_view.cc b/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
index 64f28ac79fffb97eb8023698a873f66fa1ab2cc2..a5d6595234351bfa7b641cf0224fd6af480ba90d 100644
--- a/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
+++ b/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
@@ -21,6 +21,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/signin_header_helper.h"
 #include "components/signin/core/common/profile_management_switches.h"
+#include "dwmapi.h"

[Peter Kasting, 2016/07/18 19:12:15]

This should be in <> and moved up above the <utility> #include.

[kylix_rd, 2016/07/18 20:35:10]

Done.

 #include "grit/theme_resources.h"
 #include "skia/ext/image_operations.h"
 #include "ui/base/material_design/material_design_controller.h"
@@ -28,6 +29,7 @@
 #include "ui/base/theme_provider.h"
 #include "ui/display/win/dpi.h"
 #include "ui/gfx/canvas.h"
+#include "ui/gfx/geometry/dip_util.h"
 #include "ui/gfx/icon_util.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/scoped_canvas.h"
@@ -70,6 +72,10 @@ const int kNewTabCaptionMaximizedSpacing = 16;
 // TODO(bsep): Windows 10 caption buttons look very different and we would like
 // the profile switcher button to match on that platform.
 const int kProfileSwitcherButtonHeight = 20;
+// There is a small one-pixel strip right above the caption buttons in which the
+// sizable frame "peeks" through. This will ensure that the behavior is

[Peter Kasting, 2016/07/18 19:12:16]

Nit: sizable frame -> resize border

I would omit the second sentence and do any necessary explanation thereof at the
call site where this is used.

[kylix_rd, 2016/07/18 20:35:10]

Done.

+// consistent with most Windows applications.
+const int kCaptionButtonTopInset = 1;
 
 // Converts the |image| to a Windows icon and returns the corresponding HICON
 // handle. |image| is resized to desired |width| and |height| if needed.
@@ -245,6 +251,28 @@ int GlassBrowserFrameView::NonClientHitTest(const gfx::Point& point) {
   if (frame_component != HTNOWHERE)
     return frame_component;
 
+  // On Windows 8+, the caption buttons are almost butted up to the top right
+  // of the window. This code will ensure that the mouse isn't set to a size

[Peter Kasting, 2016/07/18 19:12:15]

Nit: of -> corner of

[kylix_rd, 2016/07/18 20:35:10]

Done.

+  // cursor while hovering over the caption buttons, thus giving the incorrect
+  // impression that the user can resize the window.
+  if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
+    RECT button_bounds = {0};
+    const HWND hwnd = views::HWNDForWidget(frame());

[Peter Kasting, 2016/07/18 19:12:15]

Nit: I'd probably just inline this into the next statement

[kylix_rd, 2016/07/18 20:35:10]

Done.

+

[Peter Kasting, 2016/07/18 19:12:16]

Nit: No blank here

[kylix_rd, 2016/07/18 20:35:10]

Done.

+    if (SUCCEEDED(DwmGetWindowAttribute(hwnd, DWMWA_CAPTION_BUTTON_BOUNDS,
+                                        &button_bounds,
+                                        sizeof(button_bounds)))) {
+      // Even though the tiny area above the caption buttons is very difficult
+      // to hit, this ensures behavioral consistency with other top-level
+      // windows.

[Peter Kasting, 2016/07/18 19:12:15]

Nit: This comment really belongs on the Inset() call.  We should probably give
more detail about how this works, e.g. does Windows normally include the resize
border portion within the caption button bounds, and is it 1 px regardless of
scale factor?  The latter question also relates to my next comment.

[kylix_rd, 2016/07/18 20:35:10]

Done.

+      gfx::Rect buttons = gfx::ConvertRectToDIP(display::win::GetDPIScale(),
+                                                gfx::Rect(button_bounds));
+      buttons.Inset(0, kCaptionButtonTopInset, 0, 0);

[Peter Kasting, 2016/07/18 19:12:16]

Is it correct to inset after scaling?  It's hard for me to work out the effects
of different choices on different scale factors.

[kylix_rd, 2016/07/18 20:35:10]

From my (admittedly empirical) investigation, it does indeed seem to be a
constant 1px regardless of overall DSF. On my Surface Book set to 300% scaling,
the area above the buttons which hit-test as a sizable region seems to still be
1px.

+      if (buttons.Contains(point))
+        return (HTNOWHERE);

[Peter Kasting, 2016/07/18 19:12:16]

Should this be HTCAPTION?

Nit: No parens

[kylix_rd, 2016/07/18 20:35:10]

I want to ensure that subsequent processing lands at DefWindowProc which will do
the OS-level hit-testing. Making this HTCAPTION may cause odd follow-on effects,
such as disabling the caption buttons and make clicking on them move the window
rather than their intended function.

+    }
+  }
+
   int top_border_thickness = FrameTopBorderThickness(false);
   // We want the resize corner behavior to apply to the kResizeCornerWidth
   // pixels at each end of the top and bottom edges.  Because |point|'s x