vclptr: create Instance helpers, and set initial ref-count to 1.

Document that in README.lifecycle; the problem is that our constructors
currently take and release references left/right on the object being
created, which ... means we need an initial reference.

Change-Id: I5de952b73ac67888c3fbb150d4a7cde2a7bc9abf

13 files changed, 101 insertions, 27 deletions

diff --git a/include/rtl/ref.hxx b/include/rtl/ref.hxx
index 942923ad6819..097584b1a276 100644
--- a/include/rtl/ref.hxx
+++ b/include/rtl/ref.hxx
@@ -49,6 +49,13 @@ public:
 
     /** Constructor...
      */
+    inline Reference (reference_type * pBody, __sal_NoAcquire)
+        : m_pBody (pBody)
+    {
+    }
+
+    /** Constructor...
+     */
     inline Reference (reference_type * pBody)
         : m_pBody (pBody)
     {
@@ -56,7 +63,6 @@ public:
             m_pBody->acquire();
     }
 
-
     /** Copy constructor...
      */
     inline Reference (const Reference<reference_type> & handle)
diff --git a/include/vcl/vclptr.hxx b/include/vcl/vclptr.hxx
index 6ab6373eaf70..0c29f11d8e2d 100644
--- a/include/vcl/vclptr.hxx
+++ b/include/vcl/vclptr.hxx
@@ -22,6 +22,7 @@
 
 #include <rtl/ref.hxx>
 #include <cstddef>
+#include <utility>
 
 /// @cond INTERNAL
 namespace vcl { namespace detail {
@@ -90,14 +91,12 @@ public:
         : m_rInnerRef()
     {}
 
-
     /** Constructor...
      */
     inline VclPtr (reference_type * pBody)
         : m_rInnerRef(pBody)
     {}
 
-
     /** Copy constructor...
      */
     inline VclPtr (const VclPtr<reference_type> & handle)
@@ -216,8 +215,31 @@ public:
     {
         return (m_rInnerRef > handle.m_rInnerRef);
     }
+
+protected:
+    inline VclPtr (reference_type * pBody, __sal_NoAcquire)
+        : m_rInnerRef(pBody, SAL_NO_ACQUIRE)
+    {}
 }; // class VclPtr
 
+/**
+ * A construction helper for VclPtr. Since VclPtr types are created
+ * with a reference-count of one - to help fit into the existing
+ * code-flow; this helps us to construct them easily.
+ *
+ * For more details on the design please see vcl/README.lifecycle
+ *
+ * @param reference_type must be a subclass of vcl::Window
+ */
+template <class reference_type>
+class VclPtrInstance : public VclPtr<reference_type>
+{
+public:
+    template<typename... Arg> VclPtrInstance(Arg &&... arg)
+        : VclPtr<reference_type>( new reference_type(std::forward<Arg>(arg)...), SAL_NO_ACQUIRE )
+    {
+    }
+};
 
 template <class reference_type>
 class ScopedVclPtr : public VclPtr<reference_type>
@@ -277,6 +299,30 @@ private:
     ScopedVclPtr (const ScopedVclPtr<reference_type> &) SAL_DELETED_FUNCTION;
     // And certainly we don't want a default assignment operator.
     ScopedVclPtr<reference_type>& SAL_CALL operator= (const ScopedVclPtr<reference_type> &) SAL_DELETED_FUNCTION;
+
+protected:
+    inline ScopedVclPtr (reference_type * pBody, __sal_NoAcquire)
+        : VclPtr<reference_type>(pBody, SAL_NO_ACQUIRE)
+    {}
+};
+
+/**
+ * A construction helper for ScopedVclPtr. Since VclPtr types are created
+ * with a reference-count of one - to help fit into the existing
+ * code-flow; this helps us to construct them easily.
+ *
+ * For more details on the design please see vcl/README.lifecycle
+ *
+ * @param reference_type must be a subclass of vcl::Window
+ */
+template <class reference_type>
+class ScopedVclPtrInstance : public ScopedVclPtr<reference_type>
+{
+public:
+    template<typename... Arg> ScopedVclPtrInstance(Arg &&... arg)
+        : ScopedVclPtr<reference_type>( new reference_type(std::forward<Arg>(arg)...), SAL_NO_ACQUIRE )
+    {
+    }
 };
 
 #endif // INCLUDED_VCL_PTR_HXX
