Use a composed tree based traversal in computing quads in LinkHighlight::computeQuads().

This is a follow-up patch for https://codereview.chromium.org/448593002 to fix a crash. The previous patch uses renderer.slowFirstChild() to get an child renderer in computeQuads(). However, this is not always correct. For example, suppose that <a><div></div></a> is given, the render tree would be: RenderBlock (anonymous) 0x6110004a6400 RenderInline 0x60c00014e6c0 A 0x60d00006cf10 RenderBlock (anonymous) 0x6110004a6180 RenderBlock 0x6110004a62c0 DIV 0x60b00004d930 CLASS="_muf" In this case, there is no child renderer for RenderInline object(0x60c00014e6c0). Instead, LinkHighlight::computeQuads() should use a composed tree based traversal to compute quads correctly because we've already used a composed tree based traversal to get an parent object in the previous patch. BUG=417191 Review URL: https://codereview.chromium.org/622253004 git-svn-id: svn://svn.chromium.org/blink/trunk@183756 bbb929c8-8fbe-4397-9dbb-9b2b20218538

Use a composed tree based traversal in computing quads in LinkHighlight::computeQuads().
This is a follow-up patch for https://codereview.chromium.org/448593002 to fix a crash. The previous patch uses renderer.slowFirstChild() to get an child renderer in computeQuads(). However, this is not always correct. For example, suppose that <a><div></div></a> is given, the render tree would be: RenderBlock (anonymous) 0x6110004a6400 RenderInline 0x60c00014e6c0 A 0x60d00006cf10 RenderBlock (anonymous) 0x6110004a6180 RenderBlock 0x6110004a62c0 DIV 0x60b00004d930 CLASS="_muf" In this case, there is no child renderer for RenderInline object(0x60c00014e6c0). Instead, LinkHighlight::computeQuads() should use a composed tree based traversal to compute quads correctly because we've already used a composed tree based traversal to get an parent object in the previous patch. BUG=417191 Review URL: https://codereview.chromium.org/622253004 git-svn-id: svn://svn.chromium.org/blink/trunk@183756 bbb929c8-8fbe-4397-9dbb-9b2b20218538
3bdf540a · hayato@chromium.org · 8a5b1ad5 · 3bdf540a · 3bdf540a · 3bdf540a
Commit 3bdf540a authored Oct 15, 2014 by hayato@chromium.org
6 changed files
--- a/third_party/WebKit/LayoutTests/compositing/gestures/gesture-tapHighlight-inline-block-crash-expected.txt
+++ b/third_party/WebKit/LayoutTests/compositing/gestures/gesture-tapHighlight-inline-block-crash-expected.txt
+Shoudn't crash
+
--- a/third_party/WebKit/LayoutTests/compositing/gestures/gesture-tapHighlight-inline-block-crash.html
+++ b/third_party/WebKit/LayoutTests/compositing/gestures/gesture-tapHighlight-inline-block-crash.html
+<!DOCTYPE html>
+<a href="" id="targetLink" width="320" height="240" ><div>Shoudn't crash</div></a>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+var targetLink = document.querySelector('#targetLink');
+var clientRect = targetLink.getBoundingClientRect();
+x = (clientRect.left + clientRect.right) / 2;
+y = (clientRect.top + clientRect.bottom) / 2;
+if (window.eventSender) {
+    eventSender.gestureShowPress(x, y);
+    window.setTimeout(function() { window.testRunner.notifyDone(); }, 0);
+} else {
+    debug("This test requires DumpRenderTree.");
+}
+</script>
--- a/third_party/WebKit/Source/core/dom/NodeRenderingTraversal.cpp
+++ b/third_party/WebKit/Source/core/dom/NodeRenderingTraversal.cpp
@@ -163,7 +163,7 @@ Node* previous(const Node* node, const Node* stayWithin)
    return parent(node);
 }

-static Node* firstChild(const Node* node)
+Node* firstChild(const Node* node)
 {
    ComposedTreeWalker walker(node);
    walker.firstChild();

--- a/third_party/WebKit/Source/core/dom/NodeRenderingTraversal.h
+++ b/third_party/WebKit/Source/core/dom/NodeRenderingTraversal.h
@@ -57,6 +57,7 @@ private:

 ContainerNode* parent(const Node*, ParentDetails* = 0);
 bool contains(const ContainerNode*, const Node*);
+Node* firstChild(const Node*);
 Node* nextSibling(const Node*);
 Node* previousSibling(const Node*);
 Node* previous(const Node*, const Node* stayWithin);

--- a/third_party/WebKit/Source/web/LinkHighlight.cpp
+++ b/third_party/WebKit/Source/web/LinkHighlight.cpp
@@ -29,6 +29,7 @@

 #include "SkMatrix44.h"
 #include "core/dom/Node.h"
+#include "core/dom/NodeRenderingTraversal.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/rendering/RenderLayer.h"
@@ -180,18 +181,23 @@ static void addQuadToPath(const FloatQuad& quad, Path& path)
    path.closeSubpath();
 }

-void LinkHighlight::computeQuads(RenderObject& renderer, Vector<FloatQuad>& outQuads) const
+void LinkHighlight::computeQuads(const Node& node, Vector<FloatQuad>& outQuads) const
 {
+    if (!node.renderer())
+        return;
+
+    RenderObject* renderer = node.renderer();
+
    // For inline elements, absoluteQuads will return a line box based on the line-height
    // and font metrics, which is technically incorrect as replaced elements like images
    // should use their intristic height and expand the linebox  as needed. To get an
    // appropriately sized highlight we descend into the children and have them add their
    // boxes.
-    if (renderer.isRenderInline()) {
-        for (RenderObject* child = renderer.slowFirstChild(); child; child = child->nextSibling())
+    if (renderer->isRenderInline()) {
+        for (Node* child = NodeRenderingTraversal::firstChild(&node); child; child = NodeRenderingTraversal::nextSibling(child))
            computeQuads(*child, outQuads);
    } else {
-        renderer.absoluteQuads(outQuads);
+        renderer->absoluteQuads(outQuads);
    }
 }

@@ -204,7 +210,7 @@ bool LinkHighlight::computeHighlightLayerPathAndPosition(RenderLayer* compositin

    // Get quads for node in absolute coordinates.
    Vector<FloatQuad> quads;
-    computeQuads(*m_node->renderer(), quads);
+    computeQuads(*m_node, quads);
    ASSERT(quads.size());

    // Adjust for offset between target graphics layer and the node's renderer.

--- a/third_party/WebKit/Source/web/LinkHighlight.h
+++ b/third_party/WebKit/Source/web/LinkHighlight.h
@@ -75,7 +75,7 @@ private:
    LinkHighlight(Node*, WebViewImpl*);

    void releaseResources();
-    void computeQuads(RenderObject&, WTF::Vector<FloatQuad>&) const;
+    void computeQuads(const Node&, WTF::Vector<FloatQuad>&) const;

    RenderLayer* computeEnclosingCompositingLayer();
    void clearGraphicsLayerLinkHighlightPointer();