Apply both phsical bounding box and clip to composited bounds.

Apply both phsical bounding box and clip to composited bounds.

Before this patch and the one that was the proximate cause of issue 674098,
clips for composited bounds worked like this:
1. If there was a clip between the compositing container and a PaintLayer, the
clip would be that PaintLayer's contribution to composited bounds.

2.  If there was no such clip, we would fall back to physicalBoundingBox, and
also recurse into children.

After the patch causing issue 674098, we would always recurse, even in case 1.

This patch observes that we can always use a local bound of the intersection of
any clip with physicalBoundingBox. Recursion still always happens.

This avoids situations like in issue 674098 where skew matrices yielded bounds
much too large.

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/009b4fbe8df4067bf05dd1e3d6d3ca9be31d6363
Cr-Commit-Position: refs/heads/master@{#440929}

[chrishtr, 2016-12-20 22:18:15]

Description was changed from

==========
none


none


none

BUG=
==========

to

==========
Fix geometry mapping in LinkHighlightImpl

BUG=
==========

[chrishtr, 2016-12-20 22:18:17]

The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-20 22:18:46]

Dry run: 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-12-21 00:12:02]

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

[commit-bot: I haz the power, 2016-12-21 00:12:03]

Dry run: 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_...)

[chrishtr, 2016-12-27 19:37:58]

The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 19:38:13]

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

[chrishtr, 2016-12-27 19:38:38]

Description was changed from

==========
Fix geometry mapping in LinkHighlightImpl

BUG=
==========

to

==========
Fix geometry mapping in LinkHighlightImpl

BUG=674098
==========

[commit-bot: I haz the power, 2016-12-27 21:09:32]

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

[commit-bot: I haz the power, 2016-12-27 21:09:33]

Dry run: 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_...)

[chrishtr, 2016-12-27 23:31:03]

Description was changed from

==========
Fix geometry mapping in LinkHighlightImpl

BUG=674098
==========

to

==========
Fix geometry mapping in LinkHighlightImpl

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-12-27 23:46:03]

The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 23:46:11]

Dry run: 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-12-28 00:16:49]

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

[commit-bot: I haz the power, 2016-12-28 00:16:50]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[chrishtr, 2016-12-28 01:38:57]

The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 01:39:13]

Dry run: 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-12-28 02:47:38]

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

[commit-bot: I haz the power, 2016-12-28 02:47:39]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)

[chrishtr, 2016-12-28 23:12:11]

Description was changed from

==========
Fix geometry mapping in LinkHighlightImpl

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Apply both phsical bounding box and clip to composited bounds.

Before this patch and the one that was the proximate cause of issue 674098,
physicalBoundingBox would be used if there was no clip, and descendant bounding
boxes, even if they exceeded the clip

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-12-28 23:15:32]

Description was changed from

==========
Apply both phsical bounding box and clip to composited bounds.

Before this patch and the one that was the proximate cause of issue 674098,
physicalBoundingBox would be used if there was no clip, and descendant bounding
boxes, even if they exceeded the clip

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Apply both phsical bounding box and clip to composited bounds.

Before this patch and the one that was the proximate cause of issue 674098,
clips for composited bounds worked like this:
1. If there was a clip between the compositing container and a PaintLayer, the
clip would be that PaintLayer's contribution to composited bounds. 

2.  If there was no such clip, we would fall back to physicalBoundingBox, and 
also recurse into children.

After the patch causing issue 674098, we would always recurse, even in case 1.

This patch observes that we can always use a local bound of the intersection of
any clip with physicalBoundingBox. Recursion still always happens.

This avoids situations like in issue 674098 where skew matrices yielded bounds
much too large.

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2016-12-28 23:16:27]

The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 23:16:45]

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

[chrishtr, 2016-12-28 23:17:26]

chrishtr@chromium.org changed reviewers:
+ wangxianzhu@chromium.org

[chrishtr, 2016-12-28 23:17:27]

The changes to layout tests are mostly empty composited layers (some width but
no height,
or vice-versa) getting truly empty bounds (isEmpty() == true). The others are
noticing that
the product of css clip and physical bounds can be applied. I verified pixel
output locally
looks ok also.

[Xianzhu, 2016-12-28 23:49:41]

How do too large bounds cause the bug?

[chrishtr, 2016-12-28 23:56:38]

https://codereview.chromium.org/2591883002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2591883002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:2570: result =
transform()->mapRect(result);
Before this patch, the input to this mapRect call could be larger than the
physical size of the PaintLayer and its descendants.

[Xianzhu, 2016-12-29 00:09:01]

The CL lgtm. Just want to understand how the bug occurs.

https://codereview.chromium.org/2591883002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2591883002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:2570: result =
transform()->mapRect(result);
On 2016/12/28 23:56:38, chrishtr wrote:
> Before this patch, the input to this mapRect call could be larger than the
> physical size of the PaintLayer and its descendants.

How does the larger rect cause wrong position of the highlight?

[commit-bot: I haz the power, 2016-12-29 00:44:43]

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

[commit-bot: I haz the power, 2016-12-29 00:44:44]

Dry run: 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_...)

[chrishtr, 2016-12-29 01:04:07]

