Prefer unprefixed clip-path to (webkit) prefixed

Prefer unprefixed clip-path to (webkit) prefixed

In the world of prefixes, a property without takes precedence over a
property with. That tends to assume that there's not actually two
properties supported though. In the case of 'clip-path' and
'-webkit-clip-path', they both exist and are handled by different code.
The latter also takes precedence over the former which leads to strange
behavior in the cases where both the prefixed and unprefixed property is
specified in a rule.
Change the behavior in this case to be more in line with prefix handling
in general, byt preferring the unprefixed property if it exists.

This requires moving one or the other of the two code-blocks that setup the
clip. In this CL the application of 'clip-path' is moved before the
application of 'mask'. With this move, the rendering-clip-as-mask code-path
can be simplified a bit. Also fix an edge-case where a clip-shape which
referenced a non-existent filter would not be rendered (TC added.)

Also clean-up the rendering-clip-as-mask handling around
opacity/blend-mode/isolation, to avoid creating a (redundant) transparency
layer when a clip-shape has isolation specified.

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

[fs, 2014-10-31 16:23:28]

fs@opera.com changed reviewers:
+ fmalita@chromium.org, pdr@chromium.org

[fs, 2014-10-31 16:23:28]

Requires one updated baseline due to the change of order between mask and clip:

https://storage.googleapis.com/chromium-layout-test-archives/mac_blink_rel/29...

[f(malita), 2014-10-31 17:24:46]

I think this makes sense.

https://codereview.chromium.org/683703005/diff/1/LayoutTests/svg/clip-path/cl...
File LayoutTests/svg/clip-path/clip-path-w-broken-filter-on-shape.html (right):

https://codereview.chromium.org/683703005/diff/1/LayoutTests/svg/clip-path/cl...
LayoutTests/svg/clip-path/clip-path-w-broken-filter-on-shape.html:4:
<text></text>
Just curious: I assume this is to force the mask path in
RenderSVGResourceClipper, but why is that needed?

https://codereview.chromium.org/683703005/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderingContext.cpp (right):

https://codereview.chromium.org/683703005/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderingContext.cpp:101: if (!isRenderingMask &&
(opacity < 1 || hasBlendMode || style->hasIsolation())) {
This shuffling has side effects: hasIsolation() no longer forces a layer when
isRenderingMask == true. Is this intentional?

[fs, 2014-10-31 19:29:22]

https://codereview.chromium.org/683703005/diff/1/LayoutTests/svg/clip-path/cl...
File LayoutTests/svg/clip-path/clip-path-w-broken-filter-on-shape.html (right):

https://codereview.chromium.org/683703005/diff/1/LayoutTests/svg/clip-path/cl...
LayoutTests/svg/clip-path/clip-path-w-broken-filter-on-shape.html:4:
<text></text>
On 2014/10/31 17:24:46, Florin Malita wrote:
> Just curious: I assume this is to force the mask path in
> RenderSVGResourceClipper, but why is that needed?

It's needed because the path/path-ops codepath (rightfully) ignores filters (and
other resources) altogether. It seemed less like that that (<text>) restriction
would be lifted than, say, the clip-on-clip or clipped-clip-shape ones.

https://codereview.chromium.org/683703005/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderingContext.cpp (right):

https://codereview.chromium.org/683703005/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderingContext.cpp:101: if (!isRenderingMask &&
(opacity < 1 || hasBlendMode || style->hasIsolation())) {
On 2014/10/31 17:24:46, Florin Malita wrote:
> This shuffling has side effects: hasIsolation() no longer forces a layer when
> isRenderingMask == true. Is this intentional?

I made (maybe too brief?) note about this in the last paragraph of the
description. Since a clipPath is "geometry only", 'isolation' will (should) have
no effect. (Since it helps define grouping for blending.)

[f(malita), 2014-10-31 19:36:35]

https://codereview.chromium.org/683703005/diff/1/LayoutTests/svg/clip-path/cl...
File LayoutTests/svg/clip-path/clip-path-w-broken-filter-on-shape.html (right):

https://codereview.chromium.org/683703005/diff/1/LayoutTests/svg/clip-path/cl...
LayoutTests/svg/clip-path/clip-path-w-broken-filter-on-shape.html:4:
<text></text>
On 2014/10/31 19:29:22, fs wrote:
> On 2014/10/31 17:24:46, Florin Malita wrote:
> > Just curious: I assume this is to force the mask path in
> > RenderSVGResourceClipper, but why is that needed?
> 
> It's needed because the path/path-ops codepath (rightfully) ignores filters
(and
> other resources) altogether. It seemed less like that that (<text>)
restriction
> would be lifted than, say, the clip-on-clip or clipped-clip-shape ones.

Ack.

https://codereview.chromium.org/683703005/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderingContext.cpp (right):

https://codereview.chromium.org/683703005/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderingContext.cpp:101: if (!isRenderingMask &&
(opacity < 1 || hasBlendMode || style->hasIsolation())) {
On 2014/10/31 19:29:22, fs wrote:
> On 2014/10/31 17:24:46, Florin Malita wrote:
> > This shuffling has side effects: hasIsolation() no longer forces a layer
when
> > isRenderingMask == true. Is this intentional?
> 
> I made (maybe too brief?) note about this in the last paragraph of the
> description. Since a clipPath is "geometry only", 'isolation' will (should)
have
> no effect. (Since it helps define grouping for blending.)

Thanks, I missed that. LGTM.

[fs, 2014-10-31 20:21:58]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-10-31 20:22:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/683703005/1

[commit-bot: I haz the power, 2014-10-31 20:25:20]

Committed patchset #1 (id:1) as 184752