Ros-diffs October 2022

ros-diffs@reactos.org

23 participants
162 discussions

[reactos] 01/02: [ATL] Prohibit the use of AddRef/Release on objects inside CComPtr

by Mark Jansen

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=a414c88daeb4ba91e0cc1… commit a414c88daeb4ba91e0cc121f7cfbf51ef2d3151a Author: Mark Jansen <mark.jansen(a)reactos.org> AuthorDate: Sun Sep 18 20:57:38 2022 +0200 Commit: Mark Jansen <mark.jansen(a)reactos.org> CommitDate: Sun Oct 2 00:38:23 2022 +0200 [ATL] Prohibit the use of AddRef/Release on objects inside CComPtr This mimics what MS's CComPtr is doing: https://learn.microsoft.com/en-us/cpp/atl/reference/ccomptrbase-class?view=… The reasoning behind this is that AddRef/Release is handled by the CComPtr, so anyone calling that is most likely not using the CComPtr correct. --- sdk/lib/atl/atlcomcli.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sdk/lib/atl/atlcomcli.h b/sdk/lib/atl/atlcomcli.h index cba78101d2d..e6effc2ffcf 100644 --- a/sdk/lib/atl/atlcomcli.h +++ b/sdk/lib/atl/atlcomcli.h @@ -59,6 +59,13 @@ inline HRESULT AtlHresultFromLastError() throw() return HRESULT_FROM_WIN32(dwError); } +template <class T> +class _NoAddRefReleaseOnCComPtr : public T +{ + private: + virtual ULONG STDMETHODCALLTYPE AddRef() = 0; + virtual ULONG STDMETHODCALLTYPE Release() = 0; +}; template<class T> class CComPtr @@ -173,10 +180,10 @@ public: return p; } - T *operator -> () + _NoAddRefReleaseOnCComPtr<T> *operator -> () const { ATLASSERT(p != NULL); - return p; + return (_NoAddRefReleaseOnCComPtr<T> *)p; } };

1 year, 11 months

[reactos] 01/01: [SHELL32] CDefView: Rework context menu handling

by Mark Jansen

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=544b7344982bdd6a1e118… commit 544b7344982bdd6a1e1180e1a9964744ff0653b0 Author: Mark Jansen <mark.jansen(a)reactos.org> AuthorDate: Thu Sep 22 23:03:21 2022 +0200 Commit: Mark Jansen <mark.jansen(a)reactos.org> CommitDate: Sun Oct 2 00:36:42 2022 +0200 [SHELL32] CDefView: Rework context menu handling Previously, we would share one object between a multitude of options. Now, the only two options that need to store something for later use each have their own space for it. The context menu always cleans up after itself, the File menu does not. CORE-18345 CORE-18361 CORE-18366 --- dll/win32/shell32/CDefView.cpp | 164 ++++++++++++++++++++++------------------- 1 file changed, 89 insertions(+), 75 deletions(-) diff --git a/dll/win32/shell32/CDefView.cpp b/dll/win32/shell32/CDefView.cpp index 98b597c59d4..0bff83e1c9f 100644 --- a/dll/win32/shell32/CDefView.cpp +++ b/dll/win32/shell32/CDefView.cpp @@ -58,7 +58,7 @@ typedef struct static void ClientToListView(HWND hwndLV, POINT *ppt) { - POINT Origin; + POINT Origin = {}; /* FIXME: LVM_GETORIGIN is broken. See CORE-17266 */ if (!ListView_GetOrigin(hwndLV, &Origin)) @@ -68,6 +68,33 @@ ClientToListView(HWND hwndLV, POINT *ppt) ppt->y += Origin.y; } +// Helper struct to automatically cleanup the IContextMenu +// We want to explicitly reset the Site, so there are no circular references +struct MenuCleanup +{ + CComPtr<IContextMenu> &m_pCM; + HMENU &m_hMenu; + + MenuCleanup(CComPtr<IContextMenu> &pCM, HMENU& menu) + : m_pCM(pCM), m_hMenu(menu) + { + } + ~MenuCleanup() + { + if (m_hMenu) + { + DestroyMenu(m_hMenu); + m_hMenu = NULL; + } + if (m_pCM) + { + IUnknown_SetSite(m_pCM, NULL); + m_pCM.Release(); + } + } +}; + + class CDefView : public CWindowImpl<CDefView, CWindow, CControlWinTraits>, public CComObjectRootEx<CComMultiThreadModelNoCS>, @@ -116,6 +143,7 @@ class CDefView : DWORD m_grfKeyState; // CComPtr<IContextMenu> m_pCM; + CComPtr<IContextMenu> m_pFileMenu; BOOL m_isEditing; BOOL m_isParentFolderSpecial; @@ -169,7 +197,7 @@ class CDefView : void OnDeactivate(); void DoActivate(UINT uState); HRESULT drag_notify_subitem(DWORD grfKeyState, POINTL pt, DWORD *pdwEffect); - HRESULT InvokeContextMenuCommand(UINT uCommand); + HRESULT InvokeContextMenuCommand(CComPtr<IContextMenu> &pCM, UINT uCommand); LRESULT OnExplorerCommand(UINT uCommand, BOOL bUseSelection); // *** IOleWindow methods *** @@ -1310,13 +1338,6 @@ HRESULT CDefView::FillFileMenu() if (!hFileMenu) return E_FAIL; - /* Release cached IContextMenu */ - if (m_pCM) - { - IUnknown_SetSite(m_pCM, NULL); - m_pCM.Release(); - } - /* Cleanup the items added previously */ for (int i = GetMenuItemCount(hFileMenu) - 1; i >= 0; i--) { @@ -1327,15 +1348,20 @@ HRESULT CDefView::FillFileMenu() m_cidl = m_ListView.GetSelectedCount(); - /* Store the context menu in m_pCM and keep it in order to invoke the selected command later on */ - HRESULT hr = GetItemObject((m_cidl ? SVGIO_SELECTION : SVGIO_BACKGROUND), - IID_PPV_ARG(IContextMenu, &m_pCM)); + /* In case we still have this left over, clean it up! */ + if (m_pFileMenu) + { + IUnknown_SetSite(m_pFileMenu, NULL); + m_pFileMenu.Release(); + } + /* Store the context menu in m_pFileMenu and keep it in order to invoke the selected command later on */ + HRESULT hr = GetItemObject((m_cidl ? SVGIO_SELECTION : SVGIO_BACKGROUND), IID_PPV_ARG(IContextMenu, &m_pFileMenu)); if (FAILED_UNEXPECTEDLY(hr)) return hr; HMENU hmenu = CreatePopupMenu(); - hr = m_pCM->QueryContextMenu(hmenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, 0); + hr = m_pFileMenu->QueryContextMenu(hmenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, 0); if (FAILED_UNEXPECTEDLY(hr)) return hr; @@ -1470,7 +1496,7 @@ UINT CDefView::GetSelections() return m_cidl; } -HRESULT CDefView::InvokeContextMenuCommand(UINT uCommand) +HRESULT CDefView::InvokeContextMenuCommand(CComPtr<IContextMenu> &pCM, UINT uCommand) { CMINVOKECOMMANDINFO cmi; @@ -1485,7 +1511,11 @@ HRESULT CDefView::InvokeContextMenuCommand(UINT uCommand) if (GetKeyState(VK_CONTROL) & 0x8000) cmi.fMask |= CMIC_MASK_CONTROL_DOWN; - HRESULT hr = m_pCM->InvokeCommand(&cmi); + HRESULT hr = pCM->InvokeCommand(&cmi); + // Most of our callers will do this, but in case they don't do that (File menu!) + IUnknown_SetSite(pCM, NULL); + pCM.Release(); + if (FAILED_UNEXPECTEDLY(hr)) return hr; @@ -1513,33 +1543,24 @@ HRESULT CDefView::OpenSelectedItems() if (!hMenu) return E_FAIL; - hResult = GetItemObject(SVGIO_SELECTION, IID_PPV_ARG(IContextMenu, &m_pCM)); + CComPtr<IContextMenu> pCM; + hResult = GetItemObject(SVGIO_SELECTION, IID_PPV_ARG(IContextMenu, &pCM)); + MenuCleanup _(pCM, hMenu); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return hResult; - hResult = m_pCM->QueryContextMenu(hMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, CMF_DEFAULTONLY); + hResult = pCM->QueryContextMenu(hMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, CMF_DEFAULTONLY); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return hResult; uCommand = GetMenuDefaultItem(hMenu, FALSE, 0); if (uCommand == (UINT)-1) { - hResult = E_FAIL; - goto cleanup; + ERR("GetMenuDefaultItem returned -1\n"); + return E_FAIL; } - InvokeContextMenuCommand(uCommand); - -cleanup: - - if (hMenu) - DestroyMenu(hMenu); - - if (m_pCM) - { - IUnknown_SetSite(m_pCM, NULL); - m_pCM.Release(); - } + InvokeContextMenuCommand(pCM, uCommand); return hResult; } @@ -1555,6 +1576,12 @@ LRESULT CDefView::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &b TRACE("(%p)\n", this); + if (m_hContextMenu != NULL) + { + ERR("HACK: Aborting context menu in nested call!\n"); + return 0; + } + m_hContextMenu = CreatePopupMenu(); if (!m_hContextMenu) return E_FAIL; @@ -1578,15 +1605,16 @@ LRESULT CDefView::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &b } m_cidl = m_ListView.GetSelectedCount(); - + /* In case we still have this left over, clean it up! */ hResult = GetItemObject( m_cidl ? SVGIO_SELECTION : SVGIO_BACKGROUND, IID_PPV_ARG(IContextMenu, &m_pCM)); + MenuCleanup _(m_pCM, m_hContextMenu); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return 0; /* Use 1 as the first id as we want 0 the mean that the user canceled the menu */ hResult = m_pCM->QueryContextMenu(m_hContextMenu, 0, CONTEXT_MENU_BASE_ID, FCIDM_SHVIEWLAST, CMF_NORMAL); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return 0; /* There is no position requested, so try to find one */ if (lParam == ~0) @@ -1624,29 +1652,17 @@ LRESULT CDefView::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &b y = pt.y; } + // This runs the message loop, calling back to us with f.e. WM_INITPOPUP (hence why m_hContextMenu and m_pCM exist) uCommand = TrackPopupMenu(m_hContextMenu, TPM_LEFTALIGN | TPM_RETURNCMD | TPM_LEFTBUTTON | TPM_RIGHTBUTTON, x, y, 0, m_hWnd, NULL); if (uCommand == 0) - goto cleanup; + return 0; if (uCommand == FCIDM_SHVIEW_OPEN && OnDefaultCommand() == S_OK) - goto cleanup; - - InvokeContextMenuCommand(uCommand - CONTEXT_MENU_BASE_ID); + return 0; -cleanup: - if (m_pCM) - { - IUnknown_SetSite(m_pCM, NULL); - m_pCM.Release(); - } - - if (m_hContextMenu) - { - DestroyMenu(m_hContextMenu); - m_hContextMenu = NULL; - } + InvokeContextMenuCommand(m_pCM, uCommand - CONTEXT_MENU_BASE_ID); return 0; } @@ -1656,18 +1672,22 @@ LRESULT CDefView::OnExplorerCommand(UINT uCommand, BOOL bUseSelection) HRESULT hResult; HMENU hMenu = NULL; - hResult = GetItemObject( bUseSelection ? SVGIO_SELECTION : SVGIO_BACKGROUND, IID_PPV_ARG(IContextMenu, &m_pCM)); - if (FAILED_UNEXPECTEDLY( hResult)) - goto cleanup; + CComPtr<IContextMenu> pCM; + hResult = GetItemObject( bUseSelection ? SVGIO_SELECTION : SVGIO_BACKGROUND, IID_PPV_ARG(IContextMenu, &pCM)); + if (FAILED_UNEXPECTEDLY(hResult)) + return 0; + + MenuCleanup _(pCM, hMenu); + if ((uCommand != FCIDM_SHVIEW_DELETE) && (uCommand != FCIDM_SHVIEW_RENAME)) { hMenu = CreatePopupMenu(); if (!hMenu) return 0; - hResult = m_pCM->QueryContextMenu(hMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, CMF_NORMAL); + hResult = pCM->QueryContextMenu(hMenu, 0, FCIDM_SHVIEWFIRST, FCIDM_SHVIEWLAST, CMF_NORMAL); if (FAILED_UNEXPECTEDLY(hResult)) - goto cleanup; + return 0; } if (bUseSelection) @@ -1690,18 +1710,7 @@ LRESULT CDefView::OnExplorerCommand(UINT uCommand, BOOL bUseSelection) return 0; } - InvokeContextMenuCommand(uCommand); - -cleanup: - if (m_pCM) - { - IUnknown_SetSite(m_pCM, NULL); - m_pCM.Release(); - } - - if (hMenu) - DestroyMenu(hMenu); - + InvokeContextMenuCommand(pCM, uCommand); return 0; } @@ -1936,10 +1945,12 @@ LRESULT CDefView::OnCommand(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &bHand case FCIDM_SHVIEW_NEWFOLDER: return OnExplorerCommand(dwCmdID, FALSE); default: - /* WM_COMMAND messages from the file menu are routed to the CDefView so as to let m_pCM handle the command */ - if (m_pCM && dwCmd == 0) + /* WM_COMMAND messages from the file menu are routed to the CDefView so as to let m_pFileMenu handle the command */ + if (m_pFileMenu && dwCmd == 0) { - InvokeContextMenuCommand(dwCmdID); + HMENU Dummy = NULL; + MenuCleanup _(m_pFileMenu, Dummy); + InvokeContextMenuCommand(m_pFileMenu, dwCmdID); } } @@ -2370,10 +2381,10 @@ HRESULT SHSetMenuIdInMenuMsg(UINT uMsg, LPARAM lParam, UINT CmdId); */ LRESULT CDefView::OnCustomItem(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL &bHandled) { - if (!m_pCM.p) + if (!m_pCM) { /* no menu */ - ERR("no menu!!!\n"); + ERR("no context menu!!!\n"); return FALSE; } @@ -2409,7 +2420,10 @@ LRESULT CDefView::OnInitMenuPopup(UINT uMsg, WPARAM wParam, LPARAM lParam, BOOL int nPos = LOWORD(lParam); UINT menuItemId; - OnCustomItem(uMsg, wParam, lParam, bHandled); + if (m_pCM) + { + OnCustomItem(uMsg, wParam, lParam, bHandled); + } HMENU hViewMenu = GetSubmenuByID(m_hMenu, FCIDM_MENU_VIEW);

1 year, 11 months

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs October 2022