Combine outline and focus ring code paths

Combine outline and focus ring code paths

Previously we paint outlines and focus rings in very different paths.
- Focus rings are painted in the way that we collect all rects first
  then combine the rects into a enclosing path, then paint the path.
- Outlines are painted by each box, line box, continuations, causing
  difficulties when handling the joints. We have issues of broken
  joints, unconnected segments, missing segments, etc.

With this CL:
- We paint outlines and focus rings in the same way: collect rects first
  paint the path. This resolves the original issues of broken joints and
  unconnected segments, etc.;
- Don't need to propagate outline style to continuations because the
  outline owner paints the whole outline. This enables us to paint a
  whole connected outline enclosing the continuations.

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

[Xianzhu, 2015-08-07 03:28:51]

Patchset #11 (id:200001) has been deleted

[Xianzhu, 2015-08-07 16:39:51]

wangxianzhu@chromium.org changed reviewers:
+ chrishtr@chromium.org, leviw@chromium.org, pdr@chromium.org

[Xianzhu, 2015-08-07 16:39:52]

This is the final CL based on the following preparation CLs:
- https://codereview.chromium.org/1269123002/ (addFocusRingRects -->
addOutlineRects) (landed)
- https://codereview.chromium.org/1280543005/ (refactor
LayoutInline::addOutlineRects)
- https://codereview.chromium.org/1278543002/ (include whole outline into visual
overflow)

It's a long-standing bug and the work is not urgent at all. Feel free to review
it in the following month or even months when you have time :)

[eae, 2015-08-12 20:39:13]

eae@chromium.org changed reviewers:
+ eae@chromium.org
- leviw@chromium.org

[eae, 2015-08-12 20:39:14]

LGTM for layout and test changes, please have chrishtr or pdr review the paint
changes as they're more familiar with that part of the code.

[pdr., 2015-08-27 18:32:42]

This is great. Can you rebase this after
https://codereview.chromium.org/1321613002 lands? Even though this patch looks
complicated, it's really pretty simple and I think we can land it today or
tomorrow.

[pdr., 2015-08-27 22:59:33]

LGTM

The actual drawing code could use a little cleanup, but that's mostly just moved
in this patch so I think we should leave it.

@chrishtr, does this LGTY2?

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/Obje...
File Source/core/paint/ObjectPainter.cpp (right):

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/Obje...
Source/core/paint/ObjectPainter.cpp:22: void paintNonAutoOutlineFastPath(const
PaintInfo& paintInfo, const IntRect& rect, const ComputedStyle& style, const
Color& color)
How "outline:auto" affects this was confusing to me at first. Can you add a
comment above here describing this a little more? Maybe something like:
// Fast path for painting a single outline.

We may want to change the name as well. Maybe just "paintSingleOutlineFastPath"?

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/Obje...
Source/core/paint/ObjectPainter.cpp:24: LayoutRect inner(rect);
Can you assert that the style is not auto here?

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/Obje...
Source/core/paint/ObjectPainter.cpp:100: void
paintNonAutoOutlineSlowPath(GraphicsContext& graphicsContext, const
Vector<IntRect> rects, const ComputedStyle& style, const Color& color)
Same comments here as above.

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/SVGC...
File Source/core/paint/SVGContainerPainter.cpp (left):

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/SVGC...
Source/core/paint/SVGContainerPainter.cpp:65:
ObjectPainter(m_layoutSVGContainer).paintOutline(outlinePaintInfo,
m_layoutSVGContainer.outlineVisualOverflowRect(), layoutBoundingBox.size(),
layoutBoundingBox.location());
We can remove LayoutSVGModelObject::outlineVisualOverflowRect now too.

[Xianzhu, 2015-08-28 00:26:04]

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/Obje...
File Source/core/paint/ObjectPainter.cpp (right):

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/Obje...
Source/core/paint/ObjectPainter.cpp:22: void paintNonAutoOutlineFastPath(const
PaintInfo& paintInfo, const IntRect& rect, const ComputedStyle& style, const
Color& color)
On 2015/08/27 22:59:33, pdr wrote:
> How "outline:auto" affects this was confusing to me at first. Can you add a
> comment above here describing this a little more? Maybe something like:
> // Fast path for painting a single outline.
> 
> We may want to change the name as well. Maybe just
"paintSingleOutlineFastPath"?

Renamed it to paintSingleRectangleOutline, and paintNonAutoOutlineSlowPath to
paintComplexOutline.

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/Obje...
Source/core/paint/ObjectPainter.cpp:24: LayoutRect inner(rect);
On 2015/08/27 22:59:33, pdr wrote:
> Can you assert that the style is not auto here?

Done.

This seems less confusing to include "NonAuto" in the function names.

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/Obje...
Source/core/paint/ObjectPainter.cpp:100: void
paintNonAutoOutlineSlowPath(GraphicsContext& graphicsContext, const
Vector<IntRect> rects, const ComputedStyle& style, const Color& color)
On 2015/08/27 22:59:33, pdr wrote:
> Same comments here as above.

Done.

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/SVGC...
File Source/core/paint/SVGContainerPainter.cpp (left):

https://codereview.chromium.org/1224933002/diff/300001/Source/core/paint/SVGC...
Source/core/paint/SVGContainerPainter.cpp:65:
ObjectPainter(m_layoutSVGContainer).paintOutline(outlinePaintInfo,
m_layoutSVGContainer.outlineVisualOverflowRect(), layoutBoundingBox.size(),
layoutBoundingBox.location());
On 2015/08/27 22:59:33, pdr wrote:
> We can remove LayoutSVGModelObject::outlineVisualOverflowRect now too.

Done.

[pdr., 2015-08-28 00:33:02]

LGTM 2

[Xianzhu, 2015-08-28 04:12:41]

Patchset #19 (id:370001) has been deleted

[Xianzhu, 2015-08-28 04:12:54]

Patchset #17 (id:300002) has been deleted

[Xianzhu, 2015-08-28 04:13:01]

Patchset #16 (id:320001) has been deleted

[Xianzhu, 2015-08-28 18:26:55]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-08-28 18:26:55]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/1224933002/#ps390001
(title: "Expectations for virtual/spv2/fast/repaint")

[commit-bot: I haz the power, 2015-08-28 18:27:12]

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

[commit-bot: I haz the power, 2015-08-28 18:28:38]

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

[commit-bot: I haz the power, 2015-08-28 18:28:39]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Xianzhu, 2015-08-28 18:35:17]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-08-28 18:35:17]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
eae@chromium.org
Link to the patchset: https://codereview.chromium.org/1224933002/#ps410001
(title: "Rebase")

[commit-bot: I haz the power, 2015-08-28 18:35:38]

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

[commit-bot: I haz the power, 2015-08-28 18:38:54]

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

[commit-bot: I haz the power, 2015-08-28 18:38:55]

Try jobs failed on following builders:
  linux_arm_compile on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_arm_compi...)

[Xianzhu, 2015-08-31 16:23:03]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-08-31 16:23:04]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org,
eae@chromium.org
Link to the patchset: https://codereview.chromium.org/1224933002/#ps430001
(title: "Fix empty focus ring on Mac")

[commit-bot: I haz the power, 2015-08-31 16:23:17]

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

[commit-bot: I haz the power, 2015-08-31 17:33:40]

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

[commit-bot: I haz the power, 2015-08-31 17:33:42]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Xianzhu, 2015-08-31 18:31:17]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2015-08-31 18:31:28]

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

[commit-bot: I haz the power, 2015-08-31 19:11:49]

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