Reland "We need to account for culled inline parents of hit-tested nodes"

Reland "We need to account for culled inline parents of hit-tested nodes"

There are two kind of problem.
One is that we get the wrong hit-test result from culled inlines
because we try to overrun checking them until top of parent.
The other is that mouse events aren't fired when there is the
element culled inline like label element that doesn't has any
background color or border or self-layer.

This patch makes hit-test include not only area-based hit-tests
but also point-based ones at the place handling culled inline.

More information: https://docs.google.com/presentation/d/1KlT_FQxCTGeH-lPWjzXnPdZ3Bb1p7GL-IwoP-SfGvEo/edit#slide=id.g535467cad_40

BUG=312199, 435483
R=pdr@chromium.org
TEST=fast/events/hit-test-culled_inline.html
    =fast/dom/nodesFromRect/nodesFromRect-culled-inlines-between-silblings.html

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

[Miyoung Shin(g), 2015-02-22 09:09:07]

myid.o.shin@gmail.com changed reviewers:
- leviw@chromium.org, rbyers@chromium.org

[Miyoung Shin(g), 2015-02-22 09:13:36]

pdr@, PTAL.

We can re-land this patch reverted at http://crrev.com/914573003/.
The issue that reverted this was solved at http://crrev.com/929883003/.

[pdr., 2015-02-23 23:18:34]

On 2015/02/22 at 09:13:36, myid.o.shin wrote:
> pdr@, PTAL.
> 
> We can re-land this patch reverted at http://crrev.com/914573003/.
> The issue that reverted this was solved at http://crrev.com/929883003/.

We know this will break users that incorrectly use firstLineBox instead of
firstLineBoxIncludingCulling. Can you investigate all callers of firstLineBox to
ensure that we are using the correct call?

[Miyoung Shin(g), 2015-02-24 01:48:04]

On 2015/02/23 23:18:34, pdr wrote:
> On 2015/02/22 at 09:13:36, myid.o.shin wrote:
> > pdr@, PTAL.
> > 
> > We can re-land this patch reverted at http://crrev.com/914573003/.
> > The issue that reverted this was solved at http://crrev.com/929883003/.
> 
> We know this will break users that incorrectly use firstLineBox instead of
> firstLineBoxIncludingCulling. Can you investigate all callers of firstLineBox
to
> ensure that we are using the correct call?

OK, I will do.

[Miyoung Shin(g), 2015-03-31 16:28:17]

myid.o.shin@gmail.com changed reviewers:
+ tonikitoo@gmail.com

[Miyoung Shin(g), 2015-03-31 16:46:00]

@pdr,
I couldn't find any doubt caller of firstLineBox.

One of my doubt caller was in RenderInline::localCaretRect.
But I realized that we don't need to change firstLineBox to
firstLineBoxIncludingCulling.
Because the culled inline has the outline style when the some text is selected,
we changes the culled inline to the normal inline(creating inline) before
calling RenderInline::localCaretRect. 
That means it's imposible to call RenderInline::localCaretRect in case of culled
inline.

I think, basically, we should use firstLineBoxIncludingCulling if LayoutInline
is the culled inline,
and it is possible to be called from hitTest or be directly called by JS. It's
already handled.

BTW, I added @tonikitoo who reviewed the culled inline's patch for the first
time on WebKit. I believe he can give the good comment for this patch.

[Miyoung Shin(g), 2015-04-04 06:49:41]

On 2015/03/31 16:46:00, Miyoung Shin(g) wrote:
> @pdr,
> I couldn't find any doubt caller of firstLineBox.
> 
> One of my doubt caller was in RenderInline::localCaretRect.
> But I realized that we don't need to change firstLineBox to
> firstLineBoxIncludingCulling.
> Because the culled inline has the outline style when the some text is
selected,
> we changes the culled inline to the normal inline(creating inline) before
> calling RenderInline::localCaretRect. 
> That means it's imposible to call RenderInline::localCaretRect in case of
culled
> inline.
> 
> I think, basically, we should use firstLineBoxIncludingCulling if LayoutInline
> is the culled inline,
> and it is possible to be called from hitTest or be directly called by JS. It's
> already handled.
> 
> BTW, I added @tonikitoo who reviewed the culled inline's patch for the first
> time on WebKit. I believe he can give the good comment for this patch.

I've rebased this due to naming change to LayoutInline.cpp

[pdr., 2015-04-07 22:26:30]

pdr@chromium.org changed reviewers:
+ dsinclair@chromium.org, eae@chromium.org

[pdr., 2015-04-07 22:30:21]

+cc eae and dsinclair

This is a good patch and fixes a real bug but there's a high chance that it will
cause  regressions or changes in behavior. When we landed this last time, I
ended up sinking a full day into rolling out patches because myid.o.shin is not
a committer.

Would one of you be up for reviewing this and shepherding this patch?

[eae, 2015-04-09 21:29:54]

On 2015/04/07 22:30:21, pdr wrote:
> +cc eae and dsinclair
> 
> This is a good patch and fixes a real bug but there's a high chance that it
will
> cause  regressions or changes in behavior. When we landed this last time, I
> ended up sinking a full day into rolling out patches because myid.o.shin is
not
> a committer.
> 
> Would one of you be up for reviewing this and shepherding this patch?

OK. Once all your comments have been addressed I'll gladly take over.

[pdr., 2015-04-10 05:13:05]

On 2015/04/09 at 21:29:54, eae wrote:
> On 2015/04/07 22:30:21, pdr wrote:
> > +cc eae and dsinclair
> > 
> > This is a good patch and fixes a real bug but there's a high chance that it
will
> > cause  regressions or changes in behavior. When we landed this last time, I
> > ended up sinking a full day into rolling out patches because myid.o.shin is
not
> > a committer.
> > 
> > Would one of you be up for reviewing this and shepherding this patch?
> 
> OK. Once all your comments have been addressed I'll gladly take over.

I think this patch is good to go. Caveat emptor: I thought it looked good the
first time too. I'm not sure anyone fully understands culled inlines so we're
all learning with this one.

[eae, 2015-04-13 17:56:20]

LGTM

[Miyoung Shin(g), 2015-04-14 13:47:18]

On 2015/04/13 17:56:20, eae wrote:
> LGTM

Thanks, Can I land this ?

[pdr., 2015-04-14 18:08:59]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2015-04-14 18:09:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/947793002/20001

[commit-bot: I haz the power, 2015-04-14 19:49:21]

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