Reviewed by Maciej. (Patch by me, Maciej, and Harrison.)
authorbdakin <bdakin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 23 Jul 2006 23:36:45 +0000 (23:36 +0000)
committerbdakin <bdakin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 23 Jul 2006 23:36:45 +0000 (23:36 +0000)
        Fix for <rdar://problem/4529398> WebCore crashes when pasting rich
        text - WebCore::InlineBox::root()

        The initial rendering crash was due to a render object having a
        stale reference to an inline box that had already been deleted and
        then recreated in the exact same location in memory. (Crazy, I
        know.) The situation seemed pretty specific to list markers
        according to Hyatt according to Maciej, so that is what I patched
        specifically. Fixing this crash unearthed a separate editing crash
        where we were trying to insert a block into itself. I worked on
        that with Maciej and Harrison, and Harrison came up with a fix.

        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary): This is the fix for the editing crash. If paragraphStart is an atomic
        node, insert the new block into the parent instead.
        * rendering/InlineBox.cpp:
        (WebCore::InlineBox::isChildOfParent): This function is for
        posterity. It will help keep the linebox tree in check.
        * rendering/InlineBox.h:
        * rendering/InlineFlowBox.cpp:
        (WebCore::InlineFlowBox::addToLine): Added assert.
        (WebCore::InlineFlowBox::deleteLine): Added assert.
        * rendering/ListMarkerBox.cpp:
        (WebCore::ListMarkerBox::destroy): If this has a parent, call
        removeChild on this.
        (WebCore::ListMarkerBox::operator delete):
        * rendering/ListMarkerBox.h:

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

WebCore/ChangeLog
WebCore/editing/CompositeEditCommand.cpp
WebCore/rendering/InlineBox.cpp
WebCore/rendering/InlineBox.h
WebCore/rendering/InlineFlowBox.cpp
WebCore/rendering/ListMarkerBox.cpp
WebCore/rendering/ListMarkerBox.h

index fc941fd..6c8e7ed 100644 (file)
@@ -1,3 +1,35 @@
+2006-07-23  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Maciej. (Patch by me, Maciej, and Harrison.)
+
+        Fix for <rdar://problem/4529398> WebCore crashes when pasting rich 
+        text - WebCore::InlineBox::root()
+
+        The initial rendering crash was due to a render object having a 
+        stale reference to an inline box that had already been deleted and 
+        then recreated in the exact same location in memory. (Crazy, I 
+        know.) The situation seemed pretty specific to list markers 
+        according to Hyatt according to Maciej, so that is what I patched 
+        specifically. Fixing this crash unearthed a separate editing crash 
+        where we were trying to insert a block into itself. I worked on 
+        that with Maciej and Harrison, and Harrison came up with a fix.
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary): This is the fix for the editing crash. If paragraphStart is an atomic 
+        node, insert the new block into the parent instead.
+        * rendering/InlineBox.cpp:
+        (WebCore::InlineBox::isChildOfParent): This function is for 
+        posterity. It will help keep the linebox tree in check.
+        * rendering/InlineBox.h:
+        * rendering/InlineFlowBox.cpp:
+        (WebCore::InlineFlowBox::addToLine): Added assert.
+        (WebCore::InlineFlowBox::deleteLine): Added assert.
+        * rendering/ListMarkerBox.cpp:
+        (WebCore::ListMarkerBox::destroy): If this has a parent, call 
+        removeChild on this.
+        (WebCore::ListMarkerBox::operator delete):
+        * rendering/ListMarkerBox.h:
+
 2006-07-23  Alice Liu  <alice.liu@apple.com>
 
         Reviewed by Darin.
index 06b8fff..8da332b 100644 (file)
@@ -588,7 +588,13 @@ void CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Posi
         moveNode = moveNode->traverseNextNode();
     Node *endNode = end.node();
     
