Views which cannot process events cannot be tooltip targets

Views which cannot process events cannot be tooltip targets

The check for CanProcessEventsWithinSubtree() needs
to be made on |this| before the loop (along with
HitTestRect()) instead of inside the loop on the
children. Otherwise it would be possible to return
a view for which CanProcessEventsWithinSubtree()
returns false.

BUG=382873, 378530
TEST=Added coverage in ViewTest.CanProcessEventsWithinSubtree

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

[tdanderson, 2014-06-10 17:08:20]

Sadrul, can you please take a look? When landing this code last week I
mistakenly put the check inside the loop for the tooltips case when it should
have been outside (in the same place as HitTestRect()).

[sadrul, 2014-06-12 10:50:20]

You should add a test for this change.

[tdanderson, 2014-06-12 18:55:57]

Sadrul, I added test coverage for the early return. Can you please take another
look?

[sadrul, 2014-06-12 20:28:50]

LGTM

https://codereview.chromium.org/327843002/diff/20001/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/327843002/diff/20001/ui/views/view.cc#newcode852
ui/views/view.cc:852: if (!child->CanProcessEventsWithinSubtree())
Should this also move up to the beginning of the function?

[tdanderson, 2014-06-12 23:33:11]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-06-12 23:34:23]

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

[tdanderson, 2014-06-12 23:37:03]

https://codereview.chromium.org/327843002/diff/20001/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/327843002/diff/20001/ui/views/view.cc#newcode852
ui/views/view.cc:852: if (!child->CanProcessEventsWithinSubtree())
On 2014/06/12 20:28:49, sadrul wrote:
> Should this also move up to the beginning of the function?

No, since that would change existing behaviour. CPEWS() is replacing
HitTestRect() in cases where location is not important, but to preserve existing
behaviour they both still need to be called at the same places. Since HTR() is
inside the loop here, CPEWS() should be as well. (Similarly for
View::GetTooltipHandlerForPoint(), which is what this CL is fixing.)

Does that seem logical?

[commit-bot: I haz the power, 2014-06-13 00:53:16]

Change committed as 276873