Crash at WebCore::SVGUseElement::expandSymbolElementsInShadowTree
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Feb 2012 17:06:11 +0000 (17:06 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Feb 2012 17:06:11 +0000 (17:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=77639

Patch by Stephen Chenney <schenney@chromium.org> on 2012-02-17
Reviewed by Nikolas Zimmermann.

Source/WebCore:

Fix a SVG crash in Release builds, although it still crashes in Debug builds.
The crash occurred when an SVG use element attempted to reference a style element while the file
contained an error causing the error banner to display. The fix is to prevent SVGUseElement
from recalculating style during tree building and return immediately when style is recalculated and
the tree is building.

Test: svg/custom/use-referencing-style-crash.svg

* svg/SVGUseElement.cpp:
(WebCore::SVGUseElement::willRecalcStyle): Return false if the tree is being built.
(WebCore::SVGUseElement::didRecalcStyle): Check and return if the tree
is being built and we are not yet ready for style update.

LayoutTests:

Fix a SVG crash in Release builds, although it still crashes in Debug builds.
This test is to verify no crash in Release builds, while expectations/Skipped
are added for Debug builds. Bug 77764 tracks the Debug fix.

* platform/chromium/test_expectations.txt:
* platform/gtk/Skipped:
* platform/mac/Skipped:
* platform/qt/Skipped:
* platform/win/Skipped:
* svg/custom/use-referencing-style-crash-expected.txt: Added.
* svg/custom/use-referencing-style-crash.svg: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@108084 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/chromium/test_expectations.txt
LayoutTests/platform/gtk/Skipped
LayoutTests/platform/mac/Skipped
LayoutTests/platform/qt/Skipped
LayoutTests/platform/win/Skipped
LayoutTests/svg/custom/use-referencing-style-crash-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/use-referencing-style-crash.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGUseElement.cpp

index 26f2772..cae79fc 100644 (file)
@@ -1,3 +1,22 @@
+2012-02-17  Stephen Chenney  <schenney@chromium.org>
+
+        Crash at WebCore::SVGUseElement::expandSymbolElementsInShadowTree
+        https://bugs.webkit.org/show_bug.cgi?id=77639
+
+        Reviewed by Nikolas Zimmermann.
+
+        Fix a SVG crash in Release builds, although it still crashes in Debug builds.
+        This test is to verify no crash in Release builds, while expectations/Skipped
+        are added for Debug builds. Bug 77764 tracks the Debug fix.
+
+        * platform/chromium/test_expectations.txt:
+        * platform/gtk/Skipped:
+        * platform/mac/Skipped:
+        * platform/qt/Skipped:
+        * platform/win/Skipped:
+        * svg/custom/use-referencing-style-crash-expected.txt: Added.
+        * svg/custom/use-referencing-style-crash.svg: Added.
+
 2012-02-17  Florin Malita  <fmalita@google.com>
 
         chrome.dll!WebCore::SVGTRefElement::updateReferencedText ReadAV@NULL (e85cb8e140071fa7790cad215b0109dc)
index 925996b..5dbc03f 100644 (file)
@@ -943,6 +943,9 @@ BUGCR23463 LEOPARD : svg/W3C-SVG-1.1/struct-symbol-01-b.svg = IMAGE
 BUGCR23463 LINUX WIN : svg/W3C-SVG-1.1/struct-symbol-01-b.svg = IMAGE+TEXT
 BUGCR23463 LINUX WIN : svg/W3C-SVG-1.1/struct-use-01-t.svg = PASS IMAGE+TEXT IMAGE
 
+// Crashes due to debug assert until we fix issues with style elements in SVG
+BUGWK77764 DEBUG : svg/custom/use-referencing-style-crash.svg = CRASH
+
 // Merge 39744:39829 - regression
 BUGCR10284 MAC : svg/custom/path-bad-data.svg = FAIL
 
index 3f53163..346a8fb 100644 (file)
@@ -852,6 +852,9 @@ svg/batik/paints/gradientLimit.svg
 svg/custom/circular-marker-reference-2.svg
 svg/custom/non-circular-marker-reference.svg
 
+# Crashes due to debug assert until we fix issues with style elements in SVG
+svg/custom/use-referencing-style-crash.svg
+
 # Canvas tests
 
 # Tests that fail across all platforms.
index d2475a2..5cde39a 100644 (file)
@@ -53,6 +53,10 @@ media/video-controls-transformed.html
 media/video-controls-zoomed.html
 media/video-source-error.html
 
+# Crashes due to debug assert until we fix issues with style elements in SVG
+# https://bugs.webkit.org/show_bug.cgi?id=77764
+svg/custom/use-referencing-style-crash.svg
+
 # This test requires media controls has a volume slider.
 media/video-volume-slider.html
 
index fd1f3b0..89d5ccd 100644 (file)
@@ -1409,6 +1409,9 @@ svg/custom/mask-child-changes.svg
 svg/custom/mask-invalidation.svg
 svg/custom/absolute-sized-content-with-resources.xhtml
 
+# Crashes due to debug assert until we fix issues with style elements in SVG
+svg/custom/use-referencing-style-crash.svg
+
 # ============================================================================= #
 # Failing CSS Tests
 # ============================================================================= #
index d03b6f9..fee7a1a 100644 (file)
@@ -227,6 +227,9 @@ svg/zoom/text/zoom-coords-viewattr-01-b.svg
 # https://bugs.webkit.org/show_bug.cgi?id=35013 (Impossible to test text-only-zoom from DRT on Windows)
 svg/zoom/text
 
+# Crashes due to debug assert until we fix issues with style elements in SVG
+svg/custom/use-referencing-style-crash.svg
+
 # No support for WebArchives in WebKitWin <rdar://problem/6436020>
 webarchive
 svg/webarchive
diff --git a/LayoutTests/svg/custom/use-referencing-style-crash-expected.txt b/LayoutTests/svg/custom/use-referencing-style-crash-expected.txt
new file mode 100644 (file)
index 0000000..1f3b2f3
--- /dev/null
@@ -0,0 +1,6 @@
+This page contains the following errors:
+
+error on line 9 at column 12: Extra content at the end of the document
+Below is a rendering of the page up to the first error.
+
+
diff --git a/LayoutTests/svg/custom/use-referencing-style-crash.svg b/LayoutTests/svg/custom/use-referencing-style-crash.svg
new file mode 100644 (file)
index 0000000..deecb53
--- /dev/null
@@ -0,0 +1,9 @@
+<!-- This test is designed to have errors in the svg content. It should not crash. -->
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+  <use xlink:href="#foo"/>
+  <script>
+    if (window.layoutTestController)
+      layoutTestController.dumpAsText();
+  </script>
+  <symbol id="foo">
+    <style>
index 9afde8d..86cce13 100644 (file)
@@ -1,3 +1,23 @@
+2012-02-17  Stephen Chenney  <schenney@chromium.org>
+
+        Crash at WebCore::SVGUseElement::expandSymbolElementsInShadowTree
+        https://bugs.webkit.org/show_bug.cgi?id=77639
+
+        Reviewed by Nikolas Zimmermann.
+
+        Fix a SVG crash in Release builds, although it still crashes in Debug builds.
+        The crash occurred when an SVG use element attempted to reference a style element while the file
+        contained an error causing the error banner to display. The fix is to prevent SVGUseElement
+        from recalculating style during tree building and return immediately when style is recalculated and
+        the tree is building.
+
+        Test: svg/custom/use-referencing-style-crash.svg
+
+        * svg/SVGUseElement.cpp:
+        (WebCore::SVGUseElement::willRecalcStyle): Return false if the tree is being built.
+        (WebCore::SVGUseElement::didRecalcStyle): Check and return if the tree
+        is being built and we are not yet ready for style update.
+
 2012-02-17  Ilya Tikhonovsky  <loislo@chromium.org>
 
         Unreviewed, rolling out r108077.
index fd7f2d2..e342ee4 100644 (file)
@@ -345,14 +345,21 @@ bool SVGUseElement::willRecalcStyle(StyleChange change)
         if (SVGElement* shadowRoot = m_targetElementInstance->shadowTreeElement())
             shadowRoot->setNeedsStyleRecalc();
     }
+    // Do not do style calculation during shadow tree construction because it may cause nodes to
+    // be attached before they should be. Style recalc will happen when the tree is constructed
+    // and explicitly attached.
+    if (m_updatesBlocked)
+        return false;
     return true;
 }
 
 void SVGUseElement::didRecalcStyle(StyleChange change)
 {
     // Assure that the shadow tree has not been marked for recreation, while we're building it.
-    if (m_updatesBlocked)
-        ASSERT(!m_needsShadowTreeRecreation);
+    if (m_updatesBlocked && m_needsShadowTreeRecreation) {
+        // We are about to recreate the tree while in the middle of recreating the tree.
+        return;
+    }
 
     RenderSVGShadowTreeRootContainer* shadowRoot = static_cast<RenderSVGShadowTreeRootContainer*>(renderer());
     if (!shadowRoot)