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 a1ad16265753f22fc8eeedc0e241021f4cdd1259..3e1a1b8559f603104ee73feb64d17fa5c0e73581 100644
--- a/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
+++ b/third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp
@@ -44,6 +44,17 @@
 
 namespace blink {
 
+struct SearchCandidate {
+    SearchCandidate() {}

[fs, 2016/02/12 17:05:42]

Maybe this should set the layoutObject memeer to nullptr and the distance to
max() as you do below?

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    SearchCandidate(LayoutObject* layoutObject, float dist)

[fs, 2016/02/12 17:05:43]

dist -> distance

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+        : candidateLayoutObject(layoutObject)
+        , distance(dist)
+    {
+    }
+    LayoutObject* candidateLayoutObject;

[fs, 2016/02/12 17:05:42]

The 'candidate'-prefix seems redundant.

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    float distance;
+};
+
 static inline LayoutRect adjustedEnclosingIntRect(const FloatRect& rect,
     const AffineTransform& rootTransform, float strokeWidthForHairlinePadding)
 {
@@ -487,4 +498,79 @@ float SVGLayoutSupport::calculateScreenFontSizeScalingFactor(const LayoutObject*
     return narrowPrecisionToFloat(sqrt((pow(ctm.xScale(), 2) + pow(ctm.yScale(), 2)) / 2));
 }
 
+static inline bool compareCandidateDistance(const SearchCandidate& r1, const SearchCandidate& r2)
+{
+    return r1.distance < r2.distance;
+}
+
+static inline float distanceToChildLayoutObject(LayoutObject* child, const FloatPoint& point)
+{
+    const AffineTransform& localToParentTransform = child->localToParentTransform();
+    if (!localToParentTransform.isInvertible())
+        return std::numeric_limits<float>::max();
+    FloatPoint childLocalPoint = localToParentTransform.inverse().mapPoint(point);
+    return child->objectBoundingBox().squaredDistanceTo(childLocalPoint);
+}
+
+static SearchCandidate searchTreeForFindClosestLayoutSVGText(LayoutObject* layoutObject, const FloatPoint& point)
+{
+    // Try to find the closest LayoutSVGText.
+    LayoutObject* closestLayoutObject = nullptr;
+    float closestDistance = std::numeric_limits<float>::max();

[fs, 2016/02/12 17:05:42]

Maybe put these two in a SearchCandidate too. Maybe you could even move
closestSearchCandidateText to  here instead.

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    Vector<SearchCandidate> candidates;
+
+    // First find LayoutSVGText on the this tree level and also find candidates.

[fs, 2016/02/12 17:05:43]

"Find the closest LayoutSVGText on this tree level, and also collect any
containers that could contain LayoutSVGTexts that are closer."

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    for (LayoutObject* child = layoutObject->slowLastChild(); child; child = child->previousSibling()) {
+        if (child->isSVGText()) {
+            float distance = distanceToChildLayoutObject(child, point);
+            if (distance >= closestDistance)
+                continue;
+            candidates.clear();
+            closestLayoutObject = child;
+            closestDistance = distance;
+            continue;
+        }
+
+        if (child->isSVGContainer() && !layoutObject->isSVGHiddenContainer()) {
+            float distance = distanceToChildLayoutObject(child, point);
+            if (distance > closestDistance)
+                continue;
+            candidates.append(SearchCandidate(child, distance));
+        }
+    }
+
+    // If find LayoutSVGText and don't have candidates, just return |closestLayoutObject|.

[fs, 2016/02/12 17:05:42]

"If a LayoutSVGText was found and there are no potentially closer sub-trees,
just return ..."

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    if (closestLayoutObject && candidates.isEmpty())
+        return SearchCandidate(closestLayoutObject, closestDistance);
+
+    std::stable_sort(candidates.begin(), candidates.end(), compareCandidateDistance);
+
+    SearchCandidate closestSearchCandidateText = SearchCandidate(nullptr, std::numeric_limits<float>::max());

[fs, 2016/02/12 17:05:42]

SearchCandidate closestSearchCandidateText(nullptr, ...)

but also see suggestion above. You could probably also shorten the name a bit
without losing readability - maybe just 'closestText'?

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    // Find the closest LayoutSVGText on |condidates| that is sub-trees.

[fs, 2016/02/12 17:05:42]

"on |condidates| that is sub-trees." -> "in the sub-trees in |candidates|." (or
similar)

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    // If find layoutSVGText on |searchCandidate| and it is closer then other |searchCandidate|, stop iteration statement.

[fs, 2016/02/12 17:05:42]

"If a LayoutSVGText is found that is (strictly) closer than any previous
candidate, then end the search."

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    for (SearchCandidate& searchCandidate : candidates) {

[fs, 2016/02/12 17:05:42]

const

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+        if (closestSearchCandidateText.distance < searchCandidate.distance)
+            break;
+        LayoutObject* candidateLayoutObject = searchCandidate.candidateLayoutObject;
+        FloatPoint candidateLocalPoint = candidateLayoutObject->localToParentTransform().inverse().mapPoint(point);
+
+        SearchCandidate candidateText = searchTreeForFindClosestLayoutSVGText(candidateLayoutObject, candidateLocalPoint);
+
+        if (candidateText.distance < closestSearchCandidateText.distance)
+            closestSearchCandidateText = candidateText;
+    }
+
+    // Compare distance between |closestLayoutObject| and |closestSearchCandidateText|.

[fs, 2016/02/12 17:05:42]

"Compare" is pretty obvious, maybe "Return the closest of ..."

[hyunjunekim2, 2016/02/15 13:34:21]

Done. Remove if statement.

+    if (closestDistance <= closestSearchCandidateText.distance)
+        return SearchCandidate(closestLayoutObject, closestDistance);
+
+    return closestSearchCandidateText;
+}
+
+LayoutObject* SVGLayoutSupport::findClosestLayoutSVGText(LayoutObject* layoutObject, const FloatPoint& point)
+{
+    SearchCandidate result = searchTreeForFindClosestLayoutSVGText(layoutObject, point);

[fs, 2016/02/12 17:05:42]

Could just do:

return searchTree...(...).candidateLayoutObject;

[hyunjunekim2, 2016/02/15 13:34:21]

Done.

+    return result.candidateLayoutObject;
+}
+
 } // namespace blink