Hit-test should not descend into iframe unnecessarily

Hit-test should not descend into iframe unnecessarily

LayoutObject::hitTest() calls into nodeAtPoint() 4 times
with different HitTestAction options: HitTestForeground, HitTestFloat,
HitTestChildBlockBackgrounds, HitTestBlockBackground.

LayoutPart::nodeAtPoint() descends into iframe hit-test
disregard of HitTestAction options. For rect-based hit-test,
if the test rect is not totally within the iframe bounds,
the iframe hit test will return false for the rect-based
hit test search to continue, which means we will call into
iframe hit test 4 times unnecessarily.

Lets make the change that LayoutPart::nodeAtPoint() descends
into iframe only in the HitTestForeground phase. If we do
hit the iframe, LayoutObject::hitTest() will terminate at
the HitTestForeground phase, If iframe does not hit, we
only descend into it once and would have collected enough
information (e.g. pushing overlapping nodes into hit list).

BUG=520730
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200793

[Yufeng Shen (Slow to review), 2015-08-18 16:38:29]

miletus@chromium.org changed reviewers:
+ dtapuska@chromium.org, rbyers@chromium.org

[dtapuska, 2015-08-18 20:15:10]

On 2015/08/18 16:38:35, Yufeng Shen wrote:

Although I do think this is correct I think it might have a better understanding
if it actually overloaded hitTest() in LayoutObject. I understand that is not a
virtual; but it would have a cleaner understanding if it did.

I'm going to defer to someone who knows the CSS Painting model better than I
like pdr@

[dtapuska, 2015-08-18 20:15:30]

dtapuska@chromium.org changed reviewers:
+ pdr@chromium.org

[Rick Byers, 2015-08-18 21:59:22]

On 2015/08/18 20:15:10, dtapuska wrote:
> On 2015/08/18 16:38:35, Yufeng Shen wrote:
> 
> Although I do think this is correct I think it might have a better
understanding
> if it actually overloaded hitTest() in LayoutObject. I understand that is not
a
> virtual; but it would have a cleaner understanding if it did.
> 
> I'm going to defer to someone who knows the CSS Painting model better than I
> like pdr@

Yeah I also don't know this well enough to have a ton of confidence.  pdr@ do
you?

[pdr., 2015-08-19 02:56:34]

On 2015/08/18 at 21:59:22, rbyers wrote:
> On 2015/08/18 20:15:10, dtapuska wrote:
> > On 2015/08/18 16:38:35, Yufeng Shen wrote:
> > 
> > Although I do think this is correct I think it might have a better
understanding
> > if it actually overloaded hitTest() in LayoutObject. I understand that is
not a
> > virtual; but it would have a cleaner understanding if it did.
> > 
> > I'm going to defer to someone who knows the CSS Painting model better than I
> > like pdr@
> 
> Yeah I also don't know this well enough to have a ton of confidence.  pdr@ do
you?

LGTM

LayoutSVGForeignObject uses a similar trick (though hittesting there is broken
in other ways).

[Yufeng Shen (Slow to review), 2015-08-19 03:09:18]

The CQ bit was checked by miletus@chromium.org

[commit-bot: I haz the power, 2015-08-19 03:09:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1302583002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1302583002/1

[commit-bot: I haz the power, 2015-08-19 04:16:40]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200793