Changeset 107059 in webkit

Timestamp:

Feb 8, 2012 2:42:55 AM (12 years ago)

Author:

Csaba Osztrogonác

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 <​nzimmermann@rim.com> 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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/qt/Skipped (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/svg/graphics/SVGImage.cpp (modified) (2 diffs)
• Source/WebCore/svg/graphics/SVGImage.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-02-08  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        [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  <pablof@motorola.com>
 

trunk/LayoutTests/platform/qt/Skipped

@@ -2539 +2539 @@
 # 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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-02-08  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        [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  <pablof@motorola.com>
 

trunk/Source/WebCore/svg/graphics/SVGImage.cpp

@@ -76 +76 @@
     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);
     }
@@ -91 +92 @@
 {
     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
     }
 

trunk/Source/WebCore/svg/graphics/SVGImage.h

@@ -62 +62 @@
 
 private:
+    friend class SVGImageChromeClient;
     virtual ~SVGImage();