Fix strange touch behavior on Windows titlebar.

This patch fixes three bugs:
* Dragging the window via finger touch on the titlebar only works about
50% of the time.
* Touching the custom titlebar caption buttons would "leak" to the
window underneath, if the top window was closed or minimized.
* Touching and dragging the custom titlebar caption buttons would
cause further touch input on other widgets (like popup bubbles) to
not work.

This patch fixes them all at once by revising how we handle mouse events
synthesized from touch:
* We were letting all mouse events synthesized from touch fall through
as regular mouse events, unless they were targeted at HTCLIENT. They
would cause problems like setting mouse capture incorrectly (causing bug
#3) and cause double inputs (bug #2). This patch instead marks these
events as handled and ignores them, unless otherwise noted below.
* We now DefWindowProc events targeted at HTCAPTION and HTSYSMENU
instead of simply ignoring them, which fixes bug #1. Our special drag
handling in HandleMouseInputForCaption works poorly for touch.
* We also DefWindowProc events targeted at the caption buttons when
custom titlebar is off, which is required for them to work.
* When we do the hittest for the above targeting, we no longer convert
LPARAM to window coordinates before sending WM_NCHITTEST, because the
point should remain in screen coordinates. This was causing all three
bugs to reproduce inconsistently, since we would sometimes get HTCLIENT
or HTNOWHERE for events clearly targeting non-client area.

This patch doesn't specifically address the broken pen touch case, but
as a side effect it makes dragging the window with the pen work slightly
more often.

Bug: 838870, 832291, 843486
Change-Id: Ie7d339f7b1b75acb14377d8411234d8faa16755f
Reviewed-on: https://chromium-review.googlesource.com/1048877
Commit-Queue: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560353}

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

@@ -266,12 +266,12 @@ int BrowserViewLayout::NonClientHitTest(const gfx::Point& point) {
   gfx::Point point_in_browser_view_coords(point);
   views::View::ConvertPointToTarget(
       parent, browser_view_, &point_in_browser_view_coords);
-  gfx::Point test_point(point);
 
   // Determine if the TabStrip exists and is capable of being clicked on. We
   // might be a popup window without a TabStrip.
   if (delegate_->IsTabStripVisible()) {
     // See if the mouse pointer is within the bounds of the TabStrip.
+    gfx::Point test_point(point);
     if (ConvertedHitTest(parent, tab_strip_, &test_point)) {
       if (tab_strip_->IsPositionInWindowCaption(test_point))
         return HTCAPTION;
@@ -295,10 +295,9 @@ int BrowserViewLayout::NonClientHitTest(const gfx::Point& point) {
   // If the point's y coordinate is below the top of the toolbar and otherwise
   // within the bounds of this view, the point is considered to be within the
   // client area.
-  gfx::Rect bv_bounds = browser_view_->bounds();
-  bv_bounds.Offset(0, toolbar_->y());
-  bv_bounds.set_height(bv_bounds.height() - toolbar_->y());
-  if (bv_bounds.Contains(point))
+  gfx::Rect bounds_from_toolbar_top = browser_view_->bounds();
+  bounds_from_toolbar_top.Inset(0, toolbar_->y(), 0, 0);
+  if (bounds_from_toolbar_top.Contains(point))
     return HTCLIENT;
 
   // If the point's y coordinate is above the top of the toolbar, but not
@@ -311,9 +310,9 @@ int BrowserViewLayout::NonClientHitTest(const gfx::Point& point) {
   // cause the window controls not to work. So we return HTNOWHERE so that the
   // caller will hit-test the window controls before finally falling back to
   // HTCAPTION.
-  bv_bounds = browser_view_->bounds();
-  bv_bounds.set_height(toolbar_->y());
-  if (bv_bounds.Contains(point))
+  gfx::Rect tabstrip_background_bounds = browser_view_->bounds();
+  tabstrip_background_bounds.set_height(toolbar_->y());
+  if (tabstrip_background_bounds.Contains(point))
     return HTNOWHERE;
 
   // If the point is somewhere else, delegate to the default implementation.

ui/views/win/hwnd_message_handler.cc

@@ -239,6 +239,12 @@ ui::EventType GetTouchEventType(POINTER_FLAGS pointer_flags) {
   return ui::ET_TOUCH_MOVED;
 }
 
+bool IsHitTestOnResizeHandle(LRESULT hittest) {
+  return hittest == HTRIGHT || hittest == HTLEFT || hittest == HTTOP ||
+         hittest == HTBOTTOM || hittest == HTTOPLEFT || hittest == HTTOPRIGHT ||
+         hittest == HTBOTTOMLEFT || hittest == HTBOTTOMRIGHT;
+}
+
 const int kTouchDownContextResetTimeout = 500;
 
 // Windows does not flag synthesized mouse messages from touch or pen in all
@@ -2699,25 +2705,28 @@ LRESULT HWNDMessageHandler::HandleMouseEventInternal(UINT message,
                                                      WPARAM w_param,
                                                      LPARAM l_param,
                                                      bool track_mouse) {
-  // We handle touch events on Windows Aura. Windows generates synthesized
-  // mouse messages in response to touch which we should ignore. However touch
-  // messages are only received for the client area. We need to ignore the
-  // synthesized mouse messages for all points in the client area and places
-  // which return HTNOWHERE.
-  // TODO(ananta)
-  // Windows does not reliably set the touch flag on mouse messages. Look into
-  // a better way of identifying mouse messages originating from touch.
-  if ((message != WM_MOUSEWHEEL && message != WM_MOUSEHWHEEL) &&
-      (ui::IsMouseEventFromTouch(message))) {
-    LPARAM l_param_ht = l_param;
-    // For mouse events (except wheel events), location is in window coordinates
-    // and should be converted to screen coordinates for WM_NCHITTEST.
-    POINT screen_point = CR_POINT_INITIALIZER_FROM_LPARAM(l_param_ht);
-    MapWindowPoints(hwnd(), HWND_DESKTOP, &screen_point, 1);
-    l_param_ht = MAKELPARAM(screen_point.x, screen_point.y);
-
-    LRESULT hittest = SendMessage(hwnd(), WM_NCHITTEST, 0, l_param_ht);
-    if (hittest == HTCLIENT || hittest == HTNOWHERE)
+  // We handle touch events in Aura. Windows generates synthesized mouse
+  // messages whenever there's a touch, but it doesn't give us the actual touch
+  // messages if it thinks the touch point is in non-client space.
+  if (message != WM_MOUSEWHEEL && message != WM_MOUSEHWHEEL &&
+      ui::IsMouseEventFromTouch(message)) {
+    LRESULT hittest = SendMessage(hwnd(), WM_NCHITTEST, 0, l_param);
+    // Always DefWindowProc on the titlebar. We could let the event fall through
+    // and the special handling in HandleMouseInputForCaption would take care of
+    // this, but in the touch case Windows does a better job.
+    if (hittest == HTCAPTION || hittest == HTSYSMENU)
+      SetMsgHandled(FALSE);
+    // We must let Windows handle the caption buttons if it's drawing them, or
+    // they won't work.
+    if (delegate_->GetFrameMode() == FrameMode::SYSTEM_DRAWN &&
+        (hittest == HTCLOSE || hittest == HTMINBUTTON ||
+         hittest == HTMAXBUTTON)) {
+      SetMsgHandled(FALSE);
+    }
+    // Let resize events fall through. Ignore everything else, as we're either
+    // letting Windows handle it above or we've already handled the equivalent
+    // touch message.
+    if (!IsHitTestOnResizeHandle(hittest))
       return 0;
   }