Title: [107059] trunk
- Revision
- 107059
- Author
- [email protected]
- Date
- 2012-02-08 02:42:55 -0800 (Wed, 08 Feb 2012)
Log Message
[Qt] REGRESSION(r106918): It made svg/zoom/page/zoom-foreignObject.svg crash with Qt5-WK1
https://bugs.webkit.org/show_bug.cgi?id=77995
Patch by Nikolas Zimmermann <[email protected]> on 2012-02-08
Reviewed by Csaba Osztrogonác.
Source/WebCore:
>From the stack traces it's obvious that SVGImageChromeClient tried to invalidate the root view,
while its SVGImage was being destructed, due to an updateStyleIfNeeded() call, coming
from frameDetached(). There's no point in redrawing there, so we should just stop it.
Covered by existing tests on the Qt but, unfortunately I couldn't reproduce it on Mac.
* svg/graphics/SVGImage.cpp:
(WebCore::SVGImageChromeClient::invalidateContentsAndRootView): Stop invalidating if m_page is 0.
(WebCore::SVGImage::~SVGImage): Clear m_page, so that SVGImageChromeClient knows we're destructing.
* svg/graphics/SVGImage.h:
LayoutTests:
* platform/qt/Skipped: Unskip previously skipped tests.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (107058 => 107059)
--- trunk/LayoutTests/ChangeLog 2012-02-08 10:39:13 UTC (rev 107058)
+++ trunk/LayoutTests/ChangeLog 2012-02-08 10:42:55 UTC (rev 107059)
@@ -1,3 +1,12 @@
+2012-02-08 Nikolas Zimmermann <[email protected]>
+
+ [Qt] REGRESSION(r106918): It made svg/zoom/page/zoom-foreignObject.svg crash with Qt5-WK1
+ https://bugs.webkit.org/show_bug.cgi?id=77995
+
+ Reviewed by Csaba Osztrogonác.
+
+ * platform/qt/Skipped: Unskip previously skipped tests.
+
2012-02-08 Pablo Flouret <[email protected]>
Add state attribute to history's dom interface.
Modified: trunk/LayoutTests/platform/qt/Skipped (107058 => 107059)
--- trunk/LayoutTests/platform/qt/Skipped 2012-02-08 10:39:13 UTC (rev 107058)
+++ trunk/LayoutTests/platform/qt/Skipped 2012-02-08 10:42:55 UTC (rev 107059)
@@ -2538,8 +2538,3 @@
# [Qt] REGRESSION(r106918): It made svg/as-object/nested-embedded-svg-size-changes.html fail in debug mode
# https://bugs.webkit.org/show_bug.cgi?id=78026
svg/as-object/nested-embedded-svg-size-changes.html
-
-# [Qt] REGRESSION(r106918): It made svg/zoom/page/zoom-foreignObject.svg crash with Qt5-WK1
-# https://bugs.webkit.org/show_bug.cgi?id=77995
-svg/zoom/page/zoom-background-images.html
-svg/zoom/page/zoom-coords-viewattr-01-b.svg
Modified: trunk/Source/WebCore/ChangeLog (107058 => 107059)
--- trunk/Source/WebCore/ChangeLog 2012-02-08 10:39:13 UTC (rev 107058)
+++ trunk/Source/WebCore/ChangeLog 2012-02-08 10:42:55 UTC (rev 107059)
@@ -1,3 +1,21 @@
+2012-02-08 Nikolas Zimmermann <[email protected]>
+
+ [Qt] REGRESSION(r106918): It made svg/zoom/page/zoom-foreignObject.svg crash with Qt5-WK1
+ https://bugs.webkit.org/show_bug.cgi?id=77995
+
+ Reviewed by Csaba Osztrogonác.
+
+ From the stack traces it's obvious that SVGImageChromeClient tried to invalidate the root view,
+ while its SVGImage was being destructed, due to an updateStyleIfNeeded() call, coming
+ from frameDetached(). There's no point in redrawing there, so we should just stop it.
+
+ Covered by existing tests on the Qt but, unfortunately I couldn't reproduce it on Mac.
+
+ * svg/graphics/SVGImage.cpp:
+ (WebCore::SVGImageChromeClient::invalidateContentsAndRootView): Stop invalidating if m_page is 0.
+ (WebCore::SVGImage::~SVGImage): Clear m_page, so that SVGImageChromeClient knows we're destructing.
+ * svg/graphics/SVGImage.h:
+
2012-02-08 Pablo Flouret <[email protected]>
Add state attribute to history's dom interface.
Modified: trunk/Source/WebCore/svg/graphics/SVGImage.cpp (107058 => 107059)
--- trunk/Source/WebCore/svg/graphics/SVGImage.cpp 2012-02-08 10:39:13 UTC (rev 107058)
+++ trunk/Source/WebCore/svg/graphics/SVGImage.cpp 2012-02-08 10:42:55 UTC (rev 107059)
@@ -75,7 +75,8 @@
virtual void invalidateContentsAndRootView(const IntRect& r, bool)
{
- if (m_image && m_image->imageObserver())
+ // If m_image->m_page is null, we're being destructed, don't fire changedInRect() in that case.
+ if (m_image && m_image->imageObserver() && m_image->m_page)
m_image->imageObserver()->changedInRect(m_image, r);
}
@@ -90,12 +91,9 @@
SVGImage::~SVGImage()
{
if (m_page) {
- m_page->mainFrame()->loader()->frameDetached(); // Break both the loader and view references to the frame
-
- // Clear explicitly because we want to delete the page before the ChromeClient.
- // FIXME: I believe that's already guaranteed by C++ object destruction rules,
- // so this may matter only for the assertion below.
- m_page.clear();
+ // Store m_page in a local variable, clearing m_page, so that SVGImageChromeClient knows we're destructed.
+ OwnPtr<Page> currentPage = m_page.release();
+ currentPage->mainFrame()->loader()->frameDetached(); // Break both the loader and view references to the frame
}
// Verify that page teardown destroyed the Chrome
Modified: trunk/Source/WebCore/svg/graphics/SVGImage.h (107058 => 107059)
--- trunk/Source/WebCore/svg/graphics/SVGImage.h 2012-02-08 10:39:13 UTC (rev 107058)
+++ trunk/Source/WebCore/svg/graphics/SVGImage.h 2012-02-08 10:42:55 UTC (rev 107059)
@@ -61,6 +61,7 @@
virtual bool hasRelativeHeight() const;
private:
+ friend class SVGImageChromeClient;
virtual ~SVGImage();
virtual String filenameExtension() const;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
