Fix invalid selection produced when dragging mouse outside the SVG text element

third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp

Index: third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
diff --git a/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp b/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
index 32073042bd914cd8999f8fd2c593feade0f70a23..2a9786c55d498747fdc3c80db32de1a7d788475f 100644
--- a/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
+++ b/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
@@ -476,4 +476,61 @@ float SVGLayoutSupport::calculateScreenFontSizeScalingFactor(const LayoutObject*
     return narrowPrecisionToFloat(sqrt((pow(ctm.xScale(), 2) + pow(ctm.yScale(), 2)) / 2));
 }
 
+LayoutObject* SVGLayoutSupport::findSelectedLayoutSVGText(LayoutObject* layoutObject, AffineTransform& localToParentTransform, const FloatPoint point)
+{
+    LayoutObject* closestLayoutObject = nullptr;
+    float closestDistance = std::numeric_limits<float>::max();
+    Vector<LayoutObject*> overlappedObjects;
+    for (LayoutObject* child = layoutObject->slowLastChild(); child; child = child->previousSibling()) {
+        if (child->style()->visibility() != VISIBLE)

[fs, 2016/01/15 10:34:16]

This is not correct at this level - 'visibility' can still be 'visible' for any
descendant.

+            continue;
+
+        if (!child->isSVGContainer() && !child->isSVGText())

[fs, 2016/01/15 10:34:16]

"Hidden containers" still return true for isSVGContainer(), so this needs to be
"(isSVGContainer() && !isSVGHiddenContainer())".

+            continue;
+
+        FloatRect bound = child->objectBoundingBox();
+        float distance = bound.squaredDistanceTo(child->localToParentTransform().inverse().mapPoint(point));

[fs, 2016/01/15 10:34:16]

Since you're using the inverse transform you should first check that it is
invertable (if it isn't then the subtree can be skipped.)

+
+        if (distance < closestDistance) {
+            AffineTransform transform;
+
+            if (closestLayoutObject) {
+                transform = localToParentTransform * closestLayoutObject->localToParentTransform();

[fs, 2016/01/15 10:34:16]

No need to transform by |localToParentTransform| since |closestLayoutObject| and
|child| can be brought to the same coordinate space just by applying their
respective localToParentTransform()s (and thus so can their oBBs.)

+                bound = (localToParentTransform * child->localToParentTransform()).mapRect(bound);
+            }
+
+            if (closestLayoutObject && bound.intersects(transform.mapRect(closestLayoutObject->objectBoundingBox()))) {

[fs, 2016/01/15 10:34:16]

Not quite sure why you'd need to check overlap between the two bboxes? If this
new LayoutObject is closer, it seems we'd want to search it first? If both
contain the point, I guess we'd want to search them both (in the order in which
they were initially encountered)? This needs documentation/comments.

+                overlappedObjects.append(child);
+                continue;
+            }
+            overlappedObjects.clear();
+
+            closestLayoutObject = child;
+            closestDistance = distance;
+        }
+    }
+
+    if (closestLayoutObject && !closestLayoutObject->isSVGText()) {

[fs, 2016/01/15 10:34:16]

Use "early-out style" when possible - i.e here you'd do:

if (!closestLayoutObject || closestLayoutObject->isSVGText())
    return closestLayoutObject;
...

+        FloatPoint absolutePoint = closestLayoutObject->localToParentTransform().inverse().mapPoint(point);
+        localToParentTransform = localToParentTransform * closestLayoutObject->localToParentTransform();
+        LayoutObject* result  = findSelectedLayoutSVGText(closestLayoutObject, localToParentTransform, absolutePoint);
+
+        if (!result) {
+            localToParentTransform = localToParentTransform * closestLayoutObject->localToParentTransform().inverse();

[fs, 2016/01/15 10:34:16]

Maintaining the transform like this looks really error-prone... It'd probably be
better to just leave that out entirely - just inverse-transform the point and
work completely in the local space, and then compute the CTM for the object
found in LayoutSVGRoot.

+            for (LayoutObject* overlappedObject : overlappedObjects) {
+                absolutePoint = overlappedObject->localToParentTransform().inverse().mapPoint(point);
+                localToParentTransform = localToParentTransform * overlappedObject->localToParentTransform();
+                result = findSelectedLayoutSVGText(overlappedObject, localToParentTransform, absolutePoint);
+                if (result)
+                    break;
+                localToParentTransform = localToParentTransform * overlappedObject->localToParentTransform().inverse();
+            }
+        }
+
+        closestLayoutObject = result;
+    }
+
+    return closestLayoutObject;
+}
+
 }