Calculate correct cull rect for SVG inline text boxes.

Calculate correct cull rect for SVG inline text boxes.

Cull rects were larger than needed when text was selected.

Identified while investigating test failure of:

svg/text/text-selection-align-06-b.svg

as part of http://crrev.com/2073563002 for crbug.com/616600, though
fixing these did not actually fix any issues for that patch or bug.

BUG=627233
Committed: https://crrev.com/3f0f1c6a0edb31e2ec3ce4dbb21acc83301982ca
Cr-Commit-Position: refs/heads/master@{#405675}

[wkorman, 2016-07-09 00:10:25]

Description was changed from

==========
Fix cull rect for SVG inline text boxes.
==========

to

==========
Fix cull rect for SVG inline text boxes.

Incorrect cull rects identified while investigating test failure of:

svg/text/text-selection-align-06-b.svg

as part of http://crrev.com/2073563002 for crbug.com/616600, though
fixing these did not actually fix any issues for that patch or bug.
==========

[wkorman, 2016-07-09 00:11:32]

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

[wkorman, 2016-07-09 00:11:33]

Minimal test case we were looking at was:

<?xml version="1.0" encoding="UTF-8"?>
<svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink" width="400px" height="400px">
  <text x="50" y="30" writing-mode="tb">x</text>
<script>
if (window.testRunner)
    window.testRunner.dumpSelectionRect();
var range = document.createRange();
range.selectNode(window.document.documentElement);
var selection = window.getSelection();
selection.removeAllRanges();
selection.addRange(range);
</script>
</svg>

[pdr., 2016-07-11 18:41:09]

https://codereview.chromium.org/2137753002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp (right):

https://codereview.chromium.org/2137753002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp:50: if
(includeSelectionRect) {
I looked into this and it turns out that the code is not needed.
includeSelectionRect is poorly named and should be called
includeSelectionRectForMarkers. It turns out that the marker code only paints
within the bounds above, so this code isn't useful. Refactoring this to be
correct (i.e., still work if the marker bounds change in the future) seems very
hard though :( I couldn't think of a way to test this directly.

[wkorman, 2016-07-12 22:38:05]

PTAL, I took this as an opportunity to write a trivial RenderingTest. Passes on
both Linux/Windows though I expected to have to switch rect asserts to use
approximation due to font differences.

Had to add CORE_EXPORT to DOMSelection to allow linking which I believe is ok
but let me know if otherwise.

https://codereview.chromium.org/2137753002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp (right):

https://codereview.chromium.org/2137753002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp:50: if
(includeSelectionRect) {
On 2016/07/11 at 18:41:09, pdr. wrote:
> I looked into this and it turns out that the code is not needed.
includeSelectionRect is poorly named and should be called
includeSelectionRectForMarkers. It turns out that the marker code only paints
within the bounds above, so this code isn't useful. Refactoring this to be
correct (i.e., still work if the marker bounds change in the future) seems very
hard though :( I couldn't think of a way to test this directly.

Discussed in person briefly, it still seemed worth fixing the current broken
unnecessarily-large cull rect and thought was a unit test could work out.

[pdr., 2016-07-12 22:51:41]

Looks great! LGTM

Please add BUG=627233

https://codereview.chromium.org/2137753002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainterTest.cpp
(right):

https://codereview.chromium.org/2137753002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainterTest.cpp:101: // The
selection rect is one pixel taller due to sub-pixel difference
This may be a bug.

[wkorman, 2016-07-12 23:32:02]

Description was changed from

==========
Fix cull rect for SVG inline text boxes.

Incorrect cull rects identified while investigating test failure of:

svg/text/text-selection-align-06-b.svg

as part of http://crrev.com/2073563002 for crbug.com/616600, though
fixing these did not actually fix any issues for that patch or bug.
==========

to

==========
Fix cull rect for SVG inline text boxes.

Incorrect cull rects identified while investigating test failure of:

svg/text/text-selection-align-06-b.svg

as part of http://crrev.com/2073563002 for crbug.com/616600, though
fixing these did not actually fix any issues for that patch or bug.

BUG=627233
==========

[wkorman, 2016-07-12 23:32:45]

Description was changed from

==========
Fix cull rect for SVG inline text boxes.

Incorrect cull rects identified while investigating test failure of:

svg/text/text-selection-align-06-b.svg

as part of http://crrev.com/2073563002 for crbug.com/616600, though
fixing these did not actually fix any issues for that patch or bug.

BUG=627233
==========

to

==========
Calculate correct cull rect for SVG inline text boxes.

Cull rects were larger than needed when text was selected.

Identified while investigating test failure of:

svg/text/text-selection-align-06-b.svg

as part of http://crrev.com/2073563002 for crbug.com/616600, though
fixing these did not actually fix any issues for that patch or bug.

BUG=627233
==========

[wkorman, 2016-07-12 23:33:20]

Apologies, didn't mean to swipe your bug! Hope you had not started work on it
already.

https://codereview.chromium.org/2137753002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainterTest.cpp
(right):

https://codereview.chromium.org/2137753002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainterTest.cpp:101: // The
selection rect is one pixel taller due to sub-pixel difference
On 2016/07/12 at 22:51:41, pdr. wrote:
> This may be a bug.

Yeah, let's discuss in person soon separately. I think it just means the cull
rect could be one pixel bigger than it needs to be when selected. But it could
also be correct to be so given anti-aliasing.

[wkorman, 2016-07-12 23:36:37]

The CQ bit was checked by wkorman@chromium.org

[commit-bot: I haz the power, 2016-07-12 23:39:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 01:16:25]

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

[commit-bot: I haz the power, 2016-07-13 01:16:26]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[wkorman, 2016-07-15 00:52:03]

The CQ bit was checked by wkorman@chromium.org

[wkorman, 2016-07-15 00:52:03]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2137753002/#ps40001
(title: "Rework test to search list for painting rather than via explicit
index.")

[commit-bot: I haz the power, 2016-07-15 00:52:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 02:09:00]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-15 02:09:05]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-15 02:11:47]

Description was changed from

==========
Calculate correct cull rect for SVG inline text boxes.

Cull rects were larger than needed when text was selected.

Identified while investigating test failure of:

svg/text/text-selection-align-06-b.svg

as part of http://crrev.com/2073563002 for crbug.com/616600, though
fixing these did not actually fix any issues for that patch or bug.

BUG=627233
==========

to

==========
Calculate correct cull rect for SVG inline text boxes.

Cull rects were larger than needed when text was selected.

Identified while investigating test failure of:

svg/text/text-selection-align-06-b.svg

as part of http://crrev.com/2073563002 for crbug.com/616600, though
fixing these did not actually fix any issues for that patch or bug.

BUG=627233

Committed: https://crrev.com/3f0f1c6a0edb31e2ec3ce4dbb21acc83301982ca
Cr-Commit-Position: refs/heads/master@{#405675}
==========

[commit-bot: I haz the power, 2016-07-15 02:11:48]

Patchset 3 (id:??) landed as
https://crrev.com/3f0f1c6a0edb31e2ec3ce4dbb21acc83301982ca
Cr-Commit-Position: refs/heads/master@{#405675}

[henrika (OOO until Aug 14), 2016-07-15 08:20:47]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2151203002/ by henrika@chromium.org.

The reason for reverting is: Most likely breaks:

SVGInlineTextBoxPainterTest.TextCullRect_DefaultWritingMode
SVGInlineTextBoxPainterTest.TextCullRect_WritingModeTopToBottom

https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Andr....