Remove remaining overrides of View::HitTestRect()

Remove remaining overrides of View::HitTestRect()

Remove the remaining overrides of View::HitTestRect() and
instead implement View::ViewTargeterDelegate::DoesIntersectRect().
Make View::HitTestRect() non-virtual.

BUG=388838
TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283312

[tdanderson, 2014-07-14 14:56:45]

Scott and Sadrul,

Here is the last CL needed in order to accomplish the goal of moving all
hit-testing out of View and into ViewTargeterDelegate. Would you rather these
new overrides be landed one by one, or is it ok to handle all of them in a
single CL? Also, I'd like to make all overrides of
ViewTargeterDelegate::DoesIntersectRect() private, since they should never be
called directly (only through the call in ViewTargeter::DoesIntersectRect()).
What do you think?

[sadrul, 2014-07-14 16:44:14]

LGTM

On 2014/07/14 14:56:45, tdanderson wrote:
> Scott and Sadrul,
> 
> Here is the last CL needed in order to accomplish the goal of moving all
> hit-testing out of View and into ViewTargeterDelegate. Would you rather these
> new overrides be landed one by one, or is it ok to handle all of them in a
> single CL?

Single CL looks reasonable to me.

> Also, I'd like to make all overrides of
> ViewTargeterDelegate::DoesIntersectRect() private, since they should never be
> called directly (only through the call in ViewTargeter::DoesIntersectRect()).
> What do you think?

Sounds OK to me. Would you have to friend ViewTargeter for ViewTargeterDelegate
in that case?

https://codereview.chromium.org/380813003/diff/80001/ui/views/window/non_clie...
File ui/views/window/non_client_view.cc (right):

https://codereview.chromium.org/380813003/diff/80001/ui/views/window/non_clie...
ui/views/window/non_client_view.cc:333: // Terry - the right place for this?
This should be the right place to install the default targeter, yeah

[tdanderson, 2014-07-14 17:28:42]

On 2014/07/14 16:44:14, sadrul wrote:
> LGTM
> 
> On 2014/07/14 14:56:45, tdanderson wrote:
> > Scott and Sadrul,
> > 
> > Here is the last CL needed in order to accomplish the goal of moving all
> > hit-testing out of View and into ViewTargeterDelegate. Would you rather
these
> > new overrides be landed one by one, or is it ok to handle all of them in a
> > single CL?
> 
> Single CL looks reasonable to me.
> 
> > Also, I'd like to make all overrides of
> > ViewTargeterDelegate::DoesIntersectRect() private, since they should never
be
> > called directly (only through the call in
ViewTargeter::DoesIntersectRect()).
> > What do you think?
> 
> Sounds OK to me. Would you have to friend ViewTargeter for
ViewTargeterDelegate
> in that case?

No, since DoesIntersectRect() is still public in ViewTargeterDelegate,
and ViewTargeter will only be calling in to that.

> 
>
https://codereview.chromium.org/380813003/diff/80001/ui/views/window/non_clie...
> File ui/views/window/non_client_view.cc (right):
> 
>
https://codereview.chromium.org/380813003/diff/80001/ui/views/window/non_clie...
> ui/views/window/non_client_view.cc:333: // Terry - the right place for this?
> This should be the right place to install the default targeter, yeah

[tdanderson, 2014-07-14 17:28:58]

https://codereview.chromium.org/380813003/diff/80001/ui/views/window/non_clie...
File ui/views/window/non_client_view.cc (right):

https://codereview.chromium.org/380813003/diff/80001/ui/views/window/non_clie...
ui/views/window/non_client_view.cc:333: // Terry - the right place for this?
On 2014/07/14 16:44:14, sadrul wrote:
> This should be the right place to install the default targeter, yeah

Sorry, left this in by mistake. Removed.

[tdanderson, 2014-07-15 20:18:02]

Scott, can you please take a look?

[sky, 2014-07-15 20:50:24]

Sorry, didn't realize you were waiting for me too: LGTM

[tdanderson, 2014-07-15 20:52:05]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-07-15 20:54:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdanderson@chromium.org/380813003/100001

[commit-bot: I haz the power, 2014-07-16 01:00:15]

Change committed as 283312