Take CSS Clip and contain: paint into account when computing visual rects.

Take CSS Clip and contain: paint into account when computing visual rects.

BUG=611379, 599939
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/3d30a67e378ec3fcaac5809305584bbabca88e18
Cr-Commit-Position: refs/heads/master@{#411843}

[chrishtr, 2016-08-11 23:07:25]

Description was changed from

==========
none


none

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

to

==========
none


none

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

[chrishtr, 2016-08-11 23:09:30]

Description was changed from

==========
none


none

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

to

==========
Take CSS Clip and contain: paint into account when computing visual rects.

BUG=611379

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

[chrishtr, 2016-08-11 23:26:26]

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

[chrishtr, 2016-08-11 23:26:32]

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

[commit-bot: I haz the power, 2016-08-11 23:27:58]

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-08-12 00:44:49]

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

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

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[chrishtr, 2016-08-12 15:45:25]

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

[commit-bot: I haz the power, 2016-08-12 15:45:48]

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

[chrishtr, 2016-08-12 16:42:22]

ping

[chrishtr, 2016-08-12 17:04:00]

Description was changed from

==========
Take CSS Clip and contain: paint into account when computing visual rects.

BUG=611379

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

to

==========
Take CSS Clip and contain: paint into account when computing visual rects.

BUG=611379,599939

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

[Xianzhu, 2016-08-12 17:07:57]

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.h (right):

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.h:806: // Returns the
combination of overflow clip, contain: paint clip and CSS clip for this object,
in local space.
What about control clip?

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:320:
addClipRectRelativeToPaintOffset(box.clippingRect());
The original code won't apply clip on descendant absolute-position objects
unless the current object is the containing block ancestor of the
absolute-position object. This is not fully correct for css clip, but we should
be at the safe side as we won't apply wrong clip.

Please add a comment about this.

[commit-bot: I haz the power, 2016-08-12 17:20:00]

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

[commit-bot: I haz the power, 2016-08-12 17:20:01]

Dry run: This issue passed the CQ dry run.

[chrishtr, 2016-08-12 17:56:24]

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.h (right):

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.h:806: // Returns the
combination of overflow clip, contain: paint clip and CSS clip for this object,
in local space.
On 2016/08/12 at 17:07:57, Xianzhu wrote:
> What about control clip?

Control clip on only affects LayoutButton, LayoutMenuList and
LayoutTextControlSingleLine. I tried to modify
PaintPropertyTreeBuilderTest.ControlClip to check visual rects for hidden layout
children of the button, but the LayoutBlockFlow which contains the LayoutText of
the button already has a size that is smaller than the control clip, and the
LayoutText doesn't
have an ObjectPaintProperties object at present (optimized out), which makes it
more boilerplate to test.

I don't think it's worth it to add this. WDYT?

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp:320:
addClipRectRelativeToPaintOffset(box.clippingRect());
On 2016/08/12 at 17:07:57, Xianzhu wrote:
> The original code won't apply clip on descendant absolute-position objects
unless the current object is the containing block ancestor of the
absolute-position object. This is not fully correct for css clip, but we should
be at the safe side as we won't apply wrong clip.
> 
> Please add a comment about this.

Added one here and in LayoutBox.

[Xianzhu, 2016-08-12 18:45:46]

lgtm

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.h (right):

https://codereview.chromium.org/2241663002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.h:806: // Returns the
combination of overflow clip, contain: paint clip and CSS clip for this object,
in local space.
On 2016/08/12 17:56:24, chrishtr wrote:
> On 2016/08/12 at 17:07:57, Xianzhu wrote:
> > What about control clip?
> 
> Control clip on only affects LayoutButton, LayoutMenuList and
> LayoutTextControlSingleLine. I tried to modify
> PaintPropertyTreeBuilderTest.ControlClip to check visual rects for hidden
layout
> children of the button, but the LayoutBlockFlow which contains the LayoutText
of
> the button already has a size that is smaller than the control clip, and the
> LayoutText doesn't
> have an ObjectPaintProperties object at present (optimized out), which makes
it
> more boilerplate to test.
> 
> I don't think it's worth it to add this. WDYT?

SGTM.

[chrishtr, 2016-08-12 18:56:30]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2016-08-12 18:56:51]

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-08-12 18:59:41]

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

[commit-bot: I haz the power, 2016-08-12 18:59:43]

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

[chrishtr, 2016-08-12 20:04:47]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-12 20:04:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2241663002/#ps100001
(title: "none")

[commit-bot: I haz the power, 2016-08-12 20:05:09]

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-08-12 20:35:40]

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

[commit-bot: I haz the power, 2016-08-12 20:35:41]

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

[chrishtr, 2016-08-12 20:47:20]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-08-12 20:47:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2241663002/#ps120001
(title: "none")

[commit-bot: I haz the power, 2016-08-12 20:47:54]

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-08-13 01:18:37]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-08-13 01:20:27]

Description was changed from

==========
Take CSS Clip and contain: paint into account when computing visual rects.

BUG=611379,599939

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

to

==========
Take CSS Clip and contain: paint into account when computing visual rects.

BUG=611379,599939

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

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

[commit-bot: I haz the power, 2016-08-13 01:20:28]

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