[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

rolandsteiner at chromium.org rolandsteiner at chromium.org
Wed Apr 7 23:48:37 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit a08e93603202eb9b5833f1006550a8c37ca088cb
Author: rolandsteiner at chromium.org <rolandsteiner at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Thu Nov 19 03:15:49 2009 +0000

    WebCore: Bug 31574 -  Crashing bug when removing <ruby> element
    (https://bugs.webkit.org/show_bug.cgi?id=31574)
    
    Reviewed by Darin Adler.
    
    Cause of the bug:
    1.) RenderBlock::destroy() of the RenderRubyRun called destroyLeftoverChildren()
    2.) that called destroy() of the RenderRubyBase(), which in RenderObject::destroy() calls remove()
    3.) remove() is being redirected as parent()->removeChild() in RenderObject.h
    4.) this triggers the special handling of child removal in RenderRubyRun that
        causes it to destroy itself
    5.) On returning from all this the renderer crashes when accessing a member
        or virtual function on this now illegal object.
    
    I therefore added a flag that tracks if the ruby run is being destroyed.
    If so, avoid doing the special handling in removeChild that caused this.
    It's not the most elegant solution, but the easiest to implement without
    touching unrelated code. Also, it's self-documenting.
    
    Test: fast/ruby/ruby-remove.html
    
    * rendering/RenderRubyRun.cpp:
    (WebCore::RenderRubyRun::RenderRubyRun):
    (WebCore::RenderRubyRun::destroy):
    (WebCore::RenderRubyRun::removeChild):
    * rendering/RenderRubyRun.h:
    
    LayoutTests: Bug 31574 -  Crashing bug when removing <ruby> element
    (https://bugs.webkit.org/show_bug.cgi?id=31574)
    
    Reviewed by Darin Adler.
    
    Layout test to verify it no longer crashes when the <ruby> element
    is being removed.
    
    * fast/ruby/ruby-remove-expected.txt: Added.
    * fast/ruby/ruby-remove.html: Added.
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@51169 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 9543ac5..11d170c 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2009-11-19  Roland Steiner  <rolandsteiner at chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Bug 31574 -  Crashing bug when removing <ruby> element
+        (https://bugs.webkit.org/show_bug.cgi?id=31574)
+        
+        Layout test to verify it no longer crashes when the <ruby> element
+        is being removed.
+
+        * fast/ruby/ruby-remove-expected.txt: Added.
+        * fast/ruby/ruby-remove.html: Added.
+
 2009-11-18  Kent Tamura  <tkent at chromium.org>
 
         Reviewed by Darin Adler.
diff --git a/LayoutTests/fast/ruby/ruby-remove-expected.txt b/LayoutTests/fast/ruby/ruby-remove-expected.txt
new file mode 100644
index 0000000..ab69dcf
--- /dev/null
+++ b/LayoutTests/fast/ruby/ruby-remove-expected.txt
@@ -0,0 +1 @@
+SUCCESS!
diff --git a/LayoutTests/fast/ruby/ruby-remove.html b/LayoutTests/fast/ruby/ruby-remove.html
new file mode 100644
index 0000000..b834583
--- /dev/null
+++ b/LayoutTests/fast/ruby/ruby-remove.html
@@ -0,0 +1,18 @@
+<html>
+<head>
+<script>
+function test()
+{
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    var ruby = document.getElementById("t");
+    ruby.innerHTML = ''; // This line mustn't crash 
+    document.getElementById("result").firstChild.data = 'SUCCESS!';
+}
+</script>
+</head>
+<body onload="test()">
+<div id="t"><ruby>f</div>
+<div id="result">FAILED!</div>
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index c8a8576..0c3a147 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2009-11-19  Roland Steiner  <rolandsteiner at chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Bug 31574 -  Crashing bug when removing <ruby> element
+        (https://bugs.webkit.org/show_bug.cgi?id=31574)
+
+        Cause of the bug:
+        1.) RenderBlock::destroy() of the RenderRubyRun called destroyLeftoverChildren()
+        2.) that called destroy() of the RenderRubyBase(), which in RenderObject::destroy() calls remove()
+        3.) remove() is being redirected as parent()->removeChild() in RenderObject.h
+        4.) this triggers the special handling of child removal in RenderRubyRun that
+            causes it to destroy itself
+        5.) On returning from all this the renderer crashes when accessing a member
+            or virtual function on this now illegal object.
+
+        I therefore added a flag that tracks if the ruby run is being destroyed.
+        If so, avoid doing the special handling in removeChild that caused this.
+        It's not the most elegant solution, but the easiest to implement without
+        touching unrelated code. Also, it's self-documenting.
+
+        Test: fast/ruby/ruby-remove.html
+
+        * rendering/RenderRubyRun.cpp:
+        (WebCore::RenderRubyRun::RenderRubyRun):
+        (WebCore::RenderRubyRun::destroy):
+        (WebCore::RenderRubyRun::removeChild):
+        * rendering/RenderRubyRun.h:
+
 2009-11-18  Laszlo Gombos  <laszlo.1.gombos at nokia.com>
 
         Reviewed by Kenneth Rohde Christiansen.
