Title: [157605] trunk
Revision
157605
Author
ach...@adobe.com
Date
2013-10-17 15:08:18 -0700 (Thu, 17 Oct 2013)

Log Message

Web Inspector: [CSS Regions] Crash when highlighting a node of a flow with no regions
https://bugs.webkit.org/show_bug.cgi?id=122993

Reviewed by Joseph Pecoraro.

Source/WebCore:

Test: inspector-protocol/dom/highlight-flow-with-no-region.html

Even if a named flow has no regions the content of the flow will still have renderer objects created.
Removed the assumption that all renderers inside a RenderFlowThread will always have an enclosing RenderRegion.

* inspector/InspectorOverlay.cpp:
(WebCore::buildObjectForRendererFragments):
(WebCore::InspectorOverlay::buildObjectForHighlightedNode):

LayoutTests:

Added test to check that DOM.highlightNode is not crashing WebCore when the node is inside
a flow with no associated regions.

* inspector-protocol/dom/highlight-flow-with-no-region-expected.txt: Added.
* inspector-protocol/dom/highlight-flow-with-no-region.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (157604 => 157605)


--- trunk/LayoutTests/ChangeLog	2013-10-17 21:45:40 UTC (rev 157604)
+++ trunk/LayoutTests/ChangeLog	2013-10-17 22:08:18 UTC (rev 157605)
@@ -1,3 +1,16 @@
+2013-10-17  Alexandru Chiculita  <ach...@adobe.com>
+
+        Web Inspector: [CSS Regions] Crash when highlighting a node of a flow with no regions
+        https://bugs.webkit.org/show_bug.cgi?id=122993
+
+        Reviewed by Joseph Pecoraro.
+
+        Added test to check that DOM.highlightNode is not crashing WebCore when the node is inside
+        a flow with no associated regions.
+
+        * inspector-protocol/dom/highlight-flow-with-no-region-expected.txt: Added.
+        * inspector-protocol/dom/highlight-flow-with-no-region.html: Added.
+
 2013-10-17  Nico Weber  <tha...@chromium.org>
 
         Fix three bugs in the equals() implementations for css gradients.

Added: trunk/LayoutTests/inspector-protocol/dom/highlight-flow-with-no-region-expected.txt (0 => 157605)


--- trunk/LayoutTests/inspector-protocol/dom/highlight-flow-with-no-region-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector-protocol/dom/highlight-flow-with-no-region-expected.txt	2013-10-17 22:08:18 UTC (rev 157605)
@@ -0,0 +1 @@
+Testing that the backend is not crashing when highlighting a node that has no associated region.

Added: trunk/LayoutTests/inspector-protocol/dom/highlight-flow-with-no-region.html (0 => 157605)


--- trunk/LayoutTests/inspector-protocol/dom/highlight-flow-with-no-region.html	                        (rev 0)
+++ trunk/LayoutTests/inspector-protocol/dom/highlight-flow-with-no-region.html	2013-10-17 22:08:18 UTC (rev 157605)
@@ -0,0 +1,49 @@
+<html>
+<head>
+<script type="text/_javascript_" src=""
+<style>
+#flow {
+    -webkit-flow-into: flow;
+}
+</style>
+<script>
+function test()
+{
+    InspectorTest.sendCommand("DOM.getDocument", {}, onGotDocument);
+
+    function onGotDocument(msg) {
+        InspectorTest.checkForError(msg);
+        var node = msg.result.root;
+        InspectorTest.sendCommand("DOM.querySelector", { "nodeId": node.nodeId, "selector": "#inspectedElement" }, onQuerySelector);
+    }
+
+    function onQuerySelector(msg) {
+        InspectorTest.checkForError(msg);
+        var node = msg.result;
+        var highlightConfig = {
+            showInfo: true,
+            contentColor: {r: 255, g: 255, b: 255},
+            paddingColor: {r: 255, g: 255, b: 255},
+            borderColor: {r: 255, g: 255, b: 255},
+            marginColor: {r: 255, g: 255, b: 255},
+        };
+        InspectorTest.sendCommand("DOM.highlightNode", { "highlightConfig": highlightConfig, "nodeId": node.nodeId }, onHighlightComplete);
+    }
+
+    function onHighlightComplete(msg) {
+        InspectorTest.checkForError(msg);
+        InspectorTest.completeTest();
+    }
+}
+</script>
+</head>
+<body _onload_="runTest()">
+
+<div id="flow">
+    <div id="inspectedElement"></div>
+</div>
+
+<p>Testing that the backend is not crashing when highlighting a node that has no associated region.</p>
+
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (157604 => 157605)


--- trunk/Source/WebCore/ChangeLog	2013-10-17 21:45:40 UTC (rev 157604)
+++ trunk/Source/WebCore/ChangeLog	2013-10-17 22:08:18 UTC (rev 157605)
@@ -1,3 +1,19 @@
+2013-10-17  Alexandru Chiculita  <ach...@adobe.com>
+
+        Web Inspector: [CSS Regions] Crash when highlighting a node of a flow with no regions
+        https://bugs.webkit.org/show_bug.cgi?id=122993
+
+        Reviewed by Joseph Pecoraro.
+
+        Test: inspector-protocol/dom/highlight-flow-with-no-region.html
+
+        Even if a named flow has no regions the content of the flow will still have renderer objects created.
+        Removed the assumption that all renderers inside a RenderFlowThread will always have an enclosing RenderRegion.
+
+        * inspector/InspectorOverlay.cpp:
+        (WebCore::buildObjectForRendererFragments):
+        (WebCore::InspectorOverlay::buildObjectForHighlightedNode):
+
 2013-10-17  Andreas Kling  <akl...@apple.com>
 
         CTTE: RenderMathMLOperator always has a MathMLElement.

Modified: trunk/Source/WebCore/inspector/InspectorOverlay.cpp (157604 => 157605)


--- trunk/Source/WebCore/inspector/InspectorOverlay.cpp	2013-10-17 21:45:40 UTC (rev 157604)
+++ trunk/Source/WebCore/inspector/InspectorOverlay.cpp	2013-10-17 22:08:18 UTC (rev 157605)
@@ -503,7 +503,7 @@
         RenderRegion* endRegion = nullptr;
         containingFlowThread->getRegionRangeForBox(enclosingBox, startRegion, endRegion);
         if (!startRegion) {
-            ASSERT_NOT_REACHED();
+            // The flow has no visible regions. The renderer is not visible on screen.
             return nullptr;
         }
         const RenderRegionList& regionList = containingFlowThread->renderRegionList();
@@ -593,13 +593,17 @@
     if (!renderer)
         return nullptr;
 
+    RefPtr<InspectorArray> highlightFragments = buildObjectForRendererFragments(renderer, m_nodeHighlightConfig);
+    if (!highlightFragments)
+        return nullptr;
+
     RefPtr<InspectorObject> highlightObject = InspectorObject::create();
 
     // The main view's scroll offset is shared across all quads.
     FrameView* mainView = m_page->mainFrame().view();
     highlightObject->setObject("scroll", buildObjectForPoint(!mainView->delegatesScrolling() ? mainView->visibleContentRect().location() : FloatPoint()));
 
-    highlightObject->setArray("fragments", buildObjectForRendererFragments(renderer, m_nodeHighlightConfig));
+    highlightObject->setArray("fragments", highlightFragments.release());
 
     if (m_nodeHighlightConfig.showInfo) {
         RefPtr<InspectorObject> elementInfo = buildObjectForElementInfo(node);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to