diff --git a/vcl/README.lifecycle b/vcl/README.lifecycle
index 26e7a348cc39..6a9e080c176a 100644
--- a/vcl/README.lifecycle
+++ b/vcl/README.lifecycle
@@ -45,8 +45,9 @@ to lingering pointers to freed objects.
 	to reduce code-thrash. VclPtr is used to wrap all OutputDevice
 	derived classes thus:
 
-	VclPtr<Dialog> pDialog( new Dialog( ... ) );
-	// gotcha - this is not a good idea ...
+	VclPtr<Dialog> pDialog( new Dialog( ... ), SAL_NO_ACQUIRE );
+	...
+	pDialog.disposeAndClear();
 
 	However - while the VclPtr reference count controls the
 	lifecycle of the Dialog object, it is necessary to be able to
@@ -84,30 +85,47 @@ to lingering pointers to freed objects.
 	Luckily it is easy to avoid that with a ScopedVclPtr which
 	does this for you when it goes out of scope.
 
+** One extra gotcha - an initial reference-count of 1
+
+	In the normal world of love and sanity, eg. creating UNO
+	objects, the objects start with a ref-count of zero. Thus
+	the first reference is always taken after construction by
+	the surrounding smart pointer.
+
+	Unfortunately, the existing VCL code is somewhat tortured,
+	and does a lot of reference and de-reference action on the
+	class -during- construction. This forces us to construct with
+	a reference of 1 - and to hand that into the initial smart
+	pointer with a SAL_NO_ACQUIRE.
+
+	To make this easier, we have 'Instance' template wrappers
+	that make this apparently easier, by constructing the
+	pointer for you.
+
 ** How does my familiar code change ?
 
 	Lets tweak the exemplary code above to fit the new model:
 
--	Dialog aDialog(...);
+-	Dialog aDialog(... dialog params ... );
 -	aDialog.Execute(...);
-+	ScopedVclPtr<Dialog> pDialog(new Dialog(...));
++	ScopedVclPtrInstance<Dialog> pDialog(... dialog params ... );
 +	pDialog->Execute(...); // VclPtr behaves much like a pointer
 
 or:
--	Dialog *pDialog = new Dialog(...);
-+	VclPtr<Dialog> pDialog(newDialog(...));
+-	Dialog *pDialog = new Dialog(... dialog params ...);
++	VclPtrInstance<Dialog> pDialog(... dialog params ...);
 	pDialog->Execute(...);
 -	delete pDialog;
 +	pDialog.disposeAndClear(); // done manually - replaces a delete
 or:
--	boost::shared_ptr<Dialog> xDialog(new pDialog());
-+	ScopedVclPtr<Dialog> xDialog(new Dialog(...));
+-	boost::shared_ptr<Dialog> xDialog(new Dialog(...));
++	ScopedVclPtrInstance<Dialog> xDialog(...);
 	xDialog->Execute(...);
 +	// depending how shared_ptr was shared perhaps
 +	// someone else gets a VclPtr to xDialog
 or:
 -	VirtualDevice aDev;
-+	ScopedVclPtr<VirtualDevice> pDev(new VirtualDevice());
++	ScopedVclPtrInstance<VirtualDevice> pDev();
 
 ** Why are these 'disposeOnce' calls in destructors ?
 
diff --git a/vcl/qa/cppunit/lifecycle.cxx b/vcl/qa/cppunit/lifecycle.cxx
index 12b0a5146631..7c1b1c220f48 100644
--- a/vcl/qa/cppunit/lifecycle.cxx
+++ b/vcl/qa/cppunit/lifecycle.cxx
@@ -59,7 +59,11 @@ void LifecycleTest::testCast()
 
 void LifecycleTest::testVirtualDevice()
 {
-    VclPtr<VirtualDevice> pVDev = new VirtualDevice();
+    VclPtr<VirtualDevice> pVDev( new VirtualDevice() );
+    ScopedVclPtr<VirtualDevice> pVDev2( new VirtualDevice() );
+    VclPtrInstance<VirtualDevice> pVDev3;
+    VclPtrInstance<VirtualDevice> pVDev4( 1 );
+    CPPUNIT_ASSERT(!!pVDev && !!pVDev2 && !!pVDev3 && !!pVDev4);
 }
 
 void LifecycleTest::testMultiDispose()