diff --git a/WebCore/rendering/RenderRubyRun.cpp b/WebCore/rendering/RenderRubyRun.cpp
index 561178b..8578c55 100644
--- a/WebCore/rendering/RenderRubyRun.cpp
+++ b/WebCore/rendering/RenderRubyRun.cpp
@@ -41,6 +41,7 @@ namespace WebCore {
 
 RenderRubyRun::RenderRubyRun(Node* node)
     : RenderBlock(node)
+    , m_beingDestroyed(false)
 {
     setReplaced(true);
     setInline(true);
@@ -50,6 +51,13 @@ RenderRubyRun::~RenderRubyRun()
 {
 }
 
+void RenderRubyRun::destroy()
+{
+    // Mark if the run is being destroyed to avoid trouble in removeChild().
+    m_beingDestroyed = true;
+    RenderBlock::destroy();
+}
+
 bool RenderRubyRun::hasRubyText() const
 {
     // The only place where a ruby text can be is in the first position
@@ -155,7 +163,7 @@ void RenderRubyRun::removeChild(RenderObject* child)
 {
     // If the child is a ruby text, then merge the ruby base with the base of
     // the right sibling run, if possible.
-    if (!documentBeingDestroyed() && child->isRubyText()) {
+    if (!m_beingDestroyed && !documentBeingDestroyed() && child->isRubyText()) {
         RenderRubyBase* base = rubyBase();
         RenderObject* rightNeighbour = nextSibling();
         if (base && rightNeighbour && rightNeighbour->isRubyRun()) {
@@ -169,7 +177,7 @@ void RenderRubyRun::removeChild(RenderObject* child)
 
     RenderBlock::removeChild(child);
 
-    if (!documentBeingDestroyed()) {
+    if (!m_beingDestroyed && !documentBeingDestroyed()) {
         // Check if our base (if any) is now empty. If so, destroy it.
         RenderBlock* base = rubyBase();
         if (base && !base->firstChild()) {
@@ -178,7 +186,7 @@ void RenderRubyRun::removeChild(RenderObject* child)
             base->destroy();
         }
 
-        // If any of the above leaves the run empty, destroy it as well
+        // If any of the above leaves the run empty, destroy it as well.
         if (isEmpty()) {
             parent()->removeChild(this);
             deleteLineBoxTree();
diff --git a/WebCore/rendering/RenderRubyRun.h b/WebCore/rendering/RenderRubyRun.h
index 52ee72c..361dfe5 100644
--- a/WebCore/rendering/RenderRubyRun.h
+++ b/WebCore/rendering/RenderRubyRun.h
@@ -46,6 +46,8 @@ public:
     RenderRubyRun(Node*);
     virtual ~RenderRubyRun();
 
+    virtual void destroy();
+
     virtual const char* renderName() const { return "RenderRubyRun (anonymous)"; }
 
     virtual bool isRubyRun() const { return true; }
@@ -68,6 +70,9 @@ public:
 
 protected:
     RenderRubyBase* createRubyBase() const;
+    
+private:
+    bool m_beingDestroyed;
 };
 
 } // namespace WebCore

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list