Fix mouse hit detection for hosted app titlebar elements

This CL adds the HostedAppButtonContainer to GlassBrowserFrameView's
hit detection to ensure the correct mouse signal is sent to the OS.
This fixes a bug in Windows 10 causing certain mouse movements to
deactivate the page info dialog. See screencasts.

Before: https://bugs.chromium.org/p/chromium/issues/attachment?aid=352755&signed_aid=IbjUxm1YHiMINVDpeUwCXQ==&inline=1
After: https://bugs.chromium.org/p/chromium/issues/attachment?aid=352756&signed_aid=-NF8jWWjNwiA1KrUMeuVkQ==&inline=1

Bug: 864386
Change-Id: Ic706784026431d603b44c1a80302fe0f92ee5868
Reviewed-on: https://chromium-review.googlesource.com/1170664
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584366}

ash/frame/caption_buttons/frame_caption_button.cc

@@ -17,7 +17,7 @@
 #include "ui/views/animation/ink_drop_impl.h"
 #include "ui/views/animation/ink_drop_mask.h"
 #include "ui/views/animation/ink_drop_ripple.h"
-#include "ui/views/view_properties.h"
+#include "ui/views/window/hit_test_utils.h"
 
 namespace ash {
 
@@ -79,7 +79,7 @@ FrameCaptionButton::FrameCaptionButton(views::ButtonListener* listener,
       paint_as_active_(false),
       alpha_(255),
       swap_images_animation_(new gfx::SlideAnimation(this)) {
-  SetProperty(views::kHitTestComponentKey, hit_test_type);
+  views::SetHitTestComponent(this, hit_test_type);
 
   set_animate_on_state_change(true);
   swap_images_animation_->Reset(1);

ash/public/cpp/frame_border_hit_test.cc

@@ -11,6 +11,7 @@
 #include "ui/views/view_properties.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
+#include "ui/views/window/hit_test_utils.h"
 #include "ui/views/window/non_client_view.h"
 
 namespace ash {
@@ -51,20 +52,10 @@ int FrameBorderNonClientHitTest(views::NonClientFrameView* view,
 
   // Check if it intersects with children (frame caption button, back button,
   // etc.).
-  gfx::Point point_in_non_client_view(point_in_widget);
-  views::View::ConvertPointFromWidget(widget->non_client_view(),
-                                      &point_in_non_client_view);
-  for (views::View* target_view =
-           widget->non_client_view()->GetEventHandlerForPoint(
-               point_in_non_client_view);
-       target_view; target_view = target_view->parent()) {
-    int target_component =
-        target_view->GetProperty(views::kHitTestComponentKey);
-    if (target_component != HTNOWHERE)
-      return target_component;
-    if (target_view == widget->non_client_view())
-      break;
-  }
+  int hit_test_component =
+      views::GetHitTestComponent(widget->non_client_view(), point_in_widget);
+  if (hit_test_component != HTNOWHERE)
+    return hit_test_component;
 
   // Caption is a safe default.
   return HTCAPTION;

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

@@ -40,6 +40,7 @@
 #include "ui/views/resources/grit/views_resources.h"
 #include "ui/views/win/hwnd_util.h"
 #include "ui/views/window/client_view.h"
+#include "ui/views/window/hit_test_utils.h"
 
 using MD = ui::MaterialDesignController;
 
@@ -310,6 +311,15 @@ int GlassBrowserFrameView::NonClientHitTest(const gfx::Point& point) {
     return HTCLIENT;
   }
 
+  if (hosted_app_button_container_) {
+    // TODO(alancutter): Assign hit test components to all children and refactor
+    // this entire function call to just be GetHitTestComponent(this, point).
+    int hosted_app_component =
+        views::GetHitTestComponent(hosted_app_button_container_, point);
+    if (hosted_app_component != HTNOWHERE)
+      return hosted_app_component;
+  }
+
   int frame_component = frame()->client_view()->NonClientHitTest(point);
   const int client_border_thickness = ClientBorderThickness(false);
 

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

@@ -28,8 +28,8 @@
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/layout_provider.h"
-#include "ui/views/view_properties.h"
 #include "ui/views/widget/native_widget_aura.h"
+#include "ui/views/window/hit_test_utils.h"
 
 namespace {
 
@@ -186,8 +186,7 @@ HostedAppButtonContainer::ContentSettingsContainer::ContentSettingsContainer(
     image_view->SetBorder(views::CreateEmptyBorder(
         gfx::Insets(kContentSettingIconInteriorPadding)));
     image_view->disable_animation();
-    image_view->SetProperty(views::kHitTestComponentKey,
-                            static_cast<int>(HTCLIENT));
+    views::SetHitTestComponent(image_view.get(), static_cast<int>(HTCLIENT));
     content_setting_views_.push_back(image_view.get());
     AddChildView(image_view.release());
   }
@@ -229,16 +228,16 @@ HostedAppButtonContainer::HostedAppButtonContainer(views::Widget* widget,
 
   AddChildView(hosted_app_origin_text_);
 
-  content_settings_container_->SetProperty(views::kHitTestComponentKey,
-                                           static_cast<int>(HTCLIENT));
+  views::SetHitTestComponent(content_settings_container_,
+                             static_cast<int>(HTCLIENT));
   AddChildView(content_settings_container_);
 
-  page_action_icon_container_view_->SetProperty(views::kHitTestComponentKey,
-                                                static_cast<int>(HTCLIENT));
+  views::SetHitTestComponent(page_action_icon_container_view_,
+                             static_cast<int>(HTCLIENT));
   AddChildView(page_action_icon_container_view_);
 
-  browser_actions_container_->SetProperty(views::kHitTestComponentKey,
-                                          static_cast<int>(HTCLIENT));
+  views::SetHitTestComponent(browser_actions_container_,
+                             static_cast<int>(HTCLIENT));
   AddChildView(browser_actions_container_);
   AddChildView(app_menu_button_);
 

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

@@ -19,13 +19,13 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/menu_button.h"
-#include "ui/views/view_properties.h"
+#include "ui/views/window/hit_test_utils.h"
 
 constexpr int kMenuHighlightFadeDurationMs = 800;
 
 HostedAppMenuButton::HostedAppMenuButton(BrowserView* browser_view)
     : AppMenuButton(this), browser_view_(browser_view) {
-  SetProperty(views::kHitTestComponentKey, static_cast<int>(HTMENU));
+  views::SetHitTestComponent(this, static_cast<int>(HTMENU));
 
   SetInkDropMode(InkDropMode::ON);
   // Disable focus ring for consistency with sibling buttons and AppMenuButton.

ui/views/BUILD.gn

@@ -244,6 +244,7 @@ jumbo_component("views") {
     "window/dialog_observer.h",
     "window/frame_background.h",
     "window/frame_buttons.h",
+    "window/hit_test_utils.h",
     "window/native_frame_view.h",
     "window/non_client_view.h",
     "window/window_button_order_provider.h",
@@ -418,6 +419,7 @@ jumbo_component("views") {
     "window/dialog_client_view.cc",
     "window/dialog_delegate.cc",
     "window/frame_background.cc",
+    "window/hit_test_utils.cc",
     "window/native_frame_view.cc",
     "window/non_client_view.cc",
     "window/window_button_order_provider.cc",
@@ -1005,6 +1007,7 @@ source_set("views_unittests_sources") {
     "window/custom_frame_view_unittest.cc",
     "window/dialog_client_view_unittest.cc",
     "window/dialog_delegate_unittest.cc",
+    "window/hit_test_utils_unittest.cc",
     "window/non_client_view_unittest.cc",
     "window/window_resize_utils_unittest.cc",
   ]

ui/views/window/hit_test_utils.cc

+// Copyright 2018 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 "ui/views/window/hit_test_utils.h"
+
+#include "ui/base/hit_test.h"
+#include "ui/gfx/geometry/point.h"
+#include "ui/views/view.h"
+#include "ui/views/view_properties.h"
+
+namespace views {
+
+int GetHitTestComponent(View* view, const gfx::Point& point_in_widget) {
+  gfx::Point point_in_view(point_in_widget);
+  View::ConvertPointFromWidget(view, &point_in_view);
+
+  if (!view->GetLocalBounds().Contains(point_in_view))
+    return HTNOWHERE;
+
+  View* target_view = view->GetEventHandlerForPoint(point_in_view);
+  while (target_view) {
+    int component = target_view->GetProperty(kHitTestComponentKey);
+    if (component != HTNOWHERE)
+      return component;
+    if (target_view == view)
+      break;
+    target_view = target_view->parent();
+  }
+
+  return HTNOWHERE;
+}
+
+void SetHitTestComponent(View* view, int hit_test_id) {
+  view->SetProperty(kHitTestComponentKey, hit_test_id);
+}
+
+}  // namespace views

ui/views/window/hit_test_utils.h

+// Copyright 2018 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.
+
+#ifndef UI_VIEWS_WINDOW_HIT_TEST_UTILS_H_
+#define UI_VIEWS_WINDOW_HIT_TEST_UTILS_H_
+
+#include "ui/views/views_export.h"
+
+namespace gfx {
+class Point;
+}
+
+namespace views {
+
+class View;
+
+// Returns the inner most non-HTNOWHERE |kHitTestComponentKey| value at
+// |point_in_widget| within the hierarchy of |view|, otherwise returns
+// HTNOWHERE.
+VIEWS_EXPORT int GetHitTestComponent(View* view,
+                                     const gfx::Point& point_in_widget);
+
+// Sets the |kHitTestComponentKey| property of |view|.
+VIEWS_EXPORT void SetHitTestComponent(View* view, int hit_test_id);
+
+}  // namespace views
+
+#endif  // UI_VIEWS_WINDOW_HIT_TEST_UTILS_H_

ui/views/window/hit_test_utils_unittest.cc

+// Copyright 2018 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 "ui/views/window/hit_test_utils.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/hit_test.h"
+#include "ui/views/test/views_test_base.h"
+#include "ui/views/view.h"
+#include "ui/views/view_properties.h"
+#include "ui/views/widget/widget.h"
+
+namespace views {
+
+using GetHitTestComponentTest = ViewsTestBase;
+
+TEST_F(GetHitTestComponentTest, BasicTests) {
+  Widget* widget = new Widget;
+  widget->Init(CreateParams(Widget::InitParams::TYPE_WINDOW));
+
+  // Testing arrangement diagram:
+  // *=============root:HTCLIENT=============*
+  // | *=left:HTLEFT=* *=nowhere:HTNOWHERE=* |
+  // | |             | | *=right:HTRIGHT=* | |
+  // | |             | | |               | | |
+  // | |             | | *===============* | |
+  // | *=============* *===================* |
+  // *=======================================*
+  View* root = widget->GetRootView();
+  root->SetProperty(views::kHitTestComponentKey, static_cast<int>(HTCLIENT));
+  root->SetBounds(0, 0, 100, 100);
+
+  View* left = new View;
+  left->SetProperty(views::kHitTestComponentKey, static_cast<int>(HTLEFT));
+  left->SetBounds(10, 10, 30, 80);
+  root->AddChildView(left);
+
+  View* nowhere = new View;
+  nowhere->SetBounds(60, 10, 30, 80);
+  root->AddChildView(nowhere);
+
+  View* right = new View;
+  right->SetProperty(views::kHitTestComponentKey, static_cast<int>(HTRIGHT));
+  right->SetBounds(10, 10, 10, 60);
+  nowhere->AddChildView(right);
+
+  // Hit the root view.
+  EXPECT_EQ(GetHitTestComponent(root, gfx::Point(50, 50)), HTCLIENT);
+
+  // Hit the left view.
+  EXPECT_EQ(GetHitTestComponent(root, gfx::Point(25, 50)), HTLEFT);
+
+  // Hit the nowhere view, should return the root view's value.
+  EXPECT_EQ(GetHitTestComponent(root, gfx::Point(65, 50)), HTCLIENT);
+
+  // Hit the right view.
+  EXPECT_EQ(GetHitTestComponent(root, gfx::Point(75, 50)), HTRIGHT);
+
+  // Hit outside the root view.
+  EXPECT_EQ(GetHitTestComponent(root, gfx::Point(200, 50)), HTNOWHERE);
+
+  widget->CloseNow();
+}
+
+}  // namespace views