-    insertNodeAt(newBlock.get(), paragraphStart.node(), paragraphStart.offset());
+    if (!isAtomicNode(paragraphStart.node()))
+        insertNodeAt(newBlock.get(), paragraphStart.node(), paragraphStart.offset());
+    else {
+        ASSERT(paragraphStart.offset() <= 1);
+        ASSERT(paragraphStart.node()->parentNode());
+        insertNodeAt(newBlock.get(), paragraphStart.node()->parentNode(), paragraphStart.node()->nodeIndex() + paragraphStart.offset());
+    }
 
     while (moveNode && !isBlock(moveNode)) {
         Node *next = moveNode->traverseNextSibling();
index b8f18a5..5dcbe9b 100644 (file)
@@ -155,6 +155,18 @@ bool InlineBox::nodeAtPoint(RenderObject::NodeInfo& i, int x, int y, int tx, int
     return object()->hitTest(i, x, y, tx, ty);
 }
 
+bool InlineBox::isChildOfParent()
+{
+    if (!m_parent)
+        return true;
+
+    for (InlineBox* box = m_parent->firstChild(); box; box = box->nextOnLine())
+        if (box == this)
+            return true;
+
+    return false;
+}
+
 RootInlineBox* InlineBox::root()
 { 
     if (m_parent)
index 7d6a57d..c1a58d5 100644 (file)
@@ -118,6 +118,8 @@ public:
     InlineFlowBox* parent() const { return m_parent; }
     void setParent(InlineFlowBox* par) { m_parent = par; }
 
+    bool isChildOfParent();
+
     RootInlineBox* root();
     
     void setWidth(int w) { m_width = w; }
index 9186e13..cbb99fa 100644 (file)
@@ -87,7 +87,10 @@ int InlineFlowBox::getFlowSpacingWidth()
     return totWidth;
 }
 
-void InlineFlowBox::addToLine(InlineBox* child) {
+void InlineFlowBox::addToLine(InlineBox* child) 
+{
+    ASSERT(!child->parent());
+
     if (!m_firstChild)
         m_firstChild = m_lastChild = child;
     else {
@@ -127,6 +130,7 @@ void InlineFlowBox::deleteLine(RenderArena* arena)
     InlineBox* child = m_firstChild;
     InlineBox* next = 0;
     while (child) {
+        ASSERT(this == child->parent());
         next = child->nextOnLine();
         child->deleteLine(arena);
         child = next;
index 86866e9..48122ea 100644 (file)
 #include "config.h"
 #include "ListMarkerBox.h"
 
+#include "InlineFlowBox.h"
+#include "RenderArena.h"
 #include "RenderListMarker.h"
 
 namespace WebCore {
 
+#ifndef NDEBUG
+static bool listMarkerBoxDetach;
+#endif
+
 ListMarkerBox::ListMarkerBox(RenderObject* obj)
     : InlineBox(obj)
 {
 }
 
+void ListMarkerBox::destroy(RenderArena* arena)
+{
+    #ifndef NDEBUG
+        listMarkerBoxDetach = true;
+    #endif
+    if (m_parent)
+        m_parent->removeChild(this);
+    delete this;
+    #ifndef NDEBUG
+        listMarkerBoxDetach = false;
+    #endif
+
+    // Recover the size left there for us by operator delete and free the memory.
+    arena->free(*(size_t *)this, this);
+}
+
+void ListMarkerBox::operator delete(void* ptr, size_t sz)
+{
+    assert(listMarkerBoxDetach);
+
+    // Stash size where destroy can find it.
+    *(size_t *)ptr = sz;
+}
+
 bool ListMarkerBox::isText() const
 {
     return !static_cast<RenderListMarker*>(object())->listImage();
index 1809415..c013fb5 100644 (file)
@@ -33,6 +33,11 @@ class ListMarkerBox : public InlineBox
 {
 public:
     ListMarkerBox(RenderObject*);
+    virtual void destroy(RenderArena*);
+    
+    // Overridden to prevent the normal delete from being called.
+    void operator delete(void* ptr, size_t sz);
+    
     virtual bool isText() const;
 };