@@ -99,8 +103,8 @@ void LifecycleTest::testIsolatedWidgets()
 
 void LifecycleTest::testParentedWidgets()
 {
-    VclPtr<WorkWindow> xWin(new WorkWindow((vcl::Window *)NULL,
-                                                 WB_APP|WB_STDWORK));
+    ScopedVclPtrInstance<WorkWindow> xWin(new WorkWindow((vcl::Window *)NULL,
+                                                         WB_APP|WB_STDWORK));
     CPPUNIT_ASSERT(xWin.get() != NULL);
     xWin->Show();
     testWidgets(xWin);
@@ -129,7 +133,7 @@ void LifecycleTest::testChildDispose()
 
 void LifecycleTest::testPostDispose()
 {
-    VclPtr<WorkWindow> xWin(new WorkWindow((vcl::Window *)NULL, WB_STDWORK));
+    VclPtrInstance<WorkWindow> xWin((vcl::Window *)NULL, WB_STDWORK);
     xWin->disposeOnce();
 
     // check selected methods continue to work post-dispose
diff --git a/vcl/source/outdev/outdev.cxx b/vcl/source/outdev/outdev.cxx
index beee1b7fb63f..e42c8c50bed5 100644
--- a/vcl/source/outdev/outdev.cxx
+++ b/vcl/source/outdev/outdev.cxx
@@ -82,7 +82,7 @@ namespace {
 // Begin initializer and accessor public functions
 
 OutputDevice::OutputDevice() :
-    mnRefCnt(0),
+    mnRefCnt(1), // cf. VclPtrInstance and README.lifecycle
     maRegion(true),
     maFillColor( COL_WHITE ),
     maTextLineColor( COL_TRANSPARENT ),
diff --git a/vcl/source/window/dialog.cxx b/vcl/source/window/dialog.cxx
index 9ca9c30a1b40..e59ba305a80a 100644
--- a/vcl/source/window/dialog.cxx
+++ b/vcl/source/window/dialog.cxx
@@ -1065,7 +1065,7 @@ void Dialog::GrabFocusToFirstControl()
 
 void Dialog::GetDrawWindowBorder( sal_Int32& rLeftBorder, sal_Int32& rTopBorder, sal_Int32& rRightBorder, sal_Int32& rBottomBorder ) const
 {
-    ScopedVclPtr<ImplBorderWindow> aImplWin( new ImplBorderWindow( (vcl::Window*)this, WB_BORDER|WB_STDWORK, BORDERWINDOW_STYLE_OVERLAP ) );
+    ScopedVclPtrInstance<ImplBorderWindow> aImplWin( (vcl::Window*)this, WB_BORDER|WB_STDWORK, BORDERWINDOW_STYLE_OVERLAP );
     aImplWin->GetBorder( rLeftBorder, rTopBorder, rRightBorder, rBottomBorder );
 }
 
diff --git a/vcl/source/window/menubarwindow.cxx b/vcl/source/window/menubarwindow.cxx
index 0f49c51e78b0..7c0bca777f06 100644
--- a/vcl/source/window/menubarwindow.cxx
+++ b/vcl/source/window/menubarwindow.cxx
@@ -60,7 +60,7 @@ void DecoToolBox::DataChanged( const DataChangedEvent& rDCEvt )
 
 void DecoToolBox::calcMinSize()
 {
-    ScopedVclPtr<ToolBox> aTbx( new ToolBox(GetParent()) );
+    ScopedVclPtrInstance<ToolBox> aTbx( GetParent() );
     if( GetItemCount() == 0 )
     {
         ResMgr* pResMgr = ImplGetResMgr();
diff --git a/vcl/source/window/paint.cxx b/vcl/source/window/paint.cxx
index 0f05ded46af0..1dd9b3a50e98 100644
--- a/vcl/source/window/paint.cxx
+++ b/vcl/source/window/paint.cxx
@@ -1094,7 +1094,7 @@ void Window::ImplPaintToDevice( OutputDevice* i_pTargetOutDev, const Point& i_rP
     mpWindowImpl->mbReallyVisible = bRVisible;
 
     // paint metafile to VDev
-    VclPtr<VirtualDevice> pMaskedDevice = new VirtualDevice( *i_pTargetOutDev, 0, 0 );
+    VclPtrInstance<VirtualDevice> pMaskedDevice( *i_pTargetOutDev, 0, 0 );
     pMaskedDevice->SetOutputSizePixel( GetOutputSizePixel() );
     pMaskedDevice->EnableRTL( IsRTLEnabled() );
     aMtf.WindStart();
diff --git a/vcl/source/window/printdlg.cxx b/vcl/source/window/printdlg.cxx
index 4ab06b61049c..f173ad36915d 100644
--- a/vcl/source/window/printdlg.cxx
+++ b/vcl/source/window/printdlg.cxx
@@ -624,13 +624,13 @@ PrintDialog::PrintDialog( vcl::Window* i_pParent, const std::shared_ptr<PrinterC
         if( maJobPage.mpPrinters->GetEntryPos( aValue ) != LISTBOX_ENTRY_NOTFOUND )
         {
             maJobPage.mpPrinters->SelectEntry( aValue );
-            maPController->setPrinter( VclPtr<Printer>( new Printer( aValue ) ) );
+            maPController->setPrinter( VclPtrInstance<Printer>( aValue ) );
         }
         else
         {
             // fall back to default printer
             maJobPage.mpPrinters->SelectEntry( Printer::GetDefaultPrinterName() );
-            maPController->setPrinter( VclPtr<Printer>( new Printer( Printer::GetDefaultPrinterName() ) ) );
+            maPController->setPrinter( VclPtrInstance<Printer>( Printer::GetDefaultPrinterName() ) );
         }
     }
     // not printing to file
@@ -1521,7 +1521,7 @@ IMPL_LINK( PrintDialog, SelectHdl, ListBox*, pBox )
     {
         OUString aNewPrinter( pBox->GetSelectEntry() );
         // set new printer
-        maPController->setPrinter( VclPtr<Printer>( new Printer( aNewPrinter ) ) );
+        maPController->setPrinter( VclPtrInstance<Printer>( aNewPrinter ) );
         maPController->resetPrinterOptions( maOptionsPage.mpToFileBox->IsChecked() );
         // update text fields
         updatePrinterText();
diff --git a/vcl/source/window/toolbox.cxx b/vcl/source/window/toolbox.cxx
index 58ec9241039a..b434061fb274 100644
--- a/vcl/source/window/toolbox.cxx
+++ b/vcl/source/window/toolbox.cxx
@@ -4833,7 +4833,7 @@ Size ToolBox::CalcMinimumWindowSizePixel() const
     else
     {
         // create dummy toolbox for measurements
-        VclPtr< ToolBox > pToolBox = new ToolBox( GetParent(), GetStyle() );
+        VclPtrInstance< ToolBox > pToolBox( GetParent(), GetStyle() );
 
         // copy until first useful item
         std::vector< ImplToolItem >::iterator it = mpData->m_aItems.begin();
diff --git a/vcl/unx/generic/printer/cupsmgr.cxx b/vcl/unx/generic/printer/cupsmgr.cxx
index daddd1ac286f..d51f752463df 100644
--- a/vcl/unx/generic/printer/cupsmgr.cxx
+++ b/vcl/unx/generic/printer/cupsmgr.cxx
@@ -966,7 +966,7 @@ namespace
     {
         bool bRet = false;
 
-        ScopedVclPtr<RTSPWDialog> aDialog(new RTSPWDialog(rServer, rUserName, NULL));
+        ScopedVclPtrInstance<RTSPWDialog> aDialog(rServer, rUserName, nullptr);
         if (aDialog->Execute())
         {
             rUserName = aDialog->getUserName();
diff --git a/vcl/unx/x11/x11sys.cxx b/vcl/unx/x11/x11sys.cxx
index 8cad695b5265..6723c63b8047 100644
--- a/vcl/unx/x11/x11sys.cxx
+++ b/vcl/unx/x11/x11sys.cxx
@@ -134,7 +134,7 @@ int X11SalSystem::ShowNativeDialog( const OUString& rTitle, const OUString& rMes
     if( pSVData->mpIntroWindow )
         pSVData->mpIntroWindow->Hide();
 
-    ScopedVclPtr<WarningBox> aWarn(new WarningBox(NULL, WB_STDWORK, rMessage) );
+    ScopedVclPtrInstance<WarningBox> aWarn(nullptr, WB_STDWORK, rMessage);
     aWarn->SetText( rTitle );
     aWarn->Clear();
 
diff --git a/vcl/workben/outdevgrind.cxx b/vcl/workben/outdevgrind.cxx
index d53074847f90..da14e1aac673 100644
--- a/vcl/workben/outdevgrind.cxx
+++ b/vcl/workben/outdevgrind.cxx
@@ -898,7 +898,7 @@ sal_uInt16 GrindApp::Exception( sal_uInt16 nError )
 
 int GrindApp::Main()
 {
-    ScopedVclPtr<TestWindow> aWindow(new TestWindow);
+    ScopedVclPtrInstance<TestWindow> aWindow;
     aWindow->Execute();
     return 0;
 }