Merge 241955 "Make extensions icons easier to target with gestures"

> Make extensions icons easier to target with gestures
> 
> The bounds of a BrowserActionView and the bounds
> of its child view |button_| have the following
> properties:
> 
> - Both share a common center point.
> - The bounds of |button_| are square, but the 
> bounds of BAV are not.
> - The bounds of |button_| are contained within
> the bounds of BAV.
> - Both are small enough to be easily covered
> by a finger.
> 
> This is a flaw in rect-based targeting
> making it difficult to correctly target
> |button_| with a tap gesture. To obtain the
> desired behavior, use the distance
> between the center of the touch and the center
> of the view (instead of the center line of
> the view) to break ties among rect-based
> targeting candidates.
> 
> The use of the distance to center line was
> to avoid biases against wide or tall views. 
> But this is not necessary because any view 
> which can have at least 60% of its area 
> covered by a touch cannot be sufficiently wide 
> or tall enough to benefit from such a metric.
> 
> BUG=328182
> 
> Review URL: https://codereview.chromium.org/94003003

TBR=tdanderson@chromium.org

Review URL: https://codereview.chromium.org/108283008

git-svn-id: svn://svn.chromium.org/chrome/branches/1750/src@242156 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 102 insertions, 69 deletions

diff --git a/ui/views/rect_based_targeting_utils.cc b/ui/views/rect_based_targeting_utils.cc
index 2e267cc..de4d346 100644
--- a/ui/views/rect_based_targeting_utils.cc
+++ b/ui/views/rect_based_targeting_utils.cc
@@ -9,21 +9,6 @@
 
 namespace views {
 
-namespace {
-
-// The positive distance from |pos| to the nearest endpoint of the interval
-// [start, end] is returned if |pos| lies within the interval, otherwise
-// 0 is returned.
-int DistanceToInterval(int pos, int start, int end) {
-  if (pos < start)
-    return start - pos;
-  if (pos > end)
-    return pos - end;
-  return 0;
-}
-
-}  // namespace
-
 bool UsePointBasedTargeting(const gfx::Rect& rect) {
   return rect.width() == 1 && rect.height() == 1;
 }
@@ -37,21 +22,11 @@ float PercentCoveredBy(const gfx::Rect& rect_1, const gfx::Rect& rect_2) {
       static_cast<float>(intersect_area) / static_cast<float>(rect_1_area) : 0;
 }
 
-int DistanceSquaredFromCenterLineToPoint(const gfx::Point& point,
-                                         const gfx::Rect& rect) {
+int DistanceSquaredFromCenterToPoint(const gfx::Point& point,
+                                     const gfx::Rect& rect) {
   gfx::Point center_point = rect.CenterPoint();
   int dx = center_point.x() - point.x();
   int dy = center_point.y() - point.y();
-
-  if (rect.width() > rect.height()) {
-    dx = DistanceToInterval(point.x(),
-                            rect.x() + (rect.height() / 2),
-                            rect.right() - (rect.height() / 2));
-  } else {
-    dy = DistanceToInterval(point.y(),
-                            rect.y() + (rect.width() / 2),
-                            rect.bottom() - (rect.width() / 2));
-  }
   return (dx * dx) + (dy * dy);
 }
 
diff --git a/ui/views/rect_based_targeting_utils.h b/ui/views/rect_based_targeting_utils.h
index e3f9d11..94da5a5 100644
--- a/ui/views/rect_based_targeting_utils.h
+++ b/ui/views/rect_based_targeting_utils.h
@@ -21,11 +21,9 @@ VIEWS_EXPORT bool UsePointBasedTargeting(const gfx::Rect& rect);
 VIEWS_EXPORT float PercentCoveredBy(const gfx::Rect& rect_1,
                                     const gfx::Rect& rect_2);
 
-// Returns the square of the distance from |point| to the center line of
-// |rect|. The center line of a rectangle is obtained by repeatedly
-// stripping away 1px borders around the rectangle until a line remains.
-VIEWS_EXPORT int DistanceSquaredFromCenterLineToPoint(const gfx::Point& point,
-                                                      const gfx::Rect& rect);
+// Returns the square of the distance from |point| to the center of |rect|.
+VIEWS_EXPORT int DistanceSquaredFromCenterToPoint(const gfx::Point& point,
+                                                  const gfx::Rect& rect);
 
 }  // namespace views
 
diff --git a/ui/views/rect_based_targeting_utils_unittest.cc b/ui/views/rect_based_targeting_utils_unittest.cc
index 61e63e5..134119b 100644
--- a/ui/views/rect_based_targeting_utils_unittest.cc
+++ b/ui/views/rect_based_targeting_utils_unittest.cc
@@ -48,7 +48,7 @@ TEST(RectBasedTargetingUtils, PercentCoveredBy) {
   EXPECT_FLOAT_EQ(0.0f, PercentCoveredBy(rect_4, rect_2));
 }
 
-TEST(RectBasedTargetingUtils, DistanceSquaredFromCenterLineToPoint) {
+TEST(RectBasedTargetingUtils, DistanceSquaredFromCenterToPoint) {
   gfx::Rect rect_1(gfx::Point(0, 0), gfx::Size(10, 10));
   gfx::Rect rect_2(gfx::Point(20, 0), gfx::Size(80, 10));
   gfx::Rect rect_3(gfx::Point(0, 20), gfx::Size(10, 20));
@@ -58,20 +58,13 @@ TEST(RectBasedTargetingUtils, DistanceSquaredFromCenterLineToPoint) {
   gfx::Point point_3(11, 15);
   gfx::Point point_4(33, 44);
 
-  // The point lies on the center line of the rect.
-  EXPECT_EQ(0, DistanceSquaredFromCenterLineToPoint(point_1, rect_1));
-  EXPECT_EQ(0, DistanceSquaredFromCenterLineToPoint(point_2, rect_2));
-
-  // Distance from a point to the left/top endpoint of a rect's center line.
-  EXPECT_EQ(400, DistanceSquaredFromCenterLineToPoint(point_1, rect_2));
-  EXPECT_EQ(800, DistanceSquaredFromCenterLineToPoint(point_2, rect_3));
-  EXPECT_EQ(296, DistanceSquaredFromCenterLineToPoint(point_3, rect_2));
-
-  // Distance from a point to the center point of a square.
-  EXPECT_EQ(136, DistanceSquaredFromCenterLineToPoint(point_3, rect_1));
-
-  // Distance from a point to the bottom endpoint of a rect's center line.
-  EXPECT_EQ(865, DistanceSquaredFromCenterLineToPoint(point_4, rect_3));
+  EXPECT_EQ(0, DistanceSquaredFromCenterToPoint(point_1, rect_1));
+  EXPECT_EQ(1225, DistanceSquaredFromCenterToPoint(point_2, rect_2));
+  EXPECT_EQ(3025, DistanceSquaredFromCenterToPoint(point_1, rect_2));
+  EXPECT_EQ(1025, DistanceSquaredFromCenterToPoint(point_2, rect_3));
+  EXPECT_EQ(2501, DistanceSquaredFromCenterToPoint(point_3, rect_2));
+  EXPECT_EQ(136, DistanceSquaredFromCenterToPoint(point_3, rect_1));
+  EXPECT_EQ(980, DistanceSquaredFromCenterToPoint(point_4, rect_3));
 }
 
 }  // namespace views
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 0cec760..40ad7cc 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -875,8 +875,8 @@ View* View::GetEventHandlerForRect(const gfx::Rect& rect) {
       // |cur_view| is a suitable candidate for rect-based targeting.
       // Check to see if it is the closest suitable candidate so far.
       gfx::Point touch_center(rect.CenterPoint());
-      int cur_dist = views::DistanceSquaredFromCenterLineToPoint(
-          touch_center, cur_view_bounds);
+      int cur_dist = views::DistanceSquaredFromCenterToPoint(touch_center,
+                                                             cur_view_bounds);
       if (!rect_view || cur_dist < rect_view_distance) {
         rect_view = cur_view;
         rect_view_distance = cur_dist;
@@ -898,8 +898,8 @@ View* View::GetEventHandlerForRect(const gfx::Rect& rect) {
   gfx::Rect local_bounds(GetLocalBounds());
   if (views::PercentCoveredBy(local_bounds, rect) >= kRectTargetOverlap) {
     gfx::Point touch_center(rect.CenterPoint());
-    int cur_dist = views::DistanceSquaredFromCenterLineToPoint(touch_center,
-                                                               local_bounds);
+    int cur_dist = views::DistanceSquaredFromCenterToPoint(touch_center,
+                                                           local_bounds);
     if (!rect_view || cur_dist < rect_view_distance)
       rect_view = this;
   }
diff --git a/ui/views/view.h b/ui/views/view.h
index 848d847..dd298ed 100644
--- a/ui/views/view.h
+++ b/ui/views/view.h
@@ -577,6 +577,7 @@ class VIEWS_EXPORT View : public ui::LayerDelegate,
   // closest visible descendant having at least kRectTargetOverlap of
   // its area covered by |rect|. If no such descendant exists, return the
   // deepest visible descendant that contains the center point of |rect|.
+  // See http://goo.gl/3Jp2BD for more information about rect-based targeting.
   virtual View* GetEventHandlerForRect(const gfx::Rect& rect);
 
   // Returns the deepest visible descendant that contains the specified point
diff --git a/ui/views/view_unittest.cc b/ui/views/view_unittest.cc
index 1597f5d..77601b2 100644
--- a/ui/views/view_unittest.cc
+++ b/ui/views/view_unittest.cc
@@ -1058,6 +1058,9 @@ TEST_F(ViewTest, HitTestMasks) {
   widget->CloseNow();
 }
 
+// Tests the correctness of the rect-based targeting algorithm implemented in
+// View::GetEventHandlerForRect(). See http://goo.gl/3Jp2BD for a description
+// of rect-based targeting.
 TEST_F(ViewTest, GetEventHandlerForRect) {
   Widget* widget = new Widget;
   Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP);
@@ -1075,6 +1078,8 @@ TEST_F(ViewTest, GetEventHandlerForRect) {
   // v4 (300, 200, 100, 100)
   //     v41 (310, 210, 80, 80)
   //         v411 (370, 275, 10, 5)
+  // v5 (450, 197, 30, 36)
+  //     v51 (450, 200, 30, 30)
 
   // The coordinates used for SetBounds are in parent coordinates.
 
@@ -1110,6 +1115,14 @@ TEST_F(ViewTest, GetEventHandlerForRect) {
   v411->SetBounds(60, 65, 10, 5);
   v41->AddChildView(v411);
 
+  TestView* v5 = new TestView;
+  v5->SetBounds(450, 197, 30, 36);
+  root_view->AddChildView(v5);
+
+  TestView* v51 = new TestView;
+  v51->SetBounds(0, 3, 30, 30);
+  v5->AddChildView(v51);
+
   // |touch_rect| does not intersect any descendant view of |root_view|.
   gfx::Rect touch_rect(105, 105, 30, 45);
   View* result_view = root_view->GetEventHandlerForRect(touch_rect);
@@ -1143,14 +1156,14 @@ TEST_F(ViewTest, GetEventHandlerForRect) {
   result_view = NULL;
 
   // Covers both |v1| and |v2| by at least 60%, but the center point
-  // of |touch_rect| is closer to the center line of |v2|.
+  // of |touch_rect| is closer to the center point of |v2|.
   touch_rect.SetRect(20, 20, 400, 100);
   result_view = root_view->GetEventHandlerForRect(touch_rect);
   EXPECT_EQ(v2, result_view);
   result_view = NULL;
 
   // Covers both |v1| and |v2| by at least 60%, but the center point
-  // of |touch_rect| is closer to the center line (point) of |v1|.
+  // of |touch_rect| is closer to the center point of |v1|.
   touch_rect.SetRect(-700, -15, 1050, 110);
   result_view = root_view->GetEventHandlerForRect(touch_rect);
   EXPECT_EQ(v1, result_view);
@@ -1163,16 +1176,9 @@ TEST_F(ViewTest, GetEventHandlerForRect) {
   result_view = NULL;
 
   // Intersects |v3| and |v31| by at least 60% and the center point
-  // of |touch_rect| is closer to the center line of |v3|.
+  // of |touch_rect| is closer to the center point of |v31|.
   touch_rect.SetRect(0, 200, 110, 100);
   result_view = root_view->GetEventHandlerForRect(touch_rect);
-  EXPECT_EQ(v3, result_view);
-  result_view = NULL;
-
-  // Intersects |v3| and |v31| by at least 60% and the center point
-  // of |touch_rect| is equally close to the center lines of both.
-  touch_rect.SetRect(-60, 140, 200, 200);
-  result_view = root_view->GetEventHandlerForRect(touch_rect);
   EXPECT_EQ(v31, result_view);
   result_view = NULL;
 
@@ -1184,16 +1190,16 @@ TEST_F(ViewTest, GetEventHandlerForRect) {
   result_view = NULL;
 
   // Covers |v3|, |v31|, and |v32| all by at least 60%, and the
-  // center point of |touch_rect| is closest to the center line
-  // of |v3|.
+  // center point of |touch_rect| is closest to the center point
+  // of |v32|.
   touch_rect.SetRect(0, 200, 200, 100);
   result_view = root_view->GetEventHandlerForRect(touch_rect);
-  EXPECT_EQ(v3, result_view);
+  EXPECT_EQ(v32, result_view);
   result_view = NULL;
 
   // Intersects all of |v3|, |v31|, and |v32|, but only covers
   // |v31| and |v32| by at least 60%. The center point of
-  // |touch_rect| is closest to the center line of |v32|.
+  // |touch_rect| is closest to the center point of |v32|.
   touch_rect.SetRect(30, 225, 180, 115);
   result_view = root_view->GetEventHandlerForRect(touch_rect);
   EXPECT_EQ(v32, result_view);
@@ -1279,7 +1285,7 @@ TEST_F(ViewTest, GetEventHandlerForRect) {
 
   // Covers |v3|, |v4|, and all of their descendants by at
   // least 60%. The center point of |touch_rect| is closest
-  // to the center line of |v32|.
+  // to the center point of |v32|.
   touch_rect.SetRect(0, 200, 400, 100);
   result_view = root_view->GetEventHandlerForRect(touch_rect);
   EXPECT_EQ(v32, result_view);
@@ -1294,11 +1300,71 @@ TEST_F(ViewTest, GetEventHandlerForRect) {
   EXPECT_EQ(root_view, result_view);
   result_view = NULL;
 
-  // Covers all views by at least 60%. The center point of
-  // |touch_rect| is closest to the center line of |v2|.
+  // Covers all views (except |v5| and |v51|) by at least 60%. The
+  // center point of |touch_rect| is equally close to the center
+  // points of |v2| and |v32|. One is not a descendant of the other,
+  // so in this case the view selected is arbitrary (i.e.,
+  // it depends only on the ordering of nodes in the views
+  // hierarchy).
   touch_rect.SetRect(0, 0, 400, 300);
   result_view = root_view->GetEventHandlerForRect(touch_rect);
-  EXPECT_EQ(v2, result_view);
+  EXPECT_EQ(v32, result_view);
+  result_view = NULL;
+
+  // Covers |v5| and |v51| by at least 60%, and the center point of
+  // the touch is located within both views. Since both views share
+  // the same center point, the child view should be selected.
+  touch_rect.SetRect(440, 190, 40, 40);
+  result_view = root_view->GetEventHandlerForRect(touch_rect);
+  EXPECT_EQ(v51, result_view);
+  result_view = NULL;
+
+  // Covers |v5| and |v51| by at least 60%, but the center point of
+  // the touch is not located within either view. Since both views
+  // share the same center point, the child view should be selected.
+  touch_rect.SetRect(455, 187, 60, 60);
+  result_view = root_view->GetEventHandlerForRect(touch_rect);
+  EXPECT_EQ(v51, result_view);
+  result_view = NULL;
+
+  // Covers neither |v5| nor |v51| by at least 60%, but the center
+  // of the touch is located within |v51|.
+  touch_rect.SetRect(450, 197, 10, 10);
+  result_view = root_view->GetEventHandlerForRect(touch_rect);
+  EXPECT_EQ(v51, result_view);
+  result_view = NULL;
+
+  // Covers neither |v5| nor |v51| by at least 60% but intersects both.
+  // The center point is located outside of both views.
+  touch_rect.SetRect(433, 180, 24, 24);
+  result_view = root_view->GetEventHandlerForRect(touch_rect);
+  EXPECT_EQ(root_view, result_view);
+  result_view = NULL;
+
+  // Only intersects |v5| but does not cover it by at least 60%. The
+  // center point of the touch region is located within |v5|.
+  touch_rect.SetRect(449, 196, 3, 3);
+  result_view = root_view->GetEventHandlerForRect(touch_rect);
+  EXPECT_EQ(v5, result_view);
+  result_view = NULL;
+
+  // A mouse click within |v5| (but not |v51|) should target |v5|.
+  touch_rect.SetRect(462, 199, 1, 1);
+  result_view = root_view->GetEventHandlerForRect(touch_rect);
+  EXPECT_EQ(v5, result_view);
+  result_view = NULL;
+
+  // A mouse click |v5| and |v51| should target the child view.
+  touch_rect.SetRect(452, 226, 1, 1);
+  result_view = root_view->GetEventHandlerForRect(touch_rect);
+  EXPECT_EQ(v51, result_view);
+  result_view = NULL;
+
+  // A mouse click on the center of |v5| and |v51| should target
+  // the child view.
+  touch_rect.SetRect(465, 215, 1, 1);
+  result_view = root_view->GetEventHandlerForRect(touch_rect);
+  EXPECT_EQ(v51, result_view);
   result_view = NULL;
 
   widget->CloseNow();