If the <use> is hidden and the child of it is visible, should clip the non-resource

third_party/WebKit/Source/core/svg/SVGUseElement.cpp

Index: third_party/WebKit/Source/core/svg/SVGUseElement.cpp
diff --git a/third_party/WebKit/Source/core/svg/SVGUseElement.cpp b/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
index 195d5f7d130ff011e876479e8236841c07a110b3..d5146e11e4d921aa0015af1fdc4bb6e40051ac04 100644
--- a/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
+++ b/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
@@ -478,6 +478,21 @@ static bool isDirectReference(const SVGElement& element)
         || isSVGTextElement(element);
 }
 
+bool SVGUseElement::isVisibleGraphicsElementForClipping()
+{
+    Node* n = userAgentShadowRoot()->firstChild();

[fs, 2016/04/19 10:17:27]

This should probably just be using targetGraphicsElementForClipping, to filter
out any non-graphics element descendants. It also looks like we could merge this
with targetGraphicsElementForClipping and get something like:

const SVGGraphicsElement* visibleTargetGraphicsElementForClipping() const;

which checks for a LayoutObject etc. too before returning the element, so we
don't need to duplicate this all over the place.

[hyunjunekim2, 2016/04/27 12:18:40]

Merged this function. But occurred to console error on a test called
'clip-path-referencing-use2-expected.txt'. 
Because There is a code to print document error on
targetGraphicsElementForClipping().
Such as,
    if (!isDirectReference(element)) {
        // Spec: Indirect references are an error (14.3.5)
        document().accessSVGExtensions().reportError("Not allowed to use
indirect reference in <clip-path>");
        return nullptr;
    }

[fs, 2016/04/27 14:14:39]

Personally I'd be fine with just removing this reportError() call - it seems
more likely to result in log spam than in any help to the author. We definitely
don't want to emit this error several times per resource-layout... If removing
this reportError() result in lots of baselines to update, then we should do it
as a separate CL I think.

[hyunjunekim2, 2016/04/29 12:59:08]

Done. Remove it.

+    if (!n || !n->isSVGElement())
+        return false;
+
+    SVGElement& element = toSVGElement(*n);
+
+    const ComputedStyle* style = element.layoutObject() ? element.layoutObject()->style() : nullptr;
+    if (style && style->visibility() != VISIBLE)
+        return false;
+
+    return true;
+}
+
 void SVGUseElement::toClipPath(Path& path) const
 {
     ASSERT(path.isEmpty());
@@ -487,6 +502,10 @@ void SVGUseElement::toClipPath(Path& path) const
     if (!element || !element->layoutObject())
         return;
 
+    const ComputedStyle* style = element->layoutObject() ? element->layoutObject()->style() : nullptr;

[fs, 2016/04/19 10:17:27]

element->layoutObject() has already been null-checked here (just above.)

[hyunjunekim2, 2016/04/27 12:18:40]

Done.

+    if (style && style->visibility() != VISIBLE)

[fs, 2016/04/19 10:17:27]

This should be: !style || style->visibility() != VISIBLE

But I don't don't see when we'd call this and have a LayoutObject that haven't
had it's ComputedStyle attached yet, so using styleRef() here and removing the
null-check should be ok.

[hyunjunekim2, 2016/04/27 12:18:40]

Done.

+        return;
+
     if (element->isSVGGeometryElement()) {
         toSVGGeometryElement(*element).toClipPath(path);
         // FIXME: Avoid manual resolution of x/y here. Its potentially harmful.