Do not call into EventTargeter methods from ViewTargeter

ui/views/view_targeter.cc

 // Copyright 2014 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/view_targeter.h"
 
 #include "ui/events/event_target.h"
 #include "ui/views/focus/focus_manager.h"
 #include "ui/views/view.h"
 #include "ui/views/view_targeter_delegate.h"
 
 namespace views {
 
 ViewTargeter::ViewTargeter(ViewTargeterDelegate* delegate)
     : delegate_(delegate) {
   DCHECK(delegate_);
 }
 
 ViewTargeter::~ViewTargeter() {}
 
 bool ViewTargeter::DoesIntersectRect(const View* target,
                                      const gfx::Rect& rect) const {
   return delegate_->DoesIntersectRect(target, rect);
 }
 
 View* ViewTargeter::TargetForRect(View* root, const gfx::Rect& rect) const {
   return delegate_->TargetForRect(root, rect);
 }
 
-gfx::RectF ViewTargeter::BoundsForEvent(const ui::LocatedEvent& event) const {
-  gfx::RectF event_bounds(event.location_f(), gfx::SizeF(1, 1));
-  if (event.IsGestureEvent()) {
-    const ui::GestureEvent& gesture = *(event.AsGestureEvent());
-    event_bounds = gesture.details().bounding_box_f();
-  }
-
-  return event_bounds;
-}
-
 ui::EventTarget* ViewTargeter::FindTargetForEvent(ui::EventTarget* root,
                                                   ui::Event* event) {
   View* view = static_cast<View*>(root);
+
   if (event->IsKeyEvent())
     return FindTargetForKeyEvent(view, *static_cast<ui::KeyEvent*>(event));
-  else if (event->IsScrollEvent())
-    return EventTargeter::FindTargetForEvent(root, event);
+
+  if (event->IsScrollEvent()) {
+    return FindTargetForScrollEvent(view,
+                                    *static_cast<ui::ScrollEvent*>(event));
+  }
 
   NOTREACHED() << "ViewTargeter does not yet support this event type.";
   return NULL;
 }
 
 ui::EventTarget* ViewTargeter::FindNextBestTarget(
     ui::EventTarget* previous_target,
     ui::Event* event) {
   return previous_target->GetParentTarget();
 }
 
-bool ViewTargeter::SubtreeCanAcceptEvent(
-    ui::EventTarget* target,
-    const ui::LocatedEvent& event) const {
-  views::View* view = static_cast<views::View*>(target);
-  if (!view->visible())
-    return false;
-
-  if (!view->CanProcessEventsWithinSubtree())
-    return false;
-
-  return true;
-}
-
-bool ViewTargeter::EventLocationInsideBounds(
-    ui::EventTarget* target,
-    const ui::LocatedEvent& event) const {
-  views::View* view = static_cast<views::View*>(target);
-  gfx::Rect rect(event.location(), gfx::Size(1, 1));
-  gfx::RectF rect_in_view_coords_f(rect);
-  if (view->parent())
-    View::ConvertRectToTarget(view->parent(), view, &rect_in_view_coords_f);
-  gfx::Rect rect_in_view_coords = gfx::ToEnclosingRect(rect_in_view_coords_f);
-
-  // TODO(tdanderson): Don't call into HitTestRect() directly here.
-  //                   See crbug.com/370579.
-  return view->HitTestRect(rect_in_view_coords);
-}
-
-View* ViewTargeter::FindTargetForKeyEvent(View* view, const ui::KeyEvent& key) {
-  if (view->GetFocusManager())
-    return view->GetFocusManager()->GetFocusedView();
+View* ViewTargeter::FindTargetForKeyEvent(View* root, const ui::KeyEvent& key) {
+  if (root->GetFocusManager())
+    return root->GetFocusManager()->GetFocusedView();
   return NULL;
 }
 
-}  // namespace aura
+View* ViewTargeter::FindTargetForScrollEvent(View* root,
+                                             const ui::ScrollEvent& scroll) {
+  return root->GetEventHandlerForPoint(scroll.location());

[sadrul, 2014/07/29 18:06:50]

This should directly call into the delegate, something like:

  return root->GetEffectiveViewTargeter()->TargetForRect(...)

This would normally just do:

  Targeter* targeter = root->targeter();
  if (!targeter)
    targeter = this;
  return targeter->TargetForRect(...);

But because the 'effective' view-targeter goes upto the root-view, we should
look it up from |root|.

[tdanderson, 2014/07/29 19:40:45]

Is your suggestion about correctness or is it about design? Because I think I
disagree either way. The code should be correct as-is, and as a matter of design
I would rather call into View::GetEventHandlerForPoint() (which everyone else
already does anyway) and keep call sites of ViewTargeter::TargetForRect() to a
minimum.

[tdanderson, 2014/08/01 15:53:07]

I forget the outcome of our in-person chat about this. Is
this OK to leave as-is or is it something we definitely
need changed to your suggestion?

+}
+
+}  // namespace views