Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Nov 2004 19:53:33 +0000 (19:53 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Nov 2004 19:53:33 +0000 (19:53 +0000)
        Fix for this bug:

        <rdar://problem/3879569> Many leaks in EditCommand mechanism, seen in Mail

        Fixed a couple of object lifetime issues. The EditCommand class used to hold an
        EditCommandPtr to its parent, but this caused a a reference cycle in composite
        commands as the children held a ref to their parent. Now, the parent variable
        is a non-retained reference to an EditCommand *. It would be nice to have a
        weak reference to the parent or even override deref in composite commands (but I
        can't since deref() is not virtual). However, this should be OK since any
        dangling parent pointer is a sign of a bigger object lifetime problem that
        would need to be addressed anyway.

        * khtml/css/css_valueimpl.cpp:
        (CSSStyleDeclarationImpl::CSSStyleDeclarationImpl): Fix bug in constructor that takes a
        QPtrList<CSSProperty> *. List values must be copied into newly-allocated list, rather than
        just assigning the list variable passed in to the local list variable, or the list will be
        double-deleted.
        * khtml/editing/htmlediting.cpp:
        (khtml::EditCommand::setStartingSelection): No longer call get(). m_parent is no longer a smart pointer.
        (khtml::EditCommand::setEndingSelection): Ditto.
        (khtml::EditCommand::assignTypingStyle): Short-circuit if passed in style is identical to current style.
        Unrelated to the change, but saves some ref's and deref's.
        (khtml::EditCommand::setTypingStyle): No longer call get(). m_parent is no longer a smart pointer.
        * khtml/editing/htmlediting.h: Change m_parent to a EditCommand *. Was an EditCommandPtr. Using an
        EditCommandPtr caused a reference cycle in composite commands as the children held a ref to their parent.
        (khtml::EditCommand::parent): No longer call get(). m_parent is no longer a smart pointer.

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/css/css_valueimpl.cpp
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/htmlediting.h

index a05fab0132f657cfbd8cda311585f8f56bbca39e..5eadb1b11023d90b564aaa511dcbcebeced517c0 100644 (file)
@@ -1,3 +1,35 @@
+2004-11-15  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+
+        Fix for this bug:
+        
+        <rdar://problem/3879569> Many leaks in EditCommand mechanism, seen in Mail
+
+        Fixed a couple of object lifetime issues. The EditCommand class used to hold an
+        EditCommandPtr to its parent, but this caused a a reference cycle in composite 
+        commands as the children held a ref to their parent. Now, the parent variable
+        is a non-retained reference to an EditCommand *. It would be nice to have a 
+        weak reference to the parent or even override deref in composite commands (but I
+        can't since deref() is not virtual). However, this should be OK since any
+        dangling parent pointer is a sign of a bigger object lifetime problem that
+        would need to be addressed anyway.
+
+        * khtml/css/css_valueimpl.cpp:
+        (CSSStyleDeclarationImpl::CSSStyleDeclarationImpl): Fix bug in constructor that takes a 
+        QPtrList<CSSProperty> *. List values must be copied into newly-allocated list, rather than
+        just assigning the list variable passed in to the local list variable, or the list will be 
+        double-deleted.
+        * khtml/editing/htmlediting.cpp:
+        (khtml::EditCommand::setStartingSelection): No longer call get(). m_parent is no longer a smart pointer.
+        (khtml::EditCommand::setEndingSelection): Ditto.
+        (khtml::EditCommand::assignTypingStyle): Short-circuit if passed in style is identical to current style.
+        Unrelated to the change, but saves some ref's and deref's.
+        (khtml::EditCommand::setTypingStyle): No longer call get(). m_parent is no longer a smart pointer.
+        * khtml/editing/htmlediting.h: Change m_parent to a EditCommand *. Was an EditCommandPtr. Using an
+        EditCommandPtr caused a reference cycle in composite commands as the children held a ref to their parent.
+        (khtml::EditCommand::parent): No longer call get(). m_parent is no longer a smart pointer.
+
 2004-11-15  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Kevin.
index 2073d9dadf15640dd7f6f4ee8f18cc205fa10ff9..24765c2f4b660cfd1212d60b8fbe275cff2a8853 100644 (file)
@@ -63,7 +63,17 @@ CSSStyleDeclarationImpl::CSSStyleDeclarationImpl(CSSRuleImpl *parent)
 CSSStyleDeclarationImpl::CSSStyleDeclarationImpl(CSSRuleImpl *parent, QPtrList<CSSProperty> *lstValues)
     : StyleBaseImpl(parent)
 {
-    m_lstValues = lstValues;
+    if (!lstValues) {
+        m_lstValues = 0;
+    }
+    else {
+        m_lstValues = new QPtrList<CSSProperty>;
+        m_lstValues->setAutoDelete(true);
+
+        QPtrListIterator<CSSProperty> lstValuesIt(*lstValues);
+        for (lstValuesIt.toFirst(); lstValuesIt.current(); ++lstValuesIt)
+            m_lstValues->append(new CSSProperty(*lstValuesIt.current()));
+    }
     m_node = 0;
 }
 
index 2de5c299050b22a8b5cb5d21999ca1052e1065b7..bf00532c594eee0ccc64ae6420cfbdf886aad275 100644 (file)
@@ -492,18 +492,21 @@ void EditCommand::doReapply()
 
 void EditCommand::setStartingSelection(const Selection &s)
 {
-    for (EditCommand *cmd = this; cmd; cmd = cmd->m_parent.get())
+    for (EditCommand *cmd = this; cmd; cmd = cmd->m_parent)
         cmd->m_startingSelection = s;
 }
 
 void EditCommand::setEndingSelection(const Selection &s)
 {
-    for (EditCommand *cmd = this; cmd; cmd = cmd->m_parent.get())
+    for (EditCommand *cmd = this; cmd; cmd = cmd->m_parent)
         cmd->m_endingSelection = s;
 }
 
 void EditCommand::assignTypingStyle(CSSStyleDeclarationImpl *style)
 {
+    if (m_typingStyle == style)
+        return;
+        
     CSSStyleDeclarationImpl *old = m_typingStyle;
     m_typingStyle = style;
     if (m_typingStyle)
@@ -516,7 +519,7 @@ void EditCommand::setTypingStyle(CSSStyleDeclarationImpl *style)
 {
     // FIXME: Improve typing style.
     // See this bug: <rdar://problem/3769899> Implementation of typing style needs improvement
-    for (EditCommand *cmd = this; cmd; cmd = cmd->m_parent.get())
+    for (EditCommand *cmd = this; cmd; cmd = cmd->m_parent)
         cmd->assignTypingStyle(style);
 }
 
index 574a1aa838b08ff46bcf0c5b69cd6e7d0a7917d8..32ea47f1f0195040b496756b0eb32b2bd09f9477 100644 (file)
@@ -120,8 +120,8 @@ public:
     EditCommand(DOM::DocumentImpl *);
     virtual ~EditCommand();
 
-    bool isCompositeStep() const { return m_parent.notNull(); }
-    EditCommand *parent() const { return m_parent.get(); }
+    bool isCompositeStep() const { return m_parent != 0; }
+    EditCommand *parent() const { return m_parent; }
     void setParent(EditCommand *parent) { m_parent = parent; }
 
     enum ECommandState { NotApplied, Applied };
@@ -163,7 +163,7 @@ private:
     khtml::Selection m_startingSelection;
     khtml::Selection m_endingSelection;
     DOM::CSSStyleDeclarationImpl *m_typingStyle;
-    EditCommandPtr m_parent;
+    EditCommand *m_parent;
 };
 
 //------------------------------------------------------------------------------------------