[Libreoffice-commits] core.git: Branch 'feature/vclref' - 2 commits - include/rtl include/vcl vcl/qa vcl/README.lifecycle vcl/source vcl/unx vcl/workben
Michael Meeks
michael.meeks at collabora.com
Mon Mar 30 09:48:57 PDT 2015
include/rtl/ref.hxx | 8 +++++
include/vcl/dockwin.hxx | 2 -
include/vcl/syswin.hxx | 2 -
include/vcl/vclptr.hxx | 50 ++++++++++++++++++++++++++++++++++--
vcl/README.lifecycle | 36 +++++++++++++++++++------
vcl/qa/cppunit/lifecycle.cxx | 12 +++++---
vcl/source/outdev/outdev.cxx | 2 -
vcl/source/window/dialog.cxx | 4 +-
vcl/source/window/menubarwindow.cxx | 2 -
vcl/source/window/paint.cxx | 2 -
vcl/source/window/printdlg.cxx | 6 ++--
vcl/source/window/toolbox.cxx | 2 -
vcl/unx/generic/printer/cupsmgr.cxx | 2 -
vcl/unx/x11/x11sys.cxx | 2 -
vcl/workben/outdevgrind.cxx | 2 -
15 files changed, 104 insertions(+), 30 deletions(-)
New commits:
commit 337dad030ca439702f8d678131b57f3c95845c95
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Mon Mar 30 17:52:43 2015 +0100
Revert part of "Unwind mpDialogParent oddness."
Now its ok to reference / un-reference ourself during construction.
Change-Id: Ib3b302719c7d40d7a42272f93db5e4c437859cad
diff --git a/include/vcl/dockwin.hxx b/include/vcl/dockwin.hxx
index dd9667e..65b78d1 100644
--- a/include/vcl/dockwin.hxx
+++ b/include/vcl/dockwin.hxx
@@ -264,7 +264,7 @@ private:
mbIsCalculatingInitialLayoutSize:1,
mbInitialLayoutDone:1;
- vcl::Window *mpDialogParent; // deliberately not a VclPtr
+ VclPtr<vcl::Window> mpDialogParent;
SAL_DLLPRIVATE void ImplInitDockingWindowData();
SAL_DLLPRIVATE void setPosSizeOnContainee(Size aSize, Window &rBox);
diff --git a/include/vcl/syswin.hxx b/include/vcl/syswin.hxx
index d30cd42..d44086f 100644
--- a/include/vcl/syswin.hxx
+++ b/include/vcl/syswin.hxx
@@ -155,7 +155,7 @@ private:
Idle maLayoutIdle;
protected:
bool mbIsDefferedInit;
- vcl::Window *mpDialogParent; // deliberately not a VclPtr
+ VclPtr<vcl::Window> mpDialogParent;
public:
using Window::ImplIsInTaskPaneList;
SAL_DLLPRIVATE bool ImplIsInTaskPaneList( vcl::Window* pWin );
diff --git a/vcl/source/window/dialog.cxx b/vcl/source/window/dialog.cxx
index 11b6499..4bca694 100644
--- a/vcl/source/window/dialog.cxx
+++ b/vcl/source/window/dialog.cxx
@@ -492,7 +492,7 @@ OUString VclBuilderContainer::getUIRootDir()
//do the init. Find the real parent stashed in mpDialogParent.
void Dialog::doDeferredInit(WinBits nBits)
{
- vcl::Window *pParent = mpDialogParent;
+ VclPtr<vcl::Window> pParent = mpDialogParent;
mpDialogParent = NULL;
ImplInit(pParent, nBits);
mbIsDefferedInit = false;
commit 69b0663ec52dd79f4c04f9fe83c545083d74431a
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Mon Mar 30 17:49:20 2015 +0100
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
diff --git a/include/rtl/ref.hxx b/include/rtl/ref.hxx
index 942923a..097584b 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 6ab6373..0c29f11 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 26e7a34..6a9e080 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 12b0a51..7c1b1c2 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 2118450..db93e11 100644
--- a/vcl/source/outdev/outdev.cxx
+++ b/vcl/source/outdev/outdev.cxx
@@ -80,7 +80,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 cf4abe8..11b6499 100644
--- a/vcl/source/window/dialog.cxx
+++ b/vcl/source/window/dialog.cxx
@@ -1110,7 +1110,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 f13e1e8..9b75a86 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 cc00d00..6d7e114 100644
--- a/vcl/source/window/paint.cxx
+++ b/vcl/source/window/paint.cxx
@@ -1087,7 +1087,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 0685413..8568856 100644
--- a/vcl/source/window/printdlg.cxx
+++ b/vcl/source/window/printdlg.cxx
@@ -632,13 +632,13 @@ PrintDialog::PrintDialog( vcl::Window* i_pParent, const boost::shared_ptr<Printe
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
@@ -1531,7 +1531,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 9772d96e..69e8a32 100644
--- a/vcl/source/window/toolbox.cxx
+++ b/vcl/source/window/toolbox.cxx
@@ -4835,7 +4835,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 3ceef18..e56f4b4 100644
--- a/vcl/unx/generic/printer/cupsmgr.cxx
+++ b/vcl/unx/generic/printer/cupsmgr.cxx
@@ -892,7 +892,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 8cad695..6723c63 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 d530748..da14e1a 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;
}
More information about the Libreoffice-commits
mailing list