On 2016/12/29 at 00:09:01, wangxianzhu wrote:
> The CL lgtm. Just want to understand how the bug occurs.
> 
>
https://codereview.chromium.org/2591883002/diff/200001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):
> 
>
https://codereview.chromium.org/2591883002/diff/200001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/paint/PaintLayer.cpp:2570: result =
transform()->mapRect(result);
> On 2016/12/28 23:56:38, chrishtr wrote:
> > Before this patch, the input to this mapRect call could be larger than the
> > physical size of the PaintLayer and its descendants.
> 
> How does the larger rect cause wrong position of the highlight?

On reflection, I realized I didn't know the answer fully myself. I just
discovered that
the reason is the difference in offsetFromLayoutObject between m_graphicsLayer
and
m_scrollingContentsLayer. I have another patch which fixes that in
LinkHighlightImpl,
but I think this CL has value regardless.

I'll remove the added test here and put it in the other CL.

[chrishtr, 2016-12-29 01:08:35]

Updated. Looks ok?

https://codereview.chromium.org/2591883002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2591883002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:2570: result =
transform()->mapRect(result);
On 2016/12/29 at 00:09:01, Xianzhu wrote:
> On 2016/12/28 23:56:38, chrishtr wrote:
> > Before this patch, the input to this mapRect call could be larger than the
> > physical size of the PaintLayer and its descendants.
> 
> How does the larger rect cause wrong position of the highlight?

The entire graphics layer has an incorrect offsetFromLayoutObject.

[chrishtr, 2016-12-29 01:08:40]

The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-29 01:08:56]

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

[Xianzhu, 2016-12-29 01:18:54]

lgtm

[commit-bot: I haz the power, 2016-12-29 02:46:56]

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

[commit-bot: I haz the power, 2016-12-29 02:46:57]

Dry run: This issue passed the CQ dry run.

[chrishtr, 2016-12-29 02:51:01]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2016-12-29 02:51:10]

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-12-29 02:54:58]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1482979861914460,
"parent_rev": "24bffaad2be8dd781cc1f6277296070c250f1c67", "commit_rev":
"dfc6efc2065ddbe8ff45cebd61c329fbb927e4cf"}

[commit-bot: I haz the power, 2016-12-29 02:55:23]

Description was changed from

==========
Apply both phsical bounding box and clip to composited bounds.

Before this patch and the one that was the proximate cause of issue 674098,
clips for composited bounds worked like this:
1. If there was a clip between the compositing container and a PaintLayer, the
clip would be that PaintLayer's contribution to composited bounds. 

2.  If there was no such clip, we would fall back to physicalBoundingBox, and 
also recurse into children.

After the patch causing issue 674098, we would always recurse, even in case 1.

This patch observes that we can always use a local bound of the intersection of
any clip with physicalBoundingBox. Recursion still always happens.

This avoids situations like in issue 674098 where skew matrices yielded bounds
much too large.

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Apply both phsical bounding box and clip to composited bounds.

Before this patch and the one that was the proximate cause of issue 674098,
clips for composited bounds worked like this:
1. If there was a clip between the compositing container and a PaintLayer, the
clip would be that PaintLayer's contribution to composited bounds. 

2.  If there was no such clip, we would fall back to physicalBoundingBox, and 
also recurse into children.

After the patch causing issue 674098, we would always recurse, even in case 1.

This patch observes that we can always use a local bound of the intersection of
any clip with physicalBoundingBox. Recursion still always happens.

This avoids situations like in issue 674098 where skew matrices yielded bounds
much too large.

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2591883002
==========

[commit-bot: I haz the power, 2016-12-29 02:55:25]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2017-01-02 15:51:31]

Description was changed from

==========
Apply both phsical bounding box and clip to composited bounds.

Before this patch and the one that was the proximate cause of issue 674098,
clips for composited bounds worked like this:
1. If there was a clip between the compositing container and a PaintLayer, the
clip would be that PaintLayer's contribution to composited bounds. 

2.  If there was no such clip, we would fall back to physicalBoundingBox, and 
also recurse into children.

After the patch causing issue 674098, we would always recurse, even in case 1.

This patch observes that we can always use a local bound of the intersection of
any clip with physicalBoundingBox. Recursion still always happens.

This avoids situations like in issue 674098 where skew matrices yielded bounds
much too large.

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2591883002
==========

to

==========
Apply both phsical bounding box and clip to composited bounds.

Before this patch and the one that was the proximate cause of issue 674098,
clips for composited bounds worked like this:
1. If there was a clip between the compositing container and a PaintLayer, the
clip would be that PaintLayer's contribution to composited bounds.

2.  If there was no such clip, we would fall back to physicalBoundingBox, and
also recurse into children.

After the patch causing issue 674098, we would always recurse, even in case 1.

This patch observes that we can always use a local bound of the intersection of
any clip with physicalBoundingBox. Recursion still always happens.

This avoids situations like in issue 674098 where skew matrices yielded bounds
much too large.

BUG=674098
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/009b4fbe8df4067bf05dd1e3d6d3ca9be31d6363
Cr-Commit-Position: refs/heads/master@{#440929}
==========

[commit-bot: I haz the power, 2017-01-02 15:51:32]

Patchset 12 (id:??) landed as
https://crrev.com/009b4fbe8df4067bf05dd1e3d6d3ca9be31d6363
Cr-Commit-Position: refs/heads/master@{#440929}