[reactos] 01/01: [SHDOCVW][BROWSEUI][SHELL32] Correctly refcount CNSCBand and Explorer File menu (#7569)

[Whindmar Saksit]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=1fec01561d06ae6a4cfce…

commit 1fec01561d06ae6a4cfce38daa474631ed6a9fbe
Author:     Whindmar Saksit <whindsaks(a)proton.me>
AuthorDate: Fri Dec 27 14:46:30 2024 +0100
Commit:     GitHub <noreply(a)github.com>
CommitDate: Fri Dec 27 14:46:30 2024 +0100

    [SHDOCVW][BROWSEUI][SHELL32] Correctly refcount CNSCBand and Explorer File menu
(#7569)
    
    CORE-19879 CORE-19780
---
 dll/win32/browseui/basebarsite.cpp        | 49 ++++++++++++-------------------
 dll/win32/browseui/shellbrowser.cpp       | 15 ++++------
 dll/win32/shdocvw/CNSCBand.cpp            |  3 +-
 dll/win32/shell32/CDefView.cpp            | 13 +++++---
 dll/win32/shell32/shellmenu/CMenuSite.cpp |  3 ++
 dll/win32/shell32/shellmenu/CMenuSite.h   |  1 +
 6 files changed, 38 insertions(+), 46 deletions(-)

diff --git a/dll/win32/browseui/basebarsite.cpp b/dll/win32/browseui/basebarsite.cpp
index e67e9b7e9bd..dc40977de4d 100644
--- a/dll/win32/browseui/basebarsite.cpp
+++ b/dll/win32/browseui/basebarsite.cpp
@@ -380,15 +380,13 @@ HRESULT STDMETHODCALLTYPE CBaseBarSite::SetDeskBarSite(IUnknown
*punkSite)
 
     if (punkSite == NULL)
     {
-
-        TRACE("Destroying site \n");
+        TRACE("Destroying site\n");
         /* Cleanup our bands */
-        while(SUCCEEDED(EnumBands(-1, &dwBandID)) && dwBandID)
+        for (UINT i = EnumBands(-1, NULL); i;)
         {
-            hResult = EnumBands(0, &dwBandID);
-            if(FAILED_UNEXPECTEDLY(hResult))
-                continue;
-            RemoveBand(dwBandID);
+            hResult = EnumBands(--i, &dwBandID);
+            if (!FAILED_UNEXPECTEDLY(hResult))
+                RemoveBand(dwBandID);
         }
         fDeskBarSite = NULL;
     }
@@ -535,13 +533,11 @@ HRESULT STDMETHODCALLTYPE CBaseBarSite::EnumBands(UINT uBand, DWORD
*pdwBandID)
 {
     REBARBANDINFO bandInfo;
 
+    if (uBand == -1ul)
+        return (HRESULT)SendMessage(RB_GETBANDCOUNT, 0, 0);
     if (pdwBandID == NULL)
         return E_INVALIDARG;
-    if (uBand == 0xffffffff)
-    {
-        *pdwBandID = (DWORD)SendMessage(RB_GETBANDCOUNT, 0, 0);
-        return S_OK;
-    }
+
     if (!SUCCEEDED(GetInternalBandInfo(uBand, &bandInfo)))
         return E_INVALIDARG;
     *pdwBandID = bandInfo.wID;
@@ -565,7 +561,7 @@ HRESULT STDMETHODCALLTYPE CBaseBarSite::RemoveBand(DWORD dwBandID)
     HRESULT                         hr;
     CBarInfo                        *pInfo;
     CComPtr<IObjectWithSite>        pSite;
-    CComPtr<IDeskBand>              pDockWnd;
+    CComPtr<IDockingWindow>         pDockWnd;
     DWORD                           index;
 
     // Retrieve the right index of the coolbar knowing the id
@@ -580,19 +576,14 @@ HRESULT STDMETHODCALLTYPE CBaseBarSite::RemoveBand(DWORD dwBandID)
     if (!pInfo)
         return E_INVALIDARG;
 
-    hr = pInfo->fTheBar->QueryInterface(IID_PPV_ARG(IDeskBand, &pDockWnd));
-    if (FAILED_UNEXPECTEDLY(hr))
-    {
-        return E_NOINTERFACE;
-    }
-    hr = pInfo->fTheBar->QueryInterface(IID_PPV_ARG(IObjectWithSite, &pSite));
-    if (FAILED_UNEXPECTEDLY(hr))
-    {
-        return E_NOINTERFACE;
-    }
     /* Windows sends a CloseDW before setting site to NULL */
-    pDockWnd->CloseDW(0);
-    pSite->SetSite(NULL);
+    hr = pInfo->fTheBar->QueryInterface(IID_PPV_ARG(IDockingWindow,
&pDockWnd));
+    if (SUCCEEDED(hr))
+        pDockWnd->CloseDW(0);
+
+    hr = pInfo->fTheBar->QueryInterface(IID_PPV_ARG(IObjectWithSite, &pSite));
+    if (SUCCEEDED(hr))
+        pSite->SetSite(NULL);
 
     // Delete the band from rebar
     if (!SendMessage(RB_DELETEBAND, index, 0))
@@ -773,15 +764,11 @@ HRESULT CBaseBarSite::FindBandByGUID(REFGUID pGuid, DWORD
*pdwBandID)
 {
     DWORD                       numBands;
     DWORD                       i;
-    HRESULT                     hr;
     REBARBANDINFO               bandInfo;
     CBarInfo                    *realInfo;
 
-    hr = EnumBands(-1, &numBands);
-    if (FAILED_UNEXPECTEDLY(hr))
-        return E_FAIL;
-
-    for(i = 0; i < numBands; i++)
+    numBands = EnumBands(-1, NULL);
+    for (i = 0; i < numBands; i++)
     {
         if (FAILED_UNEXPECTEDLY(GetInternalBandInfo(i, &bandInfo)))
             return E_FAIL;
diff --git a/dll/win32/browseui/shellbrowser.cpp b/dll/win32/browseui/shellbrowser.cpp
index eb67209a674..cd729c02998 100644
--- a/dll/win32/browseui/shellbrowser.cpp
+++ b/dll/win32/browseui/shellbrowser.cpp
@@ -1276,11 +1276,8 @@ BOOL CShellBrowser::IsBandLoaded(const CLSID clsidBand, bool
vertical, DWORD *pd
     if (FAILED_UNEXPECTEDLY(hResult))
         return FALSE;
 
-    hResult = bandSite->EnumBands(-1, &numBands);
-    if (FAILED_UNEXPECTEDLY(hResult))
-        return FALSE;
-
-    for(i = 0; i < numBands; i++)
+    numBands = bandSite->EnumBands(-1, NULL);
+    for (i = 0; i < numBands; i++)
     {
         CComPtr<IPersist> bandPersist;
 
@@ -1504,7 +1501,6 @@ LRESULT CALLBACK CShellBrowser::WindowProc(HWND hWnd, UINT uMsg,
WPARAM wParam,
             wParam = msg.wParam;
             lParam = msg.lParam;
         }
-        menuBand.Release();
     }
 
     handled = pThis->ProcessWindowMessage(hWnd, uMsg, wParam, lParam, lResult, 0);
@@ -3658,16 +3654,15 @@ LRESULT CShellBrowser::OnDestroy(UINT uMsg, WPARAM wParam, LPARAM
lParam, BOOL &
     // TODO: rip down everything
     {
         m_Destroyed = true; // Ignore browse requests from Explorer band TreeView during
destruction
+        fCurrentShellView->UIActivate(SVUIA_DEACTIVATE);
         fToolbarProxy.Destroy();
         fCurrentShellView->DestroyViewWindow();
-        fCurrentShellView->UIActivate(SVUIA_DEACTIVATE);
 
         for (int i = 0; i < 3; i++)
         {
             CComPtr<IDockingWindow> pdw;
             CComPtr<IDeskBar> bar;
             CComPtr<IUnknown> pBarSite;
-            CComPtr<IDeskBarClient> pClient;
 
             if (fClientBars[i].clientBar == NULL)
                 continue;
@@ -3683,6 +3678,7 @@ LRESULT CShellBrowser::OnDestroy(UINT uMsg, WPARAM wParam, LPARAM
lParam, BOOL &
                 hr = bar->GetClient(&pBarSite);
                 if (SUCCEEDED(hr) && pBarSite)
                 {
+                    CComPtr<IDeskBarClient> pClient;
                     hr = pBarSite->QueryInterface(IID_PPV_ARG(IDeskBarClient,
&pClient));
                     if (SUCCEEDED(hr))
                         pClient->SetDeskBarSite(NULL);
@@ -3690,7 +3686,6 @@ LRESULT CShellBrowser::OnDestroy(UINT uMsg, WPARAM wParam, LPARAM
lParam, BOOL &
             }
             pdw->CloseDW(0);
 
-            pClient = NULL;
             pBarSite = NULL;
             pdw = NULL;
             bar = NULL;
@@ -3720,7 +3715,7 @@ LRESULT CShellBrowser::OnSize(UINT uMsg, WPARAM wParam, LPARAM
lParam, BOOL &bHa
         GetEffectiveClientRect(m_hWnd, &availableBounds, excludeItems);
         for (INT x = 0; x < 3; x++)
         {
-            if (fClientBars[x].clientBar != NULL)
+            if (fClientBars[x].clientBar)
             {
                 hResult = fClientBars[x].clientBar->QueryInterface(
                     IID_PPV_ARG(IDockingWindow, &dockingWindow));
diff --git a/dll/win32/shdocvw/CNSCBand.cpp b/dll/win32/shdocvw/CNSCBand.cpp
index d40a4c8ca37..f653a1a043c 100644
--- a/dll/win32/shdocvw/CNSCBand.cpp
+++ b/dll/win32/shdocvw/CNSCBand.cpp
@@ -93,7 +93,8 @@ CNSCBand::~CNSCBand()
 VOID CNSCBand::OnFinalMessage(HWND)
 {
     // The message loop is finished, now we can safely destruct!
-    static_cast<IDeskBand *>(this)->Release();
+    // HACKFIX: Who did this AddRef? Commenting out Release...
+    //static_cast<IDeskBand *>(this)->Release();
 }
 
 // *** helper methods ***
diff --git a/dll/win32/shell32/CDefView.cpp b/dll/win32/shell32/CDefView.cpp
index 0505c8e7b69..21e7c1bcc4f 100644
--- a/dll/win32/shell32/CDefView.cpp
+++ b/dll/win32/shell32/CDefView.cpp
@@ -1593,13 +1593,18 @@ LRESULT CDefView::OnDestroy(UINT uMsg, WPARAM wParam, LPARAM
lParam, BOOL &bHand
     if (!m_Destroyed)
     {
         m_Destroyed = TRUE;
+        RevokeDragDrop(m_hWnd);
+        SHChangeNotifyDeregister(m_hNotify);
+        if (m_pFileMenu)
+        {
+            IUnknown_SetSite(m_pFileMenu, NULL);
+            m_pFileMenu = NULL;
+        }
         if (m_hMenu)
         {
             DestroyMenu(m_hMenu);
             m_hMenu = NULL;
         }
-        RevokeDragDrop(m_hWnd);
-        SHChangeNotifyDeregister(m_hNotify);
         m_hNotify = NULL;
         SHFree(m_pidlParent);
         m_pidlParent = NULL;
@@ -3055,7 +3060,7 @@ HRESULT WINAPI CDefView::TranslateAccelerator(LPMSG lpmsg)
         TRACE("-- key=0x%04lx\n", lpmsg->wParam);
     }
 
-    return m_pShellBrowser->TranslateAcceleratorSB(lpmsg, 0);
+    return m_pShellBrowser ? m_pShellBrowser->TranslateAcceleratorSB(lpmsg, 0) :
S_FALSE;
 }
 
 HRESULT WINAPI CDefView::EnableModeless(BOOL fEnable)
@@ -3290,7 +3295,7 @@ HRESULT CDefView::SaveViewState(IStream *pStream)
     lvc.mask = LVCF_WIDTH | LVCF_SUBITEM;
     for (UINT i = 0, j = 0; i < PERSISTCOLUMNS::MAXCOUNT &&
ListView_GetColumn(m_ListView, j, &lvc); ++j)
     {
-        HRESULT hr = MapListColumnToFolderColumn(lvc.iSubItem);
+        HRESULT hr = MapListColumnToFolderColumn(j);
         if (SUCCEEDED(hr))
         {
             cols.Columns[i] = MAKELONG(hr, lvc.cx);
diff --git a/dll/win32/shell32/shellmenu/CMenuSite.cpp
b/dll/win32/shell32/shellmenu/CMenuSite.cpp
index 6faf0fabb0c..7596237b596 100644
--- a/dll/win32/shell32/shellmenu/CMenuSite.cpp
+++ b/dll/win32/shell32/shellmenu/CMenuSite.cpp
@@ -85,6 +85,9 @@ HRESULT STDMETHODCALLTYPE CMenuSite::AddBand(IUnknown * punk)
 
 HRESULT STDMETHODCALLTYPE CMenuSite::EnumBands(UINT uBand, DWORD* pdwBandID)
 {
+    if (uBand == -1ul)
+        return GetBandCount();
+
     if (uBand != 0)
         return E_FAIL;
 
diff --git a/dll/win32/shell32/shellmenu/CMenuSite.h
b/dll/win32/shell32/shellmenu/CMenuSite.h
index 3421e1be307..713791390f0 100644
--- a/dll/win32/shell32/shellmenu/CMenuSite.h
+++ b/dll/win32/shell32/shellmenu/CMenuSite.h
@@ -104,4 +104,5 @@ public:
 
 private:
     IUnknown * ToIUnknown() { return static_cast<IDeskBarClient*>(this); }
+    UINT GetBandCount() { return m_BandObject ? 1 : 0; }
 };