simplify "no delete" logic in SvRefBase

by converting the bit munging to use bitfields.
Remove unused return values.
Add asserts to check that AddRef() is not called after the object
is deleted.
Fix the code in SfxObjectShell to not call AddRef() after
SfxObjectShell is deleted.

Change-Id: I3a3565a0bc45fc9d1d086222265ab8b8175818a7

4 files changed, 31 insertions, 19 deletions

diff --git a/include/sfx2/objsh.hxx b/include/sfx2/objsh.hxx
index 1cb8e910af05..aa767ffa5931 100644
--- a/include/sfx2/objsh.hxx
+++ b/include/sfx2/objsh.hxx
@@ -177,6 +177,7 @@ private:
                                                   // sal_False := new object
     bool                        bIsInGenerateThumbnail; //optimize thumbnail generate and store procedure to improve odt saving performance, i120030
 
+    bool                        CloseInternal();
 private:
     SAL_DLLPRIVATE void UpdateTime_Impl(const ::com::sun::star::uno::Reference<
         ::com::sun::star::document::XDocumentProperties> & i_xDocProps);
diff --git a/include/tools/ref.hxx b/include/tools/ref.hxx
index ed34f4137666..3b8b320c5fe0 100644
--- a/include/tools/ref.hxx
+++ b/include/tools/ref.hxx
@@ -146,46 +146,47 @@ public:
 /** Classes that want to be referenced-counted via SvRef<T>, should extend this base class */
 class TOOLS_DLLPUBLIC SvRefBase
 {
-    static const sal_uIntPtr SV_NO_DELETE_REFCOUNT = 0x80000000;
-    sal_uIntPtr nRefCount;
+    // the only reason this is not bool is because MSVC cannot handle mixed type bitfields
+    unsigned int bNoDelete : 1;
+    unsigned int nRefCount : 31;
 
 protected:
     virtual         ~SvRefBase();
     virtual void    QueryDelete();
 
 public:
-                    SvRefBase()
-                    { nRefCount = SV_NO_DELETE_REFCOUNT; }
+                    SvRefBase() : bNoDelete(1), nRefCount(0) {}
 
-                    SvRefBase( const SvRefBase & /* rObj */ )
-                    { nRefCount = SV_NO_DELETE_REFCOUNT; }
+                    SvRefBase( const SvRefBase & /* rObj */ )  : bNoDelete(1), nRefCount(0) {}
 
     SvRefBase &     operator = ( const SvRefBase & )
                     { return *this; }
 
     void            RestoreNoDelete()
+                    { bNoDelete = 1; }
+
+    void            AddNextRef()
                     {
-                        if( nRefCount < SV_NO_DELETE_REFCOUNT )
-                            nRefCount += SV_NO_DELETE_REFCOUNT;
+                        assert( nRefCount < (1 << 30) && "Do not add refs to dead objects" );
+                        ++nRefCount;
                     }
 
-    sal_uIntPtr     AddNextRef()
-                    { return ++nRefCount; }
-
-    sal_uIntPtr     AddRef()
+    void            AddRef()
                     {
-                        if( nRefCount >= SV_NO_DELETE_REFCOUNT )
-                            nRefCount -= SV_NO_DELETE_REFCOUNT;
-                        return ++nRefCount;
+                        assert( nRefCount < (1 << 30) && "Do not add refs to dead objects" );
+                        if( bNoDelete )
+                            bNoDelete = 0;
+                        ++nRefCount;
                     }
 
     void            ReleaseRef()
                     {
-                        if( !--nRefCount )
+                        assert( nRefCount >= 1);
+                        if( --nRefCount == 0 && !bNoDelete)
                             QueryDelete();
                     }
 
-    sal_uIntPtr     GetRefCount() const
+    unsigned int    GetRefCount() const
                     { return nRefCount; }
 };
 
diff --git a/sfx2/source/doc/objxtor.cxx b/sfx2/source/doc/objxtor.cxx
index a3745783c1db..409ce93bb900 100644
--- a/sfx2/source/doc/objxtor.cxx
+++ b/sfx2/source/doc/objxtor.cxx
@@ -354,7 +354,7 @@ SfxObjectShell::~SfxObjectShell()
 
     // Never call GetInPlaceObject(), the access to the derivative branch
     // SfxInternObject is not allowed because of a compiler bug
-    SfxObjectShell::Close();
+    SfxObjectShell::CloseInternal();
     pImp->pBaseModel.set( NULL );
 
     DELETEX(AutoReloadTimer_Impl, pImp->pReloadTimer );
@@ -438,6 +438,12 @@ void SfxObjectShell::ViewAssigned()
 bool SfxObjectShell::Close()
 {
     SfxObjectShellRef aRef(this);
+    return CloseInternal();
+}
+
+// variant that does not take a reference to itself, so we can call it during object destruction
+bool SfxObjectShell::CloseInternal()
+{
     if ( !pImp->bClosing )
     {
         // Do not close if a progress is still running
diff --git a/tools/source/ref/ref.cxx b/tools/source/ref/ref.cxx
index 881a25aaf528..3a6aa59f1743 100644
--- a/tools/source/ref/ref.cxx
+++ b/tools/source/ref/ref.cxx
@@ -25,7 +25,11 @@ SvRefBase::~SvRefBase()
 
 void SvRefBase::QueryDelete()
 {
-    nRefCount = SV_NO_DELETE_REFCOUNT / 2;
+    bNoDelete = 0;
+    // I'm not sure about the original purpose of this line, but right now
+    // it serves the purpose that anything that attempts to do an AddRef()
+    // after an object is deleted will trip an assert.
+    nRefCount = 1 << 30;
     delete this;
 }