From c784e87f3b53e823dfb303e296ede834accc9322 Mon Sep 17 00:00:00 2001
From: Tavmjong Bah <tavmjong@free.fr>
Date: Fri, 26 Dec 2014 15:44:07 +0100
Subject: SPStyle ref counting clean up.

(bzr r13822.1.7)
---
 src/display/drawing-item.cpp        | 10 ++++++----
 src/display/nr-filter-primitive.cpp |  8 +++++---
 src/libnrtype/Layout-TNG-Input.cpp  |  2 +-
 src/sp-object.cpp                   | 20 ++++++++++++++++----
 src/style.cpp                       |  8 ++++++--
 src/style.h                         |  5 +++--
 6 files changed, 37 insertions(+), 16 deletions(-)

(limited to 'src')

diff --git a/src/display/drawing-item.cpp b/src/display/drawing-item.cpp
index 83d4744c9..89ca66dc4 100644
--- a/src/display/drawing-item.cpp
+++ b/src/display/drawing-item.cpp
@@ -366,10 +366,12 @@ DrawingItem::setStyle(SPStyle *style, SPStyle *context_style)
     // std::cout << "DrawingItem::setStyle: " << name() << " " << style
     //           << " " << context_style << std::endl;
 
-    if (style) sp_style_ref(style);
-    if (_style) sp_style_unref(_style);
-    _style = style;
-
+    if( style != _style ) {
+        if (style) sp_style_ref(style);
+        if (_style) sp_style_unref(_style);
+        _style = style;
+    }
+    
     if (style && style->filter.set && style->getFilter()) {
         if (!_filter) {
             int primitives = sp_filter_primitive_count(SP_FILTER(style->getFilter()));
diff --git a/src/display/nr-filter-primitive.cpp b/src/display/nr-filter-primitive.cpp
index 3033118e4..c8b569036 100644
--- a/src/display/nr-filter-primitive.cpp
+++ b/src/display/nr-filter-primitive.cpp
@@ -171,9 +171,11 @@ Geom::Rect FilterPrimitive::filter_primitive_area(FilterUnits const &units)
 
 void FilterPrimitive::setStyle(SPStyle *style)
 {
-    if (style) sp_style_ref(style);
-    if (_style) sp_style_unref(_style);
-    _style = style;
+    if( style != _style ) {
+        if (style) sp_style_ref(style);
+        if (_style) sp_style_unref(_style);
+        _style = style;
+    }
 }
 
 
diff --git a/src/libnrtype/Layout-TNG-Input.cpp b/src/libnrtype/Layout-TNG-Input.cpp
index fa1e8c11b..cd9179c5f 100644
--- a/src/libnrtype/Layout-TNG-Input.cpp
+++ b/src/libnrtype/Layout-TNG-Input.cpp
@@ -325,7 +325,7 @@ PangoFontDescription *Layout::InputStreamTextSource::styleGetFontDescription() c
 
 Layout::InputStreamTextSource::~InputStreamTextSource()
 {
-    sp_style_unref(style);
+  sp_style_unref(style);
 }
 
 }//namespace Text
diff --git a/src/sp-object.cpp b/src/sp-object.cpp
index 4f213f943..059fa8093 100644
--- a/src/sp-object.cpp
+++ b/src/sp-object.cpp
@@ -146,7 +146,18 @@ SPObject::~SPObject() {
         this->_successor = NULL;
     }
 
-    delete this->style;
+    if( style == NULL ) {
+        // style pointer could be NULL if unreffed too many times.
+        // Conjecture: style pointer is never NULL.
+        std::cerr << "SPObject::~SPObject(): style pointer is NULL" << std::endl;
+    } else if( style->refCount() > 1 ) {
+        // Several classes ref style.
+        // Conjecture: style pointer should be unreffed by other classes before reaching here.
+        std::cerr << "SPObject::~SPObject(): someone else still holding ref to style" << std::endl;
+        sp_style_unref( this->style );
+    } else {
+        delete this->style;
+    }
 }
 
 // CPPIFY: make pure virtual
@@ -798,9 +809,10 @@ void SPObject::releaseReferences() {
         g_assert(!this->id);
     }
 
-    if (this->style) {
-        this->style = sp_style_unref(this->style);
-    }
+    // style belongs to SPObject, we should not need to unref here.
+    // if (this->style) {
+    //     this->style = sp_style_unref(this->style);
+    // }
 
     this->document = NULL;
     this->repr = NULL;
diff --git a/src/style.cpp b/src/style.cpp
index 1d08db4f8..0772a4bce 100644
--- a/src/style.cpp
+++ b/src/style.cpp
@@ -458,6 +458,10 @@ SPStyle::~SPStyle() {
 
     _properties.clear();
 
+    // Conjecture: all this SPStyle ref counting is not needed. SPObject creates an instance of
+    // SPStyle when it is constructed and deletes it when it is destructed. The refcount is
+    // incremented and decremented only in the files: display/drawing-item.cpp,
+    // display/nr-filter-primitive.cpp, and libnrtype/Layout-TNG-Input.cpp.
     if( _refcount > 1 ) {
         std::cerr << "SPStyle::~SPStyle: ref count greater than 1! " << _refcount << std::endl;
     }
@@ -1237,7 +1241,7 @@ sp_style_ref(SPStyle *style)
 {
     g_return_val_if_fail(style != NULL, NULL);
 
-    style->ref(); // Increase ref count
+    style->style_ref(); // Increase ref count
 
     return style;
 }
@@ -1250,7 +1254,7 @@ SPStyle *
 sp_style_unref(SPStyle *style)
 {
     g_return_val_if_fail(style != NULL, NULL);
-    if (style->unref() < 1) {
+    if (style->style_unref() < 1) {
         delete style;
         return NULL;
     }
diff --git a/src/style.h b/src/style.h
index 9d3f0716a..3f21f37db 100644
--- a/src/style.h
+++ b/src/style.h
@@ -58,8 +58,9 @@ public:
     void mergeString( char const *const p );
     bool operator==(const SPStyle& rhs);
 
-    int ref()   { ++_refcount; return _refcount; }
-    int unref() { --_refcount; return _refcount; }
+    int style_ref()   { ++_refcount; return _refcount; }
+    int style_unref() { --_refcount; return _refcount; }
+    int refCount() { return _refcount; }
 
 private:
     void _mergeString( char const *const p );
-- 
cgit v1.2.3