Drop ComPtr and use sal::systools::COMReference

Change-Id: I9eb6d99d883b44943ad69c2c28d4e55716dc34f9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/111673
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

11 files changed, 137 insertions, 320 deletions

diff --git a/fpicker/source/win32/IVistaFilePickerInternalNotify.hxx b/fpicker/source/win32/IVistaFilePickerInternalNotify.hxx
index f39d5aff4560..42f00c1960a4 100644
--- a/fpicker/source/win32/IVistaFilePickerInternalNotify.hxx
+++ b/fpicker/source/win32/IVistaFilePickerInternalNotify.hxx
@@ -17,22 +17,14 @@
  *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
  */
 
-#ifndef INCLUDED_FPICKER_SOURCE_WIN32_FILEPICKER_IVISTAFILEPICKERINTERNALNOTIFY_HXX
-#define INCLUDED_FPICKER_SOURCE_WIN32_FILEPICKER_IVISTAFILEPICKERINTERNALNOTIFY_HXX
+#pragma once
 
-#include "vistatypes.h"
+#include <systools/win32/uwinapi.h>
 
-#include <cppuhelper/basemutex.hxx>
-#include <osl/interlck.h>
+namespace fpicker::win32::vista{
 
-#include <shobjidl.h>
 
-namespace fpicker{
-namespace win32{
-namespace vista{
-
-
-// types, const etcpp.
+// types, const etc.
 
 
 /** todo document me
@@ -51,8 +43,6 @@ class IVistaFilePickerInternalNotify
         ~IVistaFilePickerInternalNotify() {}
 };
 
-}}}
-
-#endif // INCLUDED_FPICKER_SOURCE_WIN32_FILEPICKER_IVISTAFILEPICKERINTERNALNOTIFY_HXX
+}
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/fpicker/source/win32/VistaFilePickerEventHandler.cxx b/fpicker/source/win32/VistaFilePickerEventHandler.cxx
index 1e148bb3e1db..c4a231164ef8 100644
--- a/fpicker/source/win32/VistaFilePickerEventHandler.cxx
+++ b/fpicker/source/win32/VistaFilePickerEventHandler.cxx
@@ -229,7 +229,7 @@ void VistaFilePickerEventHandler::stopListening()
     if (m_pDialog.is())
     {
         m_pDialog->Unadvise(m_nListenerHandle);
-        m_pDialog.release();
+        m_pDialog.clear();
     }
 }
 
diff --git a/fpicker/source/win32/VistaFilePickerImpl.cxx b/fpicker/source/win32/VistaFilePickerImpl.cxx
index b0e733be88bc..2fef710b728a 100644
--- a/fpicker/source/win32/VistaFilePickerImpl.cxx
+++ b/fpicker/source/win32/VistaFilePickerImpl.cxx
@@ -27,7 +27,6 @@
 #include <com/sun/star/awt/XSystemDependentWindowPeer.hpp>
 #include <com/sun/star/lang/SystemDependent.hpp>
 #include <comphelper/sequence.hxx>
-#include <comphelper/windowserrorstring.hxx>
 #include <fpicker/strings.hrc>
 #include <fpsofficeResMgr.hxx>
 #include <osl/file.hxx>
@@ -56,7 +55,8 @@ static HWND choose_parent_window()
 
 namespace {
 
-bool createFolderItem(OUString const & url, ComPtr<IShellItem> & folder) {
+bool createFolderItem(OUString const& url, sal::systools::COMReference<IShellItem>& folder)
+{
     OUString path;
     if (osl::FileBase::getSystemPathFromFileURL(url, path)
         != osl::FileBase::E_None)
@@ -97,54 +97,53 @@ const GUID CLIENTID_FILEOPEN_LINK            = {0x39AC4BAE, 0x7D2D, 0x46BC, 0xBE
 class TDialogImplBase
 {
 public:
-    template <class ComPtrDialog> TDialogImplBase(ComPtrDialog&& iDialog)
+    TDialogImplBase(IFileDialog* iDialog)
+        : m_iDialog(iDialog)
     {
-        HRESULT hResult = iDialog.create();
-        if (FAILED(hResult))
-            throw css::uno::RuntimeException(WindowsErrorStringFromHRESULT(hResult));
-        hResult = iDialog.query(&m_iDialog);
-        if (FAILED(hResult))
-            throw css::uno::RuntimeException(WindowsErrorStringFromHRESULT(hResult));
     }
 
-    TFileDialog getComPtr() { return m_iDialog; };
-    virtual ComPtr<IShellItemArray> getResult(bool bInExecute)
+    TFileDialog getComPtr() { return m_iDialog; }
+    virtual sal::systools::COMReference<IShellItemArray> getResult(bool bInExecute)
     {
-        ComPtr<IShellItem> iItem;
+        sal::systools::COMReference<IShellItem> iItem;
         if (bInExecute)
             m_iDialog->GetCurrentSelection(&iItem);
         else
             m_iDialog->GetResult(&iItem);
         void* iItems = nullptr;
-        SHCreateShellItemArrayFromShellItem(iItem, IID_IShellItemArray, &iItems);
-        return ComPtr<IShellItemArray>(reinterpret_cast<IShellItemArray*>(iItems));
+        if (iItem.is())
+            SHCreateShellItemArrayFromShellItem(iItem.get(), IID_IShellItemArray, &iItems);
+        return static_cast<IShellItemArray*>(iItems);
     }
 
 private:
     TFileDialog m_iDialog;
 };
 
-template <class ComPtrDialog> class TDialogImpl : public TDialogImplBase
+template <class ComPtrDialog, REFCLSID CLSID> class TDialogImpl : public TDialogImplBase
 {
 public:
-    TDialogImpl() : TDialogImplBase(ComPtrDialog()) {}
+    TDialogImpl()
+        : TDialogImplBase(ComPtrDialog().CoCreateInstance(CLSID).get())
+    {
+    }
 };
 
-class TOpenDialogImpl : public TDialogImpl<TFileOpenDialog>
+class TOpenDialogImpl : public TDialogImpl<TFileOpenDialog, CLSID_FileOpenDialog>
 {
 public:
-    ComPtr<IShellItemArray> getResult(bool bInExecute) override
+    sal::systools::COMReference<IShellItemArray> getResult(bool bInExecute) override
     {
-        ComPtr<IShellItemArray> iItems;
-        TFileOpenDialog iDialog{ getComPtr() };
+        sal::systools::COMReference<IShellItemArray> iItems;
+        TFileOpenDialog iDialog(getComPtr(), sal::systools::COM_QUERY_THROW);
         if (FAILED(bInExecute ? iDialog->GetSelectedItems(&iItems) : iDialog->GetResults(&iItems)))
             iItems = TDialogImplBase::getResult(bInExecute);
         return iItems;
     }
 };
 
-using TSaveDialogImpl = TDialogImpl<TFileSaveDialog>;
-using TFolderPickerDialogImpl = TDialogImpl<TFolderPickerDialog>;
+using TSaveDialogImpl = TDialogImpl<TFileSaveDialog, CLSID_FileSaveDialog>;
+using TFolderPickerDialogImpl = TDialogImpl<TFileOpenDialog, CLSID_FileOpenDialog>;
 
 
 static OUString lcl_getURLFromShellItem (IShellItem* pItem)
@@ -384,9 +383,11 @@ void VistaFilePickerImpl::impl_sta_addFilePickerListener(const RequestRef& rRequ
     aLock.clear();
     // <- SYNCHRONIZED
 
-    VistaFilePickerEventHandler* pHandlerImpl = static_cast<VistaFilePickerEventHandler*>(iHandler.get());
-    if (pHandlerImpl)
+    if (iHandler.is())
+    {
+        auto* pHandlerImpl = static_cast<VistaFilePickerEventHandler*>(iHandler.get());
         pHandlerImpl->addFilePickerListener(xListener);
+    }
 }
 
 
@@ -403,9 +404,11 @@ void VistaFilePickerImpl::impl_sta_removeFilePickerListener(const RequestRef& rR
     aLock.clear();
     // <- SYNCHRONIZED
 
-    VistaFilePickerEventHandler* pHandlerImpl = static_cast<VistaFilePickerEventHandler*>(iHandler.get());
-    if (pHandlerImpl)
+    if (iHandler.is())
+    {
+        auto* pHandlerImpl = static_cast<VistaFilePickerEventHandler*>(iHandler.get());
         pHandlerImpl->removeFilePickerListener(xListener);
+    }
 }
 
 
@@ -534,9 +537,11 @@ void VistaFilePickerImpl::impl_sta_InitDialog(const RequestRef& rRequest, DWORD
     ::sal_Int32 nTemplate = rRequest->getArgumentOrDefault(PROP_TEMPLATE_DESCR, ::sal_Int32(0));
     impl_sta_enableFeatures(nFeatures, nTemplate);
 
-    VistaFilePickerEventHandler* pHandlerImpl = static_cast<VistaFilePickerEventHandler*>(iHandler.get());
-    if (pHandlerImpl)
+    if (iHandler.is())
+    {
+        auto* pHandlerImpl = static_cast<VistaFilePickerEventHandler*>(iHandler.get());
         pHandlerImpl->startListening(iDialog);
+    }
 }
 
 
@@ -794,21 +799,21 @@ void VistaFilePickerImpl::impl_sta_SetDirectory(const RequestRef& rRequest)
     aLock.clear();
     // <- SYNCHRONIZED
 
-    ComPtr< IShellItem > pFolder;
+    sal::systools::COMReference<IShellItem> pFolder;
     if ( !createFolderItem(sDirectory, pFolder) )
         return;
 
-    iDialog->SetFolder(pFolder);
+    iDialog->SetFolder(pFolder.get());
 }
 
 OUString VistaFilePickerImpl::GetDirectory()
 {
     TFileDialog iDialog = impl_getBaseDialogInterface();
-    ComPtr< IShellItem > pFolder;
+    sal::systools::COMReference<IShellItem> pFolder;
     HRESULT hResult = iDialog->GetFolder( &pFolder );
     if ( FAILED(hResult) )
         return OUString();
-    return lcl_getURLFromShellItem(pFolder);
+    return lcl_getURLFromShellItem(pFolder.get());
 }
 
 void VistaFilePickerImpl::impl_sta_GetDirectory(const RequestRef& rRequest)
@@ -902,8 +907,8 @@ void VistaFilePickerImpl::impl_sta_getSelectedFiles(const RequestRef& rRequest)
 
     // ask dialog for results
     // we must react different if dialog is in execute or not .-(
-    ComPtr<IShellItemArray> iItems = pDialog->getResult(bInExecute);
-    if (!iItems)
+    sal::systools::COMReference<IShellItemArray> iItems = pDialog->getResult(bInExecute);
+    if (!iItems.is())
         return;
 
     // convert and pack results
@@ -912,9 +917,10 @@ void VistaFilePickerImpl::impl_sta_getSelectedFiles(const RequestRef& rRequest)
     {
         for (DWORD i = 0; i < nCount; ++i)
         {
-            if (ComPtr<IShellItem> iItem; SUCCEEDED(iItems->GetItemAt(i, &iItem)))
+            if (sal::systools::COMReference<IShellItem> iItem;
+                SUCCEEDED(iItems->GetItemAt(i, &iItem)))
             {
-                if (const OUString sURL = lcl_getURLFromShellItem(iItem); !sURL.isEmpty())
+                if (const OUString sURL = lcl_getURLFromShellItem(iItem.get()); !sURL.isEmpty())
                     lFiles.push_back(sURL);
             }
         }
@@ -948,7 +954,7 @@ void VistaFilePickerImpl::impl_sta_ShowDialogModal(const RequestRef& rRequest)
     // according to its client guid.
     if( m_sDirectory.getLength())
     {
-        ComPtr< IShellItem > pFolder;
+        sal::systools::COMReference<IShellItem> pFolder;
         if ( createFolderItem(m_sDirectory, pFolder) )
         {
             if (m_sFilename.getLength())
@@ -988,14 +994,14 @@ void VistaFilePickerImpl::impl_sta_ShowDialogModal(const RequestRef& rRequest)
                 WIN32_FIND_DATAW aFindFileData;
                 HANDLE  hFind = FindFirstFileW( o3tl::toW(aSystemPath.getStr()), &aFindFileData );
                 if (hFind != INVALID_HANDLE_VALUE)
-                    iDialog->SetFolder(pFolder);
+                    iDialog->SetFolder(pFolder.get());
                 else
-                    hResult = iDialog->AddPlace(pFolder, FDAP_TOP);
+                    hResult = iDialog->AddPlace(pFolder.get(), FDAP_TOP);
 
                 FindClose( hFind );
             }
             else
-                iDialog->AddPlace(pFolder, FDAP_TOP);
+                iDialog->AddPlace(pFolder.get(), FDAP_TOP);
         }
     }
 
@@ -1044,15 +1050,13 @@ TFileDialog VistaFilePickerImpl::impl_getBaseDialogInterface()
 
 TFileDialogCustomize VistaFilePickerImpl::impl_getCustomizeInterface()
 {
-    TFileDialogCustomize iCustom;
-
     // SYNCHRONIZED->
     osl::MutexGuard aLock(m_aMutex);
 
     if (m_pDialog)
-        m_pDialog->getComPtr().query(&iCustom);
+        return { m_pDialog->getComPtr(), sal::systools::COM_QUERY_THROW };
 
-    return iCustom;
+    return {};
 }
 
 
diff --git a/fpicker/source/win32/VistaFilePickerImpl.hxx b/fpicker/source/win32/VistaFilePickerImpl.hxx
index 1353c55e869a..6e6e04ad56fc 100644
--- a/fpicker/source/win32/VistaFilePickerImpl.hxx
+++ b/fpicker/source/win32/VistaFilePickerImpl.hxx
@@ -25,7 +25,6 @@
 #include <shobjidl.h>
 
 #include "asyncrequests.hxx"
-#include "comptr.hxx"
 #include "vistatypes.h"
 #include "FilterContainer.hxx"
 #include "VistaFilePickerEventHandler.hxx"
diff --git a/fpicker/source/win32/comptr.hxx b/fpicker/source/win32/comptr.hxx
deleted file mode 100644
index 2469d8b5e289..000000000000
--- a/fpicker/source/win32/comptr.hxx
+++ /dev/null
@@ -1,212 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/*
- * This file is part of the LibreOffice project.
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/.
- *
- * This file incorporates work covered by the following license notice:
- *
- *   Licensed to the Apache Software Foundation (ASF) under one or more
- *   contributor license agreements. See the NOTICE file distributed
- *   with this work for additional information regarding copyright
- *   ownership. The ASF licenses this file to you under the Apache
- *   License, Version 2.0 (the "License"); you may not use this file
- *   except in compliance with the License. You may obtain a copy of
- *   the License at http://www.apache.org/licenses/LICENSE-2.0 .
- */
-
-#ifndef INCLUDED_FPICKER_SOURCE_WIN32_FILEPICKER_COMPTR_HXX
-#define INCLUDED_FPICKER_SOURCE_WIN32_FILEPICKER_COMPTR_HXX
-
-#include <sal/types.h>
-#include <osl/diagnose.h>
-#include <shobjidl.h>
-
-template< class    T_INTERFACE          ,
-          REFIID   P_IID   = IID_NULL   ,
-          REFCLSID P_CLSID = CLSID_NULL >
-class ComPtr
-{
-    public:
-
-
-        /** initialize com ptr with null.
-         */
-        ComPtr()
-        {
-            m_pInterface = nullptr;
-        }
-
-
-        /** initialize com ptr with given interface.
-         */
-        explicit ComPtr(T_INTERFACE* pInterface)
-        {
-            m_pInterface = pInterface;
-            if (m_pInterface)
-                m_pInterface->AddRef();
-        }
-
-
-        /** copy ctor.
-         */
-        ComPtr(const ComPtr< T_INTERFACE, P_IID, P_CLSID >& aCopy)
-        {
-            m_pInterface = aCopy.m_pInterface;
-            if (m_pInterface)
-                m_pInterface->AddRef();
-        }
-
-
-        /** initialize object by querying external object for the right interface.
-         */
-        explicit ComPtr(IUnknown* pIUnknown)
-        {
-            if (pIUnknown)
-                pIUnknown->QueryInterface(P_IID, (void**)&m_pInterface);
-        }
-
-
-        /** deinitialize com object right.
-         */
-        ~ComPtr()
-        {
-            release();
-        }
-
-    public:
-
-
-        HRESULT create()
-        {
-            return CoCreateInstance(P_CLSID, nullptr, CLSCTX_ALL, P_IID, reinterpret_cast<void**>(&m_pInterface));
-        }
-
-
-        operator T_INTERFACE*() const
-        {
-            return m_pInterface;
-        }
-
-
-        T_INTERFACE& operator*() const
-        {
-            return *m_pInterface;
-        }
-
-
-        T_INTERFACE** operator&()
-        {
-            return &m_pInterface;
-        }
-
-
-        T_INTERFACE* operator->() const
-        {
-            return m_pInterface;
-        }
-
-
-        T_INTERFACE* operator=(T_INTERFACE* pInterface)
-        {
-            if ( equals(pInterface) )
-                return m_pInterface;
-
-            m_pInterface->Release();
-            m_pInterface = pInterface;
-            if (m_pInterface)
-                m_pInterface->AddRef();
-
-            return m_pInterface;
-        }
-
-
-        T_INTERFACE* operator=(IUnknown* pIUnknown)
-        {
-            if (pIUnknown)
-                pIUnknown->QueryInterface(P_IID, (void**)&m_pInterface);
-            return m_pInterface;
-        }
-
-
-        T_INTERFACE* operator=(const ComPtr< T_INTERFACE, P_IID, P_CLSID >& aCopy)
-        {
-            m_pInterface = aCopy.m_pInterface;
-            if (m_pInterface)
-                m_pInterface->AddRef();
-
-            return m_pInterface;
-        }
-
-
-        T_INTERFACE* get() const
-        {
-            return m_pInterface;
-        }
-
-
-        void attach(T_INTERFACE* pInterface)
-        {
-            if (pInterface)
-            {
-                m_pInterface->Release();
-                m_pInterface = pInterface;
-            }
-        }
-
-
-        T_INTERFACE* detach()
-        {
-            T_INTERFACE* pInterface = m_pInterface;
-            m_pInterface = NULL;
-            return pInterface;
-        }
-
-
-        void release()
-        {
-            if (m_pInterface)
-            {
-                m_pInterface->Release();
-                m_pInterface = nullptr;
-            }
-        }
-
-        template< class T_QUERYINTERFACE >
-        HRESULT query(T_QUERYINTERFACE** pQuery)
-        {
-            return m_pInterface->QueryInterface(__uuidof(T_QUERYINTERFACE), reinterpret_cast<void**>(pQuery));
-        }
-
-        bool equals(IUnknown* pCheck)
-        {
-            if (
-                ( ! m_pInterface ) &&
-                ( ! pCheck       )
-               )
-                return sal_True;
-
-            IUnknown* pCurrent = NULL;
-            m_pInterface->QueryInterface(IID_IUnknown, (void**)&pCurrent);
-
-            sal_Bool bEquals = (pCheck == pCurrent);
-            pCurrent->Release();
-
-            return bEquals;
-        }
-
-
-        bool is()
-        {
-            return (m_pInterface != nullptr);
-        }
-
-    private:
-        T_INTERFACE* m_pInterface;
-};
-
-#endif
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/fpicker/source/win32/vistatypes.h b/fpicker/source/win32/vistatypes.h
index 4271b0cea482..5e6c62a439f3 100644
--- a/fpicker/source/win32/vistatypes.h
+++ b/fpicker/source/win32/vistatypes.h
@@ -20,8 +20,8 @@
 #ifndef INCLUDED_FPICKER_SOURCE_WIN32_FILEPICKER_VISTATYPES_H
 #define INCLUDED_FPICKER_SOURCE_WIN32_FILEPICKER_VISTATYPES_H
 
-#include "comptr.hxx"
 #include <shobjidl.h>
+#include <systools/win32/comtools.hxx>
 
 namespace fpicker{
 namespace win32{
@@ -31,12 +31,11 @@ namespace vista{
 // types, const etcpp.
 
 
-typedef ComPtr< IFileDialog         , IID_IFileDialog                             > TFileDialog;
-typedef ComPtr< IFileOpenDialog     , IID_IFileOpenDialog  , CLSID_FileOpenDialog > TFileOpenDialog;
-typedef ComPtr< IFileSaveDialog     , IID_IFileSaveDialog  , CLSID_FileSaveDialog > TFileSaveDialog;
-typedef ComPtr< IFileDialogEvents   , IID_IFileDialogEvents                       > TFileDialogEvents;
-typedef ComPtr< IFileDialogCustomize, IID_IFileDialogCustomize                    > TFileDialogCustomize;
-typedef TFileOpenDialog TFolderPickerDialog;
+typedef sal::systools::COMReference<IFileDialog> TFileDialog;
+typedef sal::systools::COMReference<IFileOpenDialog> TFileOpenDialog;
+typedef sal::systools::COMReference<IFileSaveDialog> TFileSaveDialog;
+typedef sal::systools::COMReference<IFileDialogEvents> TFileDialogEvents;
+typedef sal::systools::COMReference<IFileDialogCustomize> TFileDialogCustomize;
 
 } // namespace vista
 } // namespace win32
diff --git a/include/systools/win32/comtools.hxx b/include/systools/win32/comtools.hxx
index 9f8489b8faac..c37053c931b7 100644
--- a/include/systools/win32/comtools.hxx
+++ b/include/systools/win32/comtools.hxx
@@ -21,6 +21,7 @@
 
 #include <string>
 #include <stdexcept>
+#include <type_traits>
 #include <objbase.h>
 
 namespace sal::systools
@@ -40,6 +41,12 @@ namespace sal::systools
         HRESULT hr_;
     };
 
+    struct COM_QUERY_TAG {} constexpr COM_QUERY;
+    struct COM_QUERY_THROW_TAG {} constexpr COM_QUERY_THROW;
+    template <typename TAG>
+    constexpr bool is_COM_query_tag
+        = std::is_same_v<TAG, COM_QUERY_TAG> || std::is_same_v<TAG, COM_QUERY_THROW_TAG>;
+
     /* A simple COM smart pointer template */
     template <typename T>
     class COMReference
@@ -59,6 +66,13 @@ namespace sal::systools
             addRef();
         }
 
+        // Query from IUnknown*, using COM_QUERY or COM_QUERY_THROW tags
+        template <typename T2, typename TAG>
+        COMReference(const COMReference<T2>& p, TAG t)
+            : COMReference(p.QueryInterface<T>(t))
+        {
+        }
+
         COMReference<T>& operator=(const COMReference<T>& other)
         {
             other.addRef();
@@ -77,18 +91,31 @@ namespace sal::systools
 
         ~COMReference() { release(); }
 
-        template<typename InterfaceType>
-        COMReference<InterfaceType> QueryInterface(REFIID iid)
+        template <typename T2, typename TAG, std::enable_if_t<is_COM_query_tag<TAG>, int> = 0>
+        COMReference<T2> QueryInterface(TAG) const
         {
-            COMReference<InterfaceType> ip;
+            void* ip = nullptr;
             HRESULT hr = E_FAIL;
             if (com_ptr_)
-                hr = com_ptr_->QueryInterface(iid, reinterpret_cast<LPVOID*>(&ip));
+                hr = com_ptr_->QueryInterface(__uuidof(T2), &ip);
+
+            if constexpr (std::is_same_v<TAG, COM_QUERY_THROW_TAG>)
+                if (FAILED(hr))
+                    throw ComError("QueryInterface failed: Interface not supported!", hr);
 
+            return { static_cast<T2*>(ip), false };
+        }
+
+        COMReference<T>& CoCreateInstance(REFCLSID clsid, IUnknown* pOuter = nullptr,
+                                          DWORD nCtx = CLSCTX_ALL)
+        {
+            clear();
+            HRESULT hr = ::CoCreateInstance(clsid, pOuter, nCtx, __uuidof(T),
+                                            reinterpret_cast<void**>(&com_ptr_));
             if (FAILED(hr))
-                throw ComError("QueryInterface failed: Interface not supported!", hr);
+                throw ComError("CoCreateInstance failed!", hr);
 
-            return ip;
+            return *this;
         }
 
         T* operator->() const { return com_ptr_; }
@@ -105,11 +132,10 @@ namespace sal::systools
 
         T* get() const { return com_ptr_; }
 
-        COMReference<T>& clear()
+        void clear()
         {
             release();
             com_ptr_ = nullptr;
-            return *this;
         }
 
         bool is() const { return (com_ptr_ != nullptr); }
diff --git a/sal/CppunitTest_sal_comtools.mk b/sal/CppunitTest_sal_comtools.mk
index f26f4f8dffa1..1f9c24a0c812 100644
--- a/sal/CppunitTest_sal_comtools.mk
+++ b/sal/CppunitTest_sal_comtools.mk
@@ -17,4 +17,8 @@ $(eval $(call gb_CppunitTest_use_libraries,sal_comtools,\
     sal \
 ))
 
+$(eval $(call gb_CppunitTest_use_system_win32_libs,sal_comtools,\
+    ole32 \
+))
+
 # vim: set noet sw=4 ts=4:
diff --git a/sal/qa/systools/test_comtools.cxx b/sal/qa/systools/test_comtools.cxx
index 694a4a665b09..073bb79ec2d2 100644
--- a/sal/qa/systools/test_comtools.cxx
+++ b/sal/qa/systools/test_comtools.cxx
@@ -20,6 +20,7 @@
 #include <cppunit/extensions/HelperMacros.h>
 #include <cppunit/plugin/TestPlugIn.h>
 #include <systools/win32/comtools.hxx>
+#include <shobjidl.h>
 
 namespace {
 
@@ -183,30 +184,36 @@ namespace test_comtools
 
         void test_query_interface()
         {
-            try
-            {
-                sal::systools::COMReference<IUnknown> r1 = comObjectSource();
-                sal::systools::COMReference<IUnknown> r2 = r1.QueryInterface<IUnknown>(IID_IUnknown);
-                CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong reference count, 2 is expected", ULONG(2), reinterpret_cast<COMObject*>(r2.get())->GetRefCount());
-            }
-            catch(const sal::systools::ComError&)
-            {
-                CPPUNIT_ASSERT_MESSAGE("Exception should not have been thrown", false);
-            }
+            sal::systools::COMReference<IUnknown> r1 = comObjectSource();
+            sal::systools::COMReference<IUnknown> r2;
+            CPPUNIT_ASSERT_NO_THROW_MESSAGE(
+                "Exception should not have been thrown",
+                r2 = r1.QueryInterface<IUnknown>(sal::systools::COM_QUERY_THROW));
+            CPPUNIT_ASSERT_EQUAL_MESSAGE("Wrong reference count, 2 is expected", ULONG(2),
+                                         reinterpret_cast<COMObject*>(r2.get())->GetRefCount());
         }
 
         void test_query_interface_throw()
         {
-            try
-            {
-                sal::systools::COMReference<IUnknown> r1 = comObjectSource();
-                sal::systools::COMReference<IPersistFile> r2 = r1.QueryInterface<IPersistFile>(IID_IPersistFile);
-            }
-            catch(const sal::systools::ComError&)
-            {
+            sal::systools::COMReference<IUnknown> r1 = comObjectSource();
+            CPPUNIT_ASSERT_THROW_MESSAGE("Exception should have been thrown",
+                auto r2 = r1.QueryInterface<IPersistFile>(sal::systools::COM_QUERY_THROW),
+                sal::systools::ComError);
+        }
+
+        void test_CoCreateInstance()
+        {
+            if (FAILED(CoInitialize(nullptr)))
                 return;
+            {
+                // Use scope to destroy the reference before calling CoUninitialize
+                sal::systools::COMReference<IFileOpenDialog> r;
+                CPPUNIT_ASSERT_NO_THROW(r.CoCreateInstance(__uuidof(FileOpenDialog)));
+                // Immediately after CoCreateInstance, refcount must be 1; increasing once gives 2
+                CPPUNIT_ASSERT_EQUAL(ULONG(2), r->AddRef());
+                r->Release();
             }
-            CPPUNIT_ASSERT_MESSAGE("Exception should have been thrown", false);
+            CoUninitialize();
         }
 
         // Change the following lines only, if you add, remove or rename
@@ -227,6 +234,7 @@ namespace test_comtools
         CPPUNIT_TEST(test_clear);
         CPPUNIT_TEST(test_query_interface);
         CPPUNIT_TEST(test_query_interface_throw);
+        CPPUNIT_TEST(test_CoCreateInstance);
         CPPUNIT_TEST_SUITE_END();
     };
 
diff --git a/shell/source/win32/SysShExec.cxx b/shell/source/win32/SysShExec.cxx
index fde14e268c26..fa83180705a0 100644
--- a/shell/source/win32/SysShExec.cxx
+++ b/shell/source/win32/SysShExec.cxx
@@ -278,25 +278,27 @@ void SAL_CALL CSysShExec::execute( const OUString& aCommand, const OUString& aPa
                     break;
                 }
                 sal::systools::COMReference<IShellLinkW> link;
-                auto e2 = CoCreateInstance(
-                    CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_IShellLinkW,
-                    reinterpret_cast<LPVOID *>(&link));
-                if (FAILED(e2)) {
+                try
+                {
+                    link.CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER);
+                }
+                catch (sal::systools::ComError& e)
+                {
                     throw css::lang::IllegalArgumentException(
                         ("XSystemShellExecute.execute, CoCreateInstance failed with "
-                         + OUString::number(e2)),
+                         + OUString::number(e.GetHresult())),
                         {}, 0);
                 }
                 sal::systools::COMReference<IPersistFile> file;
                 try {
-                    file = link.QueryInterface<IPersistFile>(IID_IPersistFile);
+                    file = link.QueryInterface<IPersistFile>(sal::systools::COM_QUERY_THROW);
                 } catch(sal::systools::ComError & e3) {
                     throw css::lang::IllegalArgumentException(
                         ("XSystemShellExecute.execute, QueryInterface failed with: "
                          + o3tl::runtimeToOUString(e3.what())),
                         {}, 0);
                 }
-                e2 = file->Load(path, STGM_READ);
+                HRESULT e2 = file->Load(path, STGM_READ);
                 if (FAILED(e2)) {
                     throw css::lang::IllegalArgumentException(
                         ("XSystemShellExecute.execute, IPersistFile.Load failed with "
diff --git a/vcl/win/dtrans/FmtFilter.cxx b/vcl/win/dtrans/FmtFilter.cxx
index e9c4b42f8b68..535f6c122712 100644
--- a/vcl/win/dtrans/FmtFilter.cxx
+++ b/vcl/win/dtrans/FmtFilter.cxx
@@ -308,15 +308,12 @@ static std::wstring getShellLinkTarget(const std::wstring& aLnkFile)
     try
     {
         sal::systools::COMReference<IShellLinkW> pIShellLink;
-        HRESULT hr = CoCreateInstance(
-            CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_IShellLinkW, reinterpret_cast<LPVOID*>(&pIShellLink));
-        if (FAILED(hr))
-            return target;
+        pIShellLink.CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER);
 
-        sal::systools::COMReference<IPersistFile> pIPersistFile =
-            pIShellLink.QueryInterface<IPersistFile>(IID_IPersistFile);
+        sal::systools::COMReference<IPersistFile> pIPersistFile(pIShellLink,
+                                                                sal::systools::COM_QUERY_THROW);
 
-        hr = pIPersistFile->Load(aLnkFile.c_str(), STGM_READ);
+        HRESULT hr = pIPersistFile->Load(aLnkFile.c_str(), STGM_READ);
         if (FAILED(hr))
             return target;