Fix up that no render SVG shape when change clip-path to visible after hidden.

Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client using clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because set wrong mode on |LayoutSVGResourceContainer|.

So request repainting if layoutObjects of |LayoutSVGResourceContainer|
is invalidated about painting.

BUG=590153
Review-Url: https://codereview.chromium.org/2375043002
Cr-Commit-Position: refs/heads/master@{#456612}
Committed: https://chromium.googlesource.com/chromium/src/+/83fe4349b1c39dbfb74619f20852c8dc99b6a7c1

[hyunjunekim2, 2016-09-28 12:23:53]

Description was changed from

==========
WIP 590153

BUG=590153
==========

to

==========
WIP 590153

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

[hyunjunekim2, 2016-10-30 05:41:49]

Description was changed from

==========
WIP 590153

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

to

==========
Fix up that no render SVG shape when change visible after hidden.

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

[hyunjunekim2, 2016-10-30 05:50:30]

Description was changed from

==========
Fix up that no render SVG shape when change visible after hidden.

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

to

==========
Fix up that no render SVG shape when change to visible after hidden.

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

[hyunjunekim2, 2016-10-30 05:58:51]

Description was changed from

==========
Fix up that no render SVG shape when change to visible after hidden.

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

to

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint and shape-element has 'visible' property, painting is invalidate.

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

[hyunjunekim2, 2016-10-30 06:00:52]

Description was changed from

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint and shape-element has 'visible' property, painting is invalidate.

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

to

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidate.

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

[hyunjunekim2, 2016-10-30 06:09:18]

Description was changed from

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidate.

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

to

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because when painting, first check whether invalidate or not.
Therefore clip this element before check invalidation. 

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

[hyunjunekim2, 2016-10-30 06:11:27]

Description was changed from

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because when painting, first check whether invalidate or not.
Therefore clip this element before check invalidation. 

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

to

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because when painting, first check whether invalidate or not.
Therefore first clip this element if is applied clip-path before check
invalidation. 

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

[hyunjunekim2, 2016-10-30 06:11:45]

Description was changed from

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because when painting, first check whether invalidate or not.
Therefore first clip this element if is applied clip-path before check
invalidation. 

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

to

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because when painting, first check whether invalidate or not.
Therefore clip this element if is applied clip-path before check invalidation. 

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

[hyunjunekim2, 2016-10-30 06:54:55]

hyunjune.kim@samsung.com changed reviewers:
+ fs@opera.com, pdr@chromium.org

[hyunjunekim2, 2016-10-30 06:56:30]

@fs, pdr
Long time on see. Return here. And I made a patch. Could you check it?

[fs, 2016-10-31 09:59:22]

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html
(right):

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html:12:
setTimeout(function() {
It looks like it'd be better to use the runAfterLayoutAndPaint helper (from
run-after-layout-and-paint.js) or requestAnimationFrame here for the two
toggles. Why doesn't starting out with class="hidden" on #container and then
only have one toggle() work though?

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html:23:
<svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="400"
height="300">
You don't need to open/declare namespaces or specify version. Please remove
those three attributes.

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html:27:
<rect width="100" height="100" fill="green" visibility="visible"
clip-path="url(#clip)"></rect>
I think it'd make more sense to have this <rect> be larger and have the <rect>
in the clip be smaller so that when the <clipPath> applies the effect on this
<rect> is that it is made smaller (is noticably clipped.)

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/SVGShapePainter.cpp (right):

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/SVGShapePainter.cpp:134: if
(!paintInfo.cullRect().intersectsCullRect(
This does not look like the right fix. Here you're just always painting the
shape (possibly sans the outline.)

[hyunjunekim2, 2016-10-31 13:18:30]

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html
(right):

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html:12:
setTimeout(function() {
On 2016/10/31 09:59:22, fs wrote:
> It looks like it'd be better to use the runAfterLayoutAndPaint helper (from
> run-after-layout-and-paint.js) or requestAnimationFrame here for the two
> toggles. Why doesn't starting out with class="hidden" on #container and then
> only have one toggle() work though?

Done.

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html:23:
<svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="400"
height="300">
On 2016/10/31 09:59:22, fs wrote:
> You don't need to open/declare namespaces or specify version. Please remove
> those three attributes.

Done.

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html:27:
<rect width="100" height="100" fill="green" visibility="visible"
clip-path="url(#clip)"></rect>
On 2016/10/31 09:59:22, fs wrote:
> I think it'd make more sense to have this <rect> be larger and have the <rect>
> in the clip be smaller so that when the <clipPath> applies the effect on this
> <rect> is that it is made smaller (is noticably clipped.)

Done.

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/SVGShapePainter.cpp (right):

https://codereview.chromium.org/2375043002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/SVGShapePainter.cpp:134: if
(!paintInfo.cullRect().intersectsCullRect(
On 2016/10/31 09:59:22, fs wrote:
> This does not look like the right fix. Here you're just always painting the
> shape (possibly sans the outline.)

I will check it for the right fix.

[hyunjunekim2, 2016-11-06 05:40:59]

Description was changed from

==========
Fix up that no render SVG shape when change to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because when painting, first check whether invalidate or not.
Therefore clip this element if is applied clip-path before check invalidation. 

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

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because 

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

[hyunjunekim2, 2016-11-06 05:43:55]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because 

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

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because if the SVG Element has clip-path and 'visible', need whether
painting is valid or not.

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

[hyunjunekim2, 2016-11-06 05:45:40]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because if the SVG Element has clip-path and 'visible', need whether
painting is valid or not.

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

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because if the SVG Element has clip-path and 'visible', need whether
painting is invalidation or not.

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

[hyunjunekim2, 2016-11-06 05:51:44]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because if the SVG Element has clip-path and 'visible', need whether
painting is invalidation or not.

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

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because if the SVG Element has clip-path and 'visible', request paint for SVG
element.

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

[hyunjunekim2, 2016-11-06 05:52:04]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because if the SVG Element has clip-path and 'visible', request paint for SVG
element.

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

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because if the SVG Element has clip-path and 'visible', request painting
for SVG element.

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

[hyunjunekim2, 2016-11-07 01:23:58]

@fs,
Could you tell me advice about latest patch?
I changed a patch to run recalcStyle for SVG Element that has 'clip-path'.

[fs, 2016-11-07 12:03:30]

On 2016/11/07 at 01:23:58, hyunjune.kim wrote:
> @fs,
> Could you tell me advice about latest patch?
> I changed a patch to run recalcStyle for SVG Element that has 'clip-path'.

It would be easier to give (good) advice if it was more clear where things go
wrong? When 'visibility' toggles back to 'visible', is the shape repainted, but
using an old cached clip-path that is empty? If the shape isn't repainted, then
why?

https://codereview.chromium.org/2375043002/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html
(right):

https://codereview.chromium.org/2375043002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/svg/clip-path/clip-path-visible-element-as-visible-shape-element.html:9:
container.style.visibility = "hidden";
Does the problem still reproduce with style="visibility: hidden" on #container
at the start, and then just a toggle of that to 'visible'? (I.e just the inner
runAfterLayoutAndPaint.)

https://codereview.chromium.org/2375043002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2375043002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Element.cpp:1958: (layoutObject->isSVG() &&
layoutObject->hasClipPath())) {
This does not look right.

https://codereview.chromium.org/2375043002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2375043002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:1544: if
(!diff.needsPaintInvalidation() && isSVG() && hasClipPath() &&
Neither does this (very/overly case-specific.)

[hyunjunekim2, 2016-11-13 13:12:18]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

When repaint shape element that has 'visible' property, painting is invalidated.
So can't apply clip-path that has 'visible' too.
Because if the SVG Element has clip-path and 'visible', request painting
for SVG element.

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

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

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

[hyunjunekim2, 2017-01-05 16:25:51]

fs, Could you check PS 11? Thank you.

[fs, 2017-01-05 17:01:47]

https://codereview.chromium.org/2375043002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2375043002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:93: if
(style()->visibility() == EVisibility::Visible &&
I think this may be starting to approach the right track, but not quite there
yet. The 'visibility' on the LayoutSVGResourceContainer itself (<clipPath>,
<mask> and <pattern> I think are the relevant ones here) does not actually have
any influence on the actual "content". So, I'd expect the invalidation to take
place through the children of the LayoutSVGResourceContainer (probably via
SVGResourceCache::clientStyleChanged?)

https://codereview.chromium.org/2375043002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:100: //
Remove the old resource.
This looks too heavy-handed, if you wanted to invalidate clients,
removeAllClientsFromCache would probably be better. (If you want to be
selective, removeClientFromCache.) You should also not be
unregistering/re-registering the resource.

[hyunjunekim2, 2017-01-15 14:40:48]

fs, Could you check PS 12? Thank you.

[fs, 2017-01-16 10:03:10]

https://codereview.chromium.org/2375043002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp (right):

https://codereview.chromium.org/2375043002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp:132:
LayoutSVGResourceContainer::markForLayoutAndParentResourceInvalidation(client,
true);
Why isn't this handled by the call to markForLayoutAndParentResourceInvalidation
at the end of the function? (Which should already walk the ancestor chain and
"discover" a potential LayoutSVGResourceContainer ancestor if there is one.)

[hyunjunekim2, 2017-01-19 12:49:05]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

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

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client that use clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because the client have used the old cache about resources. So if resources
is toggled to visible and the client is no-change, request layouting about
'SvgResourceInvalidated'.

BUG=590153
==========

[hyunjunekim2, 2017-01-19 12:59:20]

fs, Changed that it can be handled on
|markForLayoutAndParentResourceInvalidation|.
    Could you check it?

[fs, 2017-01-19 13:24:41]

https://codereview.chromium.org/2375043002/diff/320001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2375043002/diff/320001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:313:
LayoutSVGResourceContainer* container =
The line just above here will call markAllClientsForInvalidation, which will
iterate all clients and essentially propagate the "needs layout" signal. So, I
suspect the InvalidationMode passed there is not "strong" enough, and should
have been "upgraded" on the way. So maybe all that's needed is to pass 'true' to
the removeAllClientsFromCache call above to make this work? (Preferably that
should be conditionalized on something too.)

[hyunjunekim2, 2017-01-19 14:31:43]

On 2017/01/19 13:24:41, fs wrote:
>
https://codereview.chromium.org/2375043002/diff/320001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
> (right):
> 
>
https://codereview.chromium.org/2375043002/diff/320001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:313:
> LayoutSVGResourceContainer* container =
> The line just above here will call markAllClientsForInvalidation, which will
> iterate all clients and essentially propagate the "needs layout" signal. So, I
> suspect the InvalidationMode passed there is not "strong" enough, and should
> have been "upgraded" on the way. So maybe all that's needed is to pass 'true'
to
> the removeAllClientsFromCache call above to make this work? (Preferably that
> should be conditionalized on something too.)

fs, yes, i got it about m_invalidationMask. that is clear when the client is
changed
style. but in this case, the client is not changed style. so we can't do
layouting about
invalidated client.

and on PS 18, if client is invalidated, after clear mask, call
removeAllClientsFromCache.

[fs, 2017-01-19 15:03:48]

On 2017/01/19 at 14:31:43, hyunjune.kim wrote:
> On 2017/01/19 13:24:41, fs wrote:
> >
https://codereview.chromium.org/2375043002/diff/320001/third_party/WebKit/Sou...
> > File
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
> > (right):
> > 
> >
https://codereview.chromium.org/2375043002/diff/320001/third_party/WebKit/Sou...
> >
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:313:
> > LayoutSVGResourceContainer* container =
> > The line just above here will call markAllClientsForInvalidation, which will
> > iterate all clients and essentially propagate the "needs layout" signal. So,
I
> > suspect the InvalidationMode passed there is not "strong" enough, and should
> > have been "upgraded" on the way. So maybe all that's needed is to pass
'true' to
> > the removeAllClientsFromCache call above to make this work? (Preferably that
> > should be conditionalized on something too.)
> 
> fs, yes, i got it about m_invalidationMask. that is clear when the client is
changed
> style. but in this case, the client is not changed style. so we can't do
layouting about
> invalidated client.

The client shouldn't be required to change style for a change to the resource to
trigger a layout/paint of it. (Doing that could be considered an optimization
though.) I don't see a need to touch the invalidation mask - if the correct bit
is set in the mask, then invalidation of that kind should be pending already.

> and on PS 18, if client is invalidated, after clear mask, call
removeAllClientsFromCache.

What I suspect is happening here is that clientStyleChanged for the <rect>
within the <clipPath> ends up issuing a ParentOnlyInvalidation, while it really
should be issuing a LayoutAndBoundariesInvalidation since the geometry of the
<clipPath> changes, which in turn means the visual rect of the client will
change. This could probably be verified by passing 'true' rather than 'false' in
SVGResourcesCache::clientStyleChanged. If that's the case, then we probably need
to pass along some additional data from clientStyleChanged to allow us to handle
this (issue the "right" InvalidationMode.) Maybe just passing the
StyleDifference would be enough (and then check for layout/paint when calling
removeAllClientsFromCache and change the argument accordingly.)

[hyunjunekim2, 2017-01-23 12:31:42]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client that use clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because the client have used the old cache about resources. So if resources
is toggled to visible and the client is no-change, request layouting about
'SvgResourceInvalidated'.

BUG=590153
==========

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client that use clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because the client have used the old cache about resources.

BUG=590153
==========

[hyunjunekim2, 2017-01-23 12:33:18]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client that use clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because the client have used the old cache about resources.

BUG=590153
==========

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client that use clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because set wrong mode on |LayoutSVGResourceContainer|.

BUG=590153
==========

[hyunjunekim2, 2017-01-23 12:40:00]

On PS 19, If container needs |PaintInvalidation|, set 'true' in
SVGResourcesCache::clientStyleChanged.
Could you check it? Thank you.

[fs, 2017-01-23 12:44:24]

https://codereview.chromium.org/2375043002/diff/360001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp (right):

https://codereview.chromium.org/2375043002/diff/360001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp:133: if
(layoutObject->isSVGResourceContainer())
What if we only toggle 'visibility' on the <rect>?

[hyunjunekim2, 2017-01-23 12:55:57]

On 2017/01/23 12:44:24, fs wrote:
>
https://codereview.chromium.org/2375043002/diff/360001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp (right):
> 
>
https://codereview.chromium.org/2375043002/diff/360001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp:133: if
> (layoutObject->isSVGResourceContainer())
> What if we only toggle 'visibility' on the <rect>?

Yes. i checked it. Does it add on layout-test?

[fs, 2017-01-23 13:03:40]

On 2017/01/23 at 12:55:57, hyunjune.kim wrote:
> On 2017/01/23 12:44:24, fs wrote:
> >
https://codereview.chromium.org/2375043002/diff/360001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp
(right):
> > 
> >
https://codereview.chromium.org/2375043002/diff/360001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp:133: if
> > (layoutObject->isSVGResourceContainer())
> > What if we only toggle 'visibility' on the <rect>?
> 
> Yes. i checked it. Does it add on layout-test?

It probably wouldn't hurt to test, no.

https://codereview.chromium.org/2375043002/diff/380001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp (right):

https://codereview.chromium.org/2375043002/diff/380001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp:132: needLayout
|= diff.needsPaintInvalidation();
Now we'll trigger a lot more layout than we need to. We don't want to do that.
This is why I suggested (I think) that we pass the StyleDifference to
markForLayoutAndParentResourceInvalidation, and then use that in the relevant
call when we know we're a descendant of a LayoutSVGResourceContainer.

[hyunjunekim2, 2017-01-30 11:50:26]

fs, Could separate |markForLayoutAndParentResourceInvalidation| function such as
PS21?
Because other codes use it that no need the StyleDifference.
If you give me advice, i'm going to clean up PS21.
Thank you.

[fs, 2017-02-06 10:17:35]

Sorry for the delay, I was travelling.

https://codereview.chromium.org/2375043002/diff/400001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
(right):

https://codereview.chromium.org/2375043002/diff/400001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp:319:
void LayoutSVGResourceContainer::markForLayoutAndParentResourceInvalidation(
The way this is written it doesn't need to duplicate the other
markForLayoutAndParentResourceInvalidation, but can be written as a separate
method. I.e something like:

bool needsLayout = diff.needsPaintInvalidation() &&
isChildOfResourceContainer(object);

which then feeds into the other ("old") function.

[hyunjunekim2, 2017-02-14 12:35:59]

Ok, i'm going to work it. thank you for your advice.

[hyunjunekim2, 2017-02-19 07:54:07]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client that use clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because set wrong mode on |LayoutSVGResourceContainer|.

BUG=590153
==========

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client that use clip-path is no-change and resources is toggled
to visible from hidden, can't render client.

BUG=590153
==========

[hyunjunekim2, 2017-02-19 07:55:23]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client that use clip-path is no-change and resources is toggled
to visible from hidden, can't render client.

BUG=590153
==========

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client using clip-path is no-change and resources is toggled
to visible from hidden, can't render client.

BUG=590153
==========

[hyunjunekim2, 2017-02-19 08:14:22]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client using clip-path is no-change and resources is toggled
to visible from hidden, can't render client.

BUG=590153
==========

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client using clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because set wrong mode on |LayoutSVGResourceContainer|.

So request repainting if layoutObjects of |LayoutSVGResourceContainer|
is invalidated about painting.

BUG=590153
==========

[hyunjunekim2, 2017-02-19 09:34:12]

fs,
i updated PS22. Could you review it? Thank you.

[fs, 2017-02-20 09:07:44]

lgtm

https://codereview.chromium.org/2375043002/diff/420001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp (right):

https://codereview.chromium.org/2375043002/diff/420001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp:142: bool
needsLayout = diff.needsPaintInvalidation() &&
It would be good to add a comment here to describe what we're trying to catch
here by doing this.

[hyunjunekim2, 2017-03-13 10:40:12]

Could you check this PS? Thank you.

https://codereview.chromium.org/2375043002/diff/420001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp (right):

https://codereview.chromium.org/2375043002/diff/420001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp:142: bool
needsLayout = diff.needsPaintInvalidation() &&
On 2017/02/20 09:07:43, fs wrote:
> It would be good to add a comment here to describe what we're trying to catch
> here by doing this.

Done. Added a comment. Could you check it? Thank you.

[fs, 2017-03-13 10:49:50]

On 2017/03/13 at 10:40:12, hyunjune.kim wrote:
> Could you check this PS? Thank you.
> 
>
https://codereview.chromium.org/2375043002/diff/420001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp (right):
> 
>
https://codereview.chromium.org/2375043002/diff/420001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp:142: bool
needsLayout = diff.needsPaintInvalidation() &&
> On 2017/02/20 09:07:43, fs wrote:
> > It would be good to add a comment here to describe what we're trying to
catch
> > here by doing this.
> 
> Done. Added a comment. Could you check it? Thank you.

Still LGTM.

[hyunjunekim2, 2017-03-14 01:28:41]

The CQ bit was checked by hyunjune.kim@samsung.com

[commit-bot: I haz the power, 2017-03-14 01:29:28]

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, 2017-03-14 03:22:36]

CQ is committing da patch.

Bot data: {"patchset_id": 440001, "attempt_start_ts": 1489454921565870,
"parent_rev": "59fd92a3139ea60c5fb024342174c17cbd0b7796", "commit_rev":
"83fe4349b1c39dbfb74619f20852c8dc99b6a7c1"}

[commit-bot: I haz the power, 2017-03-14 03:23:43]

Description was changed from

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client using clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because set wrong mode on |LayoutSVGResourceContainer|.

So request repainting if layoutObjects of |LayoutSVGResourceContainer|
is invalidated about painting.

BUG=590153
==========

to

==========
Fix up that no render SVG shape when change clip-path to visible after hidden.

If the client using clip-path is no-change and resources is toggled
to visible from hidden, can't render client.
Because set wrong mode on |LayoutSVGResourceContainer|.

So request repainting if layoutObjects of |LayoutSVGResourceContainer|
is invalidated about painting.

BUG=590153

Review-Url: https://codereview.chromium.org/2375043002
Cr-Commit-Position: refs/heads/master@{#456612}
Committed:
https://chromium.googlesource.com/chromium/src/+/83fe4349b1c39dbfb74619f20852...
==========

[commit-bot: I haz the power, 2017-03-14 03:23:44]

Committed patchset #23 (id:440001) as
https://chromium.googlesource.com/chromium/src/+/83fe4349b1c39dbfb74619f20852...