Title: [147538] trunk
Revision
147538
Author
[email protected]
Date
2013-04-03 04:06:49 -0700 (Wed, 03 Apr 2013)

Log Message

Web Inspector: crash in WebCore::InspectorLayerTreeAgent::buildObjectForLayer if a layer is created for an anonymous RenderObject (:first-letter)
https://bugs.webkit.org/show_bug.cgi?id=113768

Source/WebCore:

The InspectorLayerTreeAgent now knows how to deal with anonymous RenderObjects
for the :first-letter and :first-line pseudo-elements.

Reviewed by Timothy Hatcher.

Test: inspector-protocol/layers/layers-anonymous.html

* inspector/Inspector.json:
Change the Layer type in the LayerTree domain to have more accurate terminology
(pseudo-element vs. pseudo-class) and a new optional isAnonymous flag for layers
associated to anonymous RenderObjects.

* inspector/InspectorLayerTreeAgent.cpp:
(WebCore::InspectorLayerTreeAgent::buildObjectForLayer):
Check for anonymous RenderObjects and set the parent renderer's node as the node
for this layer since anonymous renderers have the Document as their node and this
would not be satisfactory to show in a front-end. We also check for anonymous
RenderObjects for the :first-letter and :first-line pseudo-elements and set them
in the pseudoElement property of the Layer object created such that a front-end
could correctly identify what type of pseudo-element for the associated node
yielded this layer.

(WebCore::InspectorLayerTreeAgent::idForNode):
Here we fix the actual crash by first checking if the node provided is null and
returning 0 if there's no such node. This ensures that any scenario where there
is no node associated with the RenderObject simply informs of the front-end that
no such node exists and we do not crash.

LayoutTests:

Reviewed by Timothy Hatcher.

* inspector-protocol/layers/layers-anonymous-expected.txt: Added.
* inspector-protocol/layers/layers-anonymous.html: Added.
New test for layers created for CSS anonymous boxes or blocks.

* inspector-protocol/layers/layers-generated-content.html:
Update test to use the "pseudoElement" property instead of "pseudoClass"
which was the previous, less accurate name.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (147537 => 147538)


--- trunk/LayoutTests/ChangeLog	2013-04-03 11:06:26 UTC (rev 147537)
+++ trunk/LayoutTests/ChangeLog	2013-04-03 11:06:49 UTC (rev 147538)
@@ -1,3 +1,18 @@
+2013-04-03  Antoine Quint  <[email protected]>
+
+        Web Inspector: crash in WebCore::InspectorLayerTreeAgent::buildObjectForLayer if a layer is created for an anonymous RenderObject (:first-letter)
+        https://bugs.webkit.org/show_bug.cgi?id=113768
+
+        Reviewed by Timothy Hatcher.
+
+        * inspector-protocol/layers/layers-anonymous-expected.txt: Added.
+        * inspector-protocol/layers/layers-anonymous.html: Added.
+        New test for layers created for CSS anonymous boxes or blocks.
+
+        * inspector-protocol/layers/layers-generated-content.html:
+        Update test to use the "pseudoElement" property instead of "pseudoClass"
+        which was the previous, less accurate name.
+
 2013-04-03  Zoltan Arvai  <[email protected]>
 
         [Qt] Unreviewed gardening. Rebaselining after r147530.

Added: trunk/LayoutTests/inspector-protocol/layers/layers-anonymous-expected.txt (0 => 147538)


--- trunk/LayoutTests/inspector-protocol/layers/layers-anonymous-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector-protocol/layers/layers-anonymous-expected.txt	2013-04-03 11:06:49 UTC (rev 147538)
@@ -0,0 +1,28 @@
+This is a test
+
+
+=== Enable the LayerTree agent ===
+
+PASS
+
+=== Get the Document ===
+
+PASS
+
+=== Get the layer tree ===
+
+PASS
+
+=== Check layers ===
+
+PASS: Expected number of anonymous layers.
+PASS: The sole anonymous layer has a non-zero node id.
+PASS: The sole anonymous layer has a :first-letter pseudo-element.
+
+=== Check node ===
+
+PASS: Node was found.
+PASS: Node has expected localName.
+PASS: Node has id.
+PASS: Node has expected id.
+

Copied: trunk/LayoutTests/inspector-protocol/layers/layers-anonymous.html (from rev 147536, trunk/LayoutTests/inspector-protocol/layers/layers-generated-content.html) (0 => 147538)


--- trunk/LayoutTests/inspector-protocol/layers/layers-anonymous.html	                        (rev 0)
+++ trunk/LayoutTests/inspector-protocol/layers/layers-anonymous.html	2013-04-03 11:06:49 UTC (rev 147538)
@@ -0,0 +1,140 @@
+<html>
+<head>
+<script type="text/_javascript_" src=""
+<script type="text/_javascript_">
+
+function test()
+{
+
+    var nodes;
+ 
+    InspectorTest.eventHandler["DOM.setChildNodes"] = setChildNodes;
+
+    enableLayerTreeAgent();
+    
+    function enableLayerTreeAgent(result)
+    {
+        step({
+            name: "Enable the LayerTree agent",
+            command: "LayerTree.enable",
+            parameters: {},
+            callback: getDocument
+        });
+    };
+
+    function getDocument()
+    {
+        // We must first get the document so that later on we may get sensible nodeIds.
+        step({
+            name: "Get the Document",
+            command: "DOM.getDocument",
+            parameters: {},
+            callback: getLayerTree
+        });
+    };
+
+    function getLayerTree(result)
+    {
+        step({
+            name: "Get the layer tree",
+            command: "LayerTree.layersForNode",
+            parameters: {"nodeId": result.root.nodeId},
+            callback: gotLayerTree
+        });
+    };
+
+    function gotLayerTree(result)
+    {
+        var anonymousLayers = result.layers.filter(function (layer) {
+            return layer.isAnonymous;
+        });
+
+        logTestName("Check layers");
+
+        assert("Expected number of anonymous layers", anonymousLayers.length, 1);
+        assert("The sole anonymous layer has a non-zero node id", anonymousLayers[0].nodeId !== 0, true);
+        assert("The sole anonymous layer has a :first-letter pseudo-element", anonymousLayers[0].pseudoElement === "first-letter", true);
+
+        var node;
+        var nodeId = anonymousLayers[0].nodeId;
+        for (var i = 0, count = nodes.length; i < count; ++i) {
+            if (nodes[i].nodeId === nodeId) {
+                node = nodes[i];
+                break;
+            }
+        }
+
+        logTestName("Check node");
+        
+        assert("Node was found", !!node, true);
+        assert("Node has expected localName", node.localName, "p");
+        assert("Node has id", node.attributes[0], "id");
+        assert("Node has expected id", node.attributes[1], "first-letter");
+        
+        InspectorTest.completeTest();
+    };
+
+    function setChildNodes (messageObject) {
+        nodes = messageObject.params.nodes;
+    };
+
+    function step(test)
+    {
+        if (test.callback)
+            logTestName(test.name);
+        runCommand(test);
+    };
+
+    function logTestName(name)
+    {
+        InspectorTest.log("\n=== " + name + " ===\n");
+    };
+
+    function runCommand(command)
+    {
+        InspectorTest.sendCommand(command.command, command.parameters, function(messageObject) {
+            if (messageObject.hasOwnProperty("error")) {
+                InspectorTest.log("FAIL: " + messageObject.error.message + " (" + messageObject.error.code + ")");
+                InspectorTest.completeTest();
+                return;
+            }
+
+            if (command.name)
+                InspectorTest.log("PASS");
+
+            if (command.callback)
+                command.callback(messageObject.result);
+        });
+    };
+
+    function assert(name, actual, expected)
+    {
+        if (expected === actual)
+            InspectorTest.log("PASS: " + name + ".");
+        else
+            InspectorTest.log("FAIL: " + name + ". Expected " + expected + " but got " + actual);
+    };
+
+};
+
+window.addEventListener("DOMContentLoaded", function()
+{
+    runTest();
+}, false);
+
+</script>
+<style type="text/css">
+
+    #first-letter::first-letter {
+        float: left;
+        -webkit-backface-visibility: hidden;
+    }
+
+</style>
+</head>
+<body>
+
+    <p id="first-letter">This is a test</p>
+
+</body>
+</html>

Modified: trunk/LayoutTests/inspector-protocol/layers/layers-generated-content.html (147537 => 147538)


--- trunk/LayoutTests/inspector-protocol/layers/layers-generated-content.html	2013-04-03 11:06:26 UTC (rev 147537)
+++ trunk/LayoutTests/inspector-protocol/layers/layers-generated-content.html	2013-04-03 11:06:49 UTC (rev 147538)
@@ -51,9 +51,9 @@
             if (!layer.isGeneratedContent)
                 return;
 
-            if (layer.pseudoClass === "before")
+            if (layer.pseudoElement === "before")
                 beforeLayers.push(layer);
-            if (layer.pseudoClass === "after")
+            if (layer.pseudoElement === "after")
                 afterLayers.push(layer);
         });
 

Modified: trunk/Source/WebCore/ChangeLog (147537 => 147538)


--- trunk/Source/WebCore/ChangeLog	2013-04-03 11:06:26 UTC (rev 147537)
+++ trunk/Source/WebCore/ChangeLog	2013-04-03 11:06:49 UTC (rev 147538)
@@ -1,3 +1,36 @@
+2013-04-03  Antoine Quint  <[email protected]>
+
+        Web Inspector: crash in WebCore::InspectorLayerTreeAgent::buildObjectForLayer if a layer is created for an anonymous RenderObject (:first-letter)
+        https://bugs.webkit.org/show_bug.cgi?id=113768
+
+        The InspectorLayerTreeAgent now knows how to deal with anonymous RenderObjects
+        for the :first-letter and :first-line pseudo-elements.
+
+        Reviewed by Timothy Hatcher.
+
+        Test: inspector-protocol/layers/layers-anonymous.html
+
+        * inspector/Inspector.json:
+        Change the Layer type in the LayerTree domain to have more accurate terminology
+        (pseudo-element vs. pseudo-class) and a new optional isAnonymous flag for layers
+        associated to anonymous RenderObjects.
+
+        * inspector/InspectorLayerTreeAgent.cpp:
+        (WebCore::InspectorLayerTreeAgent::buildObjectForLayer):
+        Check for anonymous RenderObjects and set the parent renderer's node as the node
+        for this layer since anonymous renderers have the Document as their node and this
+        would not be satisfactory to show in a front-end. We also check for anonymous
+        RenderObjects for the :first-letter and :first-line pseudo-elements and set them
+        in the pseudoElement property of the Layer object created such that a front-end
+        could correctly identify what type of pseudo-element for the associated node
+        yielded this layer.
+
+        (WebCore::InspectorLayerTreeAgent::idForNode):
+        Here we fix the actual crash by first checking if the node provided is null and
+        returning 0 if there's no such node. This ensures that any scenario where there
+        is no node associated with the RenderObject simply informs of the front-end that
+        no such node exists and we do not crash.
+
 2013-04-03  Alexander Pavlov  <[email protected]>
 
         Web Inspector: [REGRESSION(r147117)][Elements] Copy/paste keyboard shortcuts broken in "Edit as HTML"

Modified: trunk/Source/WebCore/inspector/Inspector.json (147537 => 147538)


--- trunk/Source/WebCore/inspector/Inspector.json	2013-04-03 11:06:26 UTC (rev 147537)
+++ trunk/Source/WebCore/inspector/Inspector.json	2013-04-03 11:06:49 UTC (rev 147538)
@@ -3739,8 +3739,9 @@
                     { "name": "isInShadowTree", "type": "boolean", "optional": true, "description": "Indicates whether this layer is associated with an element hosted in a shadow tree." },
                     { "name": "isReflection", "type": "boolean", "optional": true, "description": "Indicates whether this layer was used to provide a reflection for the element." },
                     { "name": "isGeneratedContent", "type": "boolean", "optional": true, "description": "Indicates whether the layer is attached to a pseudo element that is CSS generated content." },
+                    { "name": "isAnonymous", "type": "boolean", "optional": true, "description": "Indicates whether the layer was created for a CSS anonymous block or box." },
                     { "name": "pseudoElementId", "$ref": "PseudoElementId", "optional": true, "description": "The id for the pseudo element associated with this layer." },
-                    { "name": "pseudoClass", "type": "string", "optional": true, "description": "The name of the CSS pseudo-class that prompted the layer's content to be generated." }
+                    { "name": "pseudoElement", "type": "string", "optional": true, "description": "The name of the CSS pseudo-element that prompted the layer to be generated." }
                 ]
             },
             {

Modified: trunk/Source/WebCore/inspector/InspectorLayerTreeAgent.cpp (147537 => 147538)


--- trunk/Source/WebCore/inspector/InspectorLayerTreeAgent.cpp	2013-04-03 11:06:26 UTC (rev 147537)
+++ trunk/Source/WebCore/inspector/InspectorLayerTreeAgent.cpp	2013-04-03 11:06:49 UTC (rev 147538)
@@ -165,12 +165,13 @@
 
     bool isReflection = renderLayer->isReflection();
     bool isGenerated = (isReflection ? renderer->parent() : renderer)->isBeforeOrAfterContent();
+    bool isAnonymous = renderer->isAnonymous();
 
     if (isReflection && isGenerated)
         node = renderer->parent()->generatingNode();
     else if (isGenerated)
         node = renderer->generatingNode();
-    else if (isReflection)
+    else if (isReflection || isAnonymous)
         node = renderer->parent()->node();
 
     // Basic set of properties.
@@ -194,16 +195,29 @@
         layerObject->setIsGeneratedContent(true);
         layerObject->setPseudoElementId(bindPseudoElement(static_cast<PseudoElement*>(renderer->node())));
         if (renderer->isBeforeContent())
-            layerObject->setPseudoClass("before");
+            layerObject->setPseudoElement("before");
         else if (renderer->isAfterContent())
-            layerObject->setPseudoClass("after");
+            layerObject->setPseudoElement("after");
     }
 
+    if (isAnonymous) {
+        layerObject->setIsAnonymous(true);
+        if (RenderStyle* style = renderer->style()) {
+            if (style->styleType() == FIRST_LETTER)
+                layerObject->setPseudoElement("first-letter");
+            else if (style->styleType() == FIRST_LINE)
+                layerObject->setPseudoElement("first-line");
+        }
+    }
+
     return layerObject;
 }
 
 int InspectorLayerTreeAgent::idForNode(ErrorString* errorString, Node* node)
 {
+    if (!node)
+        return 0;
+
     InspectorDOMAgent* domAgent = m_instrumentingAgents->inspectorDOMAgent();
     
     int nodeId = domAgent->boundNodeId(node);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to