[Regression] Selections are incorrectly invalidated

[Regression] Selections are incorrectly invalidated

https://codereview.chromium.org/570763003 removed the wrong
codepath when selecting: |clipToVisibleContent| was true in
most cases and we removed this codepath.

This change just re-introduce the incorrectly removed
codepath. Most baselines are just reverted to their original
ones prior to the change. The SVG changes are new and
corresponds to the selection box bleeding out of the box,
which is a better behavior.

BUG=417242, 416186
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182697

[Julien - ping for review, 2014-09-24 21:17:08]

jchaffraix@chromium.org changed reviewers:
+ leviw@chromium.org, pdr@chromium.org, yosin@chromium.org

[Julien - ping for review, 2014-09-24 21:17:09]

Yosin / Levi, as selection reviewers (and original reviewer of the regressing
change).

Philip, to double-check the SVG baseline changes.

[pdr., 2014-09-24 21:29:53]

On 2014/09/24 21:17:09, Julien Chaffraix - PST wrote:
> Yosin / Levi, as selection reviewers (and original reviewer of the regressing
> change).
> 
> Philip, to double-check the SVG baseline changes.

This is pretty hard to review without seeing the image diff view :( From a high
level the new SVG baselines look fine to me (they are not horribly broken). I am
a bit concerned about the new non-svg expectations where the repaint rect
actually shrinks a lot though (e.g.,
LayoutTests/platform/linux/fast/repaint/text-selection-rect-in-overflow-2-expected.txt).
What's happening there?

[leviw_travelin_and_unemployed, 2014-09-24 21:32:29]

lgtm for the code change. I'll wait until pdr evaluates the image results after
the layout test bots have uploaded.

[pdr., 2014-09-24 21:40:12]

I also don't understand why there are svg rebaselines at all when neither patch
affects svg.

[Julien - ping for review, 2014-09-24 22:45:16]

On 2014/09/24 at 21:29:53, pdr wrote:
> On 2014/09/24 21:17:09, Julien Chaffraix - PST wrote:
> > Yosin / Levi, as selection reviewers (and original reviewer of the
regressing
> > change).
> > 
> > Philip, to double-check the SVG baseline changes.
> 
> This is pretty hard to review without seeing the image diff view :( From a
high level the new SVG baselines look fine to me (they are not horribly broken).


Yeah, unfortunately we don't have better tooling :-/

The SVG baselines changed by 1px: if you look at the red selection box, the old
baselines have only 3 edges when the new ones have the full box. That's also the
reason why unrelated SVG tests are changing. We didn't touch SVG rendering, they
just happen to have some selection that gets clipped differently.

> I am a bit concerned about the new non-svg expectations where the repaint rect
actually shrinks a lot though (e.g.,
LayoutTests/platform/linux/fast/repaint/text-selection-rect-in-overflow-2-expected.txt).
What's happening there?

The non-SVG rebaselines are just bringing the old expectations: those are the
opposite of what landed with https://codereview.chromium.org/570763003,
including the follow-up rebaseline.

[pdr., 2014-09-25 01:32:39]

On 2014/09/24 22:45:16, Julien Chaffraix - PST wrote:
> On 2014/09/24 at 21:29:53, pdr wrote:
> > On 2014/09/24 21:17:09, Julien Chaffraix - PST wrote:
> > > Yosin / Levi, as selection reviewers (and original reviewer of the
> regressing
> > > change).
> > > 
> > > Philip, to double-check the SVG baseline changes.
> > 
> > This is pretty hard to review without seeing the image diff view :( From a
> high level the new SVG baselines look fine to me (they are not horribly
broken).
> 
> 
> Yeah, unfortunately we don't have better tooling :-/
> 
> The SVG baselines changed by 1px: if you look at the red selection box, the
old
> baselines have only 3 edges when the new ones have the full box. That's also
the
> reason why unrelated SVG tests are changing. We didn't touch SVG rendering,
they
> just happen to have some selection that gets clipped differently.
> 
> > I am a bit concerned about the new non-svg expectations where the repaint
rect
> actually shrinks a lot though (e.g.,
>
LayoutTests/platform/linux/fast/repaint/text-selection-rect-in-overflow-2-expected.txt).
> What's happening there?
> 
> The non-SVG rebaselines are just bringing the old expectations: those are the
> opposite of what landed with https://codereview.chromium.org/570763003,
> including the follow-up rebaseline.

Thank you for the explanation. LGTM

[pdr., 2014-09-25 01:44:18]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2014-09-25 01:44:53]

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

[commit-bot: I haz the power, 2014-09-25 03:14:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-25 03:14:16]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/24644)

[Julien - ping for review, 2014-09-25 16:48:17]

On 2014/09/25 at 03:14:16, commit-bot wrote:
> Try jobs failed on following builders:
>   mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/24644)

The new test wasn't cross-platform due to the use of text. Updated it to use
Ahem.

[Julien - ping for review, 2014-09-25 16:48:22]

The CQ bit was checked by jchaffraix@chromium.org

[commit-bot: I haz the power, 2014-09-25 16:49:45]

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

[commit-bot: I haz the power, 2014-09-25 18:09:02]

Committed patchset #3 (id:40001) as 182697