Validate objects are a box when clipping layers for clip-path.

Validate objects are a box when clipping layers for clip-path.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

Committed: https://crrev.com/5a83553e83c5f8cdc3d7cb54d00210238e4dd83e
Cr-Commit-Position: refs/heads/master@{#440825}

[wkorman, 2016-12-27 23:05:26]

Description was changed from

==========
Validate objects are a box when clipping containing layer for css-clip.

BUG=675081
==========

to

==========
Validate objects are a box when clipping containing layer for css-clip.

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

[wkorman, 2016-12-27 23:08:15]

Description was changed from

==========
Validate objects are a box when clipping containing layer for css-clip.

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

to

==========
Validate objects are a box when clipping containing layer for css-clip.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

[wkorman, 2016-12-27 23:08:32]

Description was changed from

==========
Validate objects are a box when clipping containing layer for css-clip.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

to

==========
Validate objects are a box when clipping containing layer for clip-path.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

[wkorman, 2016-12-27 23:13:22]

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

[wkorman, 2016-12-27 23:13:23]

For discussion -- this fixes the test case, but I wonder whether we should
instead be returning false if not a box and not even getting to this point.

See other calls to hasClipRelatedProperty() in CompositingInputsUpdater,
CompositingRequirementsUpdater, and
PaintLayerCompositor::clipsCompositingDescendants():

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...

I am not yet an expert (or even pseudo-adept) in the ways of clip-path.

[wkorman, 2016-12-27 23:18:45]

On 2016/12/27 at 23:13:23, wkorman wrote:
> For discussion -- this fixes the test case, but I wonder whether we should
instead be returning false if not a box and not even getting to this point.
> 
> See other calls to hasClipRelatedProperty() in CompositingInputsUpdater,
CompositingRequirementsUpdater, and
PaintLayerCompositor::clipsCompositingDescendants():
> 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...
> 
> I am not yet an expert (or even pseudo-adept) in the ways of clip-path.

Or, are we going to miss clipping in some cases where we should by just skipping
if not-isBox?

https://www.w3.org/TR/css-masking-1/#propdef-clip-path

states clip-path applies to all elements. Perhaps we should do something to
compute the box of the inline and clip by that rather than just skipping
clipping.

But if that's the case, we are already broken, so this change would not make
things worse.

[wkorman, 2016-12-27 23:19:23]

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

[commit-bot: I haz the power, 2016-12-27 23:19:42]

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

[wkorman, 2016-12-27 23:20:24]

Description was changed from

==========
Validate objects are a box when clipping containing layer for clip-path.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

to

==========
Validate objects are a box when clipping layers for clip-path.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

[trchen, 2016-12-28 00:37:16]

On 2016/12/27 23:18:45, wkorman wrote:
> On 2016/12/27 at 23:13:23, wkorman wrote:
> > For discussion -- this fixes the test case, but I wonder whether we should
> instead be returning false if not a box and not even getting to this point.
> > 
> > See other calls to hasClipRelatedProperty() in CompositingInputsUpdater,
> CompositingRequirementsUpdater, and
> PaintLayerCompositor::clipsCompositingDescendants():
> > 
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...
> > 
> > I am not yet an expert (or even pseudo-adept) in the ways of clip-path.
> 
> Or, are we going to miss clipping in some cases where we should by just
skipping
> if not-isBox?
> 
> https://www.w3.org/TR/css-masking-1/#propdef-clip-path
> 
> states clip-path applies to all elements. Perhaps we should do something to
> compute the box of the inline and clip by that rather than just skipping
> clipping.
> 
> But if that's the case, we are already broken, so this change would not make
> things worse.

Wow this is crazy. I didn't know clip-path applies to inline elements as well. I
just did a few experiments on jsbin. I think on inline elements the enclosing
rect of layout overflow is being used as the "object bounding box" for clip-path
layout reference. Anyway lgtm because the CL only converts undefined behavior
into silent failure, which IMO is strictly better.

[commit-bot: I haz the power, 2016-12-28 01:04:51]

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

[commit-bot: I haz the power, 2016-12-28 01:04:52]

Dry run: This issue passed the CQ dry run.

[wkorman, 2016-12-28 01:37:32]

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

[wkorman, 2016-12-28 01:37:32]

+xianzhu for OWNERS.

[Xianzhu, 2016-12-28 02:40:22]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-12-28 02:40:23]

lgtm

[commit-bot: I haz the power, 2016-12-28 02:40:37]

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:44:19]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1482892822768240, "parent_rev":
"b81519c620b0310df83c980b898c4c6b078394c7", "commit_rev":
"1c2eab21a9b4765862edbe45a65803d50563d051"}

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

Description was changed from

==========
Validate objects are a box when clipping layers for clip-path.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

to

==========
Validate objects are a box when clipping layers for clip-path.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

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

[commit-bot: I haz the power, 2016-12-28 02:44:48]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2017-01-02 15:48:23]

Description was changed from

==========
Validate objects are a box when clipping layers for clip-path.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

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

to

==========
Validate objects are a box when clipping layers for clip-path.

http://crrev.com/2558633005 modified
LayoutObject::hasClipRelatedProperty() to consider clip path,
unintentionally introducing this issue.

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

Committed: https://crrev.com/5a83553e83c5f8cdc3d7cb54d00210238e4dd83e
Cr-Commit-Position: refs/heads/master@{#440825}
==========

[commit-bot: I haz the power, 2017-01-02 15:48:23]

Patchset 1 (id:??) landed as
https://crrev.com/5a83553e83c5f8cdc3d7cb54d00210238e4dd83e
Cr-Commit-Position: refs/heads/master@{#440825}