From 7ddd2648be4d4d4495928692da95600e62c08741 Mon Sep 17 00:00:00 2001 From: David Quintana Date: Sat, 7 Jun 2014 13:54:11 +0000 Subject: [PATCH] [RSHELL] * Use the debugging class to track COM refcounting of the CMenuBand. * CMenuSite: Remove an useless line. [BROWSEUI] * Refactor the CreateMenuBar method in an attempt to figure out a seemingly magic crash with VS2010 (not yet solved). * Begin fixing some unused-but-set warnings. [SHELL32] * Fix some small bugs spotted by Victor. svn path=/branches/shell-experiments/; revision=63546 --- base/shell/rshell/CMenuBand.cpp | 2 +- base/shell/rshell/CMenuSite.cpp | 1 - base/shell/rshell/precomp.h | 72 +++++++++++++++++ dll/win32/browseui/internettoolbar.cpp | 108 +++++++++++++------------ dll/win32/browseui/internettoolbar.h | 2 +- dll/win32/browseui/shellbrowser.cpp | 11 +++ dll/win32/shell32/dialogs.cpp | 2 +- dll/win32/shell32/pidl.cpp | 4 +- dll/win32/shell32/shell32_main.cpp | 2 +- dll/win32/shell32/shlfolder.cpp | 34 ++++---- 10 files changed, 167 insertions(+), 71 deletions(-) diff --git a/base/shell/rshell/CMenuBand.cpp b/base/shell/rshell/CMenuBand.cpp index 229cb1ef8e4..c26332498aa 100644 --- a/base/shell/rshell/CMenuBand.cpp +++ b/base/shell/rshell/CMenuBand.cpp @@ -44,7 +44,7 @@ HRESULT WINAPI CMenuBand_Constructor(REFIID riid, LPVOID *ppv) #else *ppv = NULL; - CMenuBand * site = new CComObject(); + CMenuBand * site = new CComDebugObject(); if (!site) return E_OUTOFMEMORY; diff --git a/base/shell/rshell/CMenuSite.cpp b/base/shell/rshell/CMenuSite.cpp index 3ebc541c42e..384b37aa49c 100644 --- a/base/shell/rshell/CMenuSite.cpp +++ b/base/shell/rshell/CMenuSite.cpp @@ -354,7 +354,6 @@ BOOL CMenuSite::ProcessWindowMessage(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM { RECT Rect = { 0 }; GetClientRect(&Rect); - Rect.right = Rect.right; pMenuPopup->OnPosRectChangeDB(&Rect); } } diff --git a/base/shell/rshell/precomp.h b/base/shell/rshell/precomp.h index 1f86acb0a0f..575154f9c73 100644 --- a/base/shell/rshell/precomp.h +++ b/base/shell/rshell/precomp.h @@ -113,3 +113,75 @@ Win32DbgPrint(const char *filename, int line, const char *lpFormat, ...) #else #define FAILED_UNEXPECTEDLY(hr) FAILED(hr) #endif + + +template +class CComDebugObject : public Base +{ +public: + CComDebugObject(void * = NULL) + { + _pAtlModule->Lock(); + } + + virtual ~CComDebugObject() + { + this->FinalRelease(); + _pAtlModule->Unlock(); + } + + STDMETHOD_(ULONG, AddRef)() + { + int rc = this->InternalAddRef(); + DbgPrint("RefCount is now %d(++)!\n", rc); + return rc; + } + + STDMETHOD_(ULONG, Release)() + { + ULONG newRefCount; + + newRefCount = this->InternalRelease(); + DbgPrint("RefCount is now %d(--)!\n", newRefCount); + if (newRefCount == 0) + delete this; + return newRefCount; + } + + STDMETHOD(QueryInterface)(REFIID iid, void **ppvObject) + { + return this->_InternalQueryInterface(iid, ppvObject); + } + + static HRESULT WINAPI CreateInstance(CComDebugObject **pp) + { + CComDebugObject *newInstance; + HRESULT hResult; + + ATLASSERT(pp != NULL); + if (pp == NULL) + return E_POINTER; + + hResult = E_OUTOFMEMORY; + newInstance = NULL; + ATLTRY(newInstance = new CComDebugObject()) + if (newInstance != NULL) + { + newInstance->SetVoid(NULL); + newInstance->InternalFinalConstructAddRef(); + hResult = newInstance->_AtlInitialConstruct(); + if (SUCCEEDED(hResult)) + hResult = newInstance->FinalConstruct(); + if (SUCCEEDED(hResult)) + hResult = newInstance->_AtlFinalConstruct(); + newInstance->InternalFinalConstructRelease(); + if (hResult != S_OK) + { + delete newInstance; + newInstance = NULL; + } + } + *pp = newInstance; + return hResult; + } +}; diff --git a/dll/win32/browseui/internettoolbar.cpp b/dll/win32/browseui/internettoolbar.cpp index 365e74e598f..eaad484da07 100644 --- a/dll/win32/browseui/internettoolbar.cpp +++ b/dll/win32/browseui/internettoolbar.cpp @@ -547,9 +547,11 @@ CInternetToolbar::CInternetToolbar() fLocked = false; fMenuBandWindow = NULL; fNavigationWindow = NULL; - fMenuCallback.AddRef(); + fMenuCallback = new CComDebugObject(); fToolbarWindow = NULL; fAdviseCookie = 0; + + fMenuCallback->AddRef(); } CInternetToolbar::~CInternetToolbar() @@ -585,73 +587,77 @@ HRESULT CInternetToolbar::ReserveBorderSpace(LONG maxHeight) return ResizeBorderDW(&availableBorderSpace, fSite, FALSE); } -HRESULT CInternetToolbar::CreateMenuBar(IShellMenu **menuBar) +HRESULT CInternetToolbar::CreateMenuBar(IShellMenu **pMenuBar) { - CComPtr siteCommandTarget; - CComPtr oleWindow; - CComPtr commandTarget; + CComPtr menubar; CComPtr callback; VARIANT menuOut; HWND ownerWindow; HRESULT hResult; + if (!pMenuBar) + return E_POINTER; + + *pMenuBar = NULL; + + hResult = E_FAIL; #if USE_CUSTOM_MENUBAND - HMODULE hrs = LoadLibraryW(L"rshell.dll"); - - if (!hrs) - { - DbgPrint("Failed: %d\n", GetLastError()); - return E_FAIL; - } - - PMENUBAND_CONSTRUCTOR func = (PMENUBAND_CONSTRUCTOR) GetProcAddress(hrs, "CMenuBand_Constructor"); - if (func) - { - hResult = func(IID_PPV_ARG(IShellMenu, menuBar)); - } - else - { - DbgPrint("Failed: %d\n", GetLastError()); - hResult = E_FAIL; - } + HMODULE hrs = GetModuleHandleW(L"rshell.dll"); + if (!hrs) hrs = LoadLibraryW(L"rshell.dll"); + + if (hrs) + { + PMENUBAND_CONSTRUCTOR func = (PMENUBAND_CONSTRUCTOR) GetProcAddress(hrs, "CMenuBand_Constructor"); + if (func) + { + hResult = func(IID_PPV_ARG(IShellMenu, &menubar)); + } + } +#endif + + menubar->AddRef(); + if (FAILED_UNEXPECTEDLY(hResult)) { hResult = CoCreateInstance(CLSID_MenuBand, NULL, CLSCTX_INPROC_SERVER, - IID_PPV_ARG(IShellMenu, menuBar)); + IID_PPV_ARG(IShellMenu, &menubar)); + if (FAILED_UNEXPECTEDLY(hResult)) + return hResult; } -#else - hResult = CoCreateInstance(CLSID_MenuBand, NULL, CLSCTX_INPROC_SERVER, - IID_PPV_ARG(IShellMenu, menuBar)); -#endif + + hResult = fMenuCallback->QueryInterface(IID_PPV_ARG(IShellMenuCallback, &callback)); if (FAILED_UNEXPECTEDLY(hResult)) return hResult; - hResult = fMenuCallback.QueryInterface(IID_PPV_ARG(IShellMenuCallback, &callback)); + + hResult = menubar->Initialize(callback, -1, ANCESTORDEFAULT, SMINIT_HORIZONTAL | SMINIT_TOPLEVEL); if (FAILED_UNEXPECTEDLY(hResult)) return hResult; - hResult = (*menuBar)->Initialize(callback, -1, ANCESTORDEFAULT, SMINIT_HORIZONTAL | SMINIT_TOPLEVEL); - if (FAILED_UNEXPECTEDLY(hResult)) - return hResult; - hResult = fSite->QueryInterface(IID_PPV_ARG(IOleWindow, &oleWindow)); - if (FAILED_UNEXPECTEDLY(hResult)) - return hResult; - hResult = oleWindow->GetWindow(&ownerWindow); - if (FAILED_UNEXPECTEDLY(hResult)) - return hResult; - hResult = fSite->QueryInterface(IID_PPV_ARG(IOleCommandTarget, &siteCommandTarget)); - if (FAILED_UNEXPECTEDLY(hResult)) - return hResult; - hResult = siteCommandTarget->Exec(&CGID_Explorer, 0x35, 0, NULL, &menuOut); - if (FAILED_UNEXPECTEDLY(hResult)) - return hResult; - if (V_VT(&menuOut) != VT_INT_PTR || V_INTREF(&menuOut) == NULL) - return E_FAIL; - hResult = (*menuBar)->SetMenu((HMENU)V_INTREF(&menuOut), ownerWindow, SMSET_DONTOWN); - if (FAILED_UNEXPECTEDLY(hResult)) - return hResult; - hResult = IUnknown_Exec(*menuBar, CGID_MenuBand, 3, 1, NULL, NULL); + + // Set Menu + { + hResult = IUnknown_Exec(fSite, CGID_Explorer, 0x35, 0, NULL, &menuOut); + if (FAILED_UNEXPECTEDLY(hResult)) + return hResult; + + if (V_VT(&menuOut) != VT_INT_PTR || V_INTREF(&menuOut) == NULL) + return E_FAIL; + + hResult = IUnknown_GetWindow(fSite, &ownerWindow); + if (FAILED_UNEXPECTEDLY(hResult)) + return hResult; + + hResult = menubar->SetMenu((HMENU) V_INTREF(&menuOut), ownerWindow, SMSET_DONTOWN); + if (FAILED_UNEXPECTEDLY(hResult)) + return hResult; + } + + hResult = IUnknown_Exec(menubar, CGID_MenuBand, 3, 1, NULL, NULL); if (FAILED_UNEXPECTEDLY(hResult)) return hResult; + + *pMenuBar = menubar.Detach(); + return S_OK; } @@ -1686,7 +1692,7 @@ LRESULT CInternetToolbar::OnContextMenu(UINT uMsg, WPARAM wParam, LPARAM lParam, mii.cbSize = sizeof(mii); mii.fMask = MIIM_STATE; mii.fState = fLocked ? MFS_CHECKED : MFS_UNCHECKED; - command = SetMenuItemInfo(contextMenu, IDM_TOOLBARS_LOCKTOOLBARS, FALSE, &mii); + SetMenuItemInfo(contextMenu, IDM_TOOLBARS_LOCKTOOLBARS, FALSE, &mii); // TODO: use GetSystemMetrics(SM_MENUDROPALIGNMENT) to determine menu alignment command = TrackPopupMenu(contextMenu, TPM_LEFTALIGN | TPM_TOPALIGN | TPM_RIGHTBUTTON | TPM_RETURNCMD, diff --git a/dll/win32/browseui/internettoolbar.h b/dll/win32/browseui/internettoolbar.h index 92316b9d596..d2c8a144014 100644 --- a/dll/win32/browseui/internettoolbar.h +++ b/dll/win32/browseui/internettoolbar.h @@ -83,7 +83,7 @@ public: CComPtr fLogoBar; // the reactos logo CComPtr fControlsBar; // navigation controls CComPtr fNavigationBar; // address bar - CComObject fMenuCallback; + CComPtr fMenuCallback; CComPtr fCommandTarget; GUID fCommandCategory; HWND fToolbarWindow; diff --git a/dll/win32/browseui/shellbrowser.cpp b/dll/win32/browseui/shellbrowser.cpp index 3dbed76805d..9d7e77c8028 100644 --- a/dll/win32/browseui/shellbrowser.cpp +++ b/dll/win32/browseui/shellbrowser.cpp @@ -903,10 +903,21 @@ HRESULT IEGetNameAndFlagsEx(LPITEMIDLIST pidl, SHGDNF uFlags, long param10, HRESULT hResult; hResult = SHBindToFolderIDListParent(NULL, pidl, &IID_PPV_ARG(IShellFolder, &parentFolder), &childPIDL); + if (FAILED(hResult)) + return hResult; + hResult = parentFolder->GetDisplayNameOf(childPIDL, uFlags, &L108); + if (FAILED(hResult)) + return hResult; + StrRetToBufW(&L108, childPIDL, pszBuf, cchBuf); if (rgfInOut) + { hResult = parentFolder->GetAttributesOf(1, const_cast(&childPIDL), rgfInOut); + if (FAILED(hResult)) + return hResult; + } + ILFree(childPIDL); return S_OK; } diff --git a/dll/win32/shell32/dialogs.cpp b/dll/win32/shell32/dialogs.cpp index d47832ee124..d4c130e1ab3 100644 --- a/dll/win32/shell32/dialogs.cpp +++ b/dll/win32/shell32/dialogs.cpp @@ -62,7 +62,7 @@ BOOL CALLBACK EnumPickIconResourceProc(HMODULE hModule, PPICK_ICON_CONTEXT pIconContext = (PPICK_ICON_CONTEXT)lParam; if (IS_INTRESOURCE(lpszName)) - swprintf(szName, L"%u", lpszName); + swprintf(szName, L"%u", (DWORD)lpszName); else wcscpy(szName, (WCHAR*)lpszName); diff --git a/dll/win32/shell32/pidl.cpp b/dll/win32/shell32/pidl.cpp index 2f811be2c7a..7f877b99ba2 100644 --- a/dll/win32/shell32/pidl.cpp +++ b/dll/win32/shell32/pidl.cpp @@ -753,11 +753,13 @@ EXTERN_C LPITEMIDLIST WINAPI SHLogILFromFSIL(LPITEMIDLIST pidl) */ UINT WINAPI ILGetSize(LPCITEMIDLIST pidl) { - LPCSHITEMID si = &(pidl->mkid); + LPCSHITEMID si; UINT len = 0; if (pidl) { + si = &(pidl->mkid); + while (si->cb) { len += si->cb; diff --git a/dll/win32/shell32/shell32_main.cpp b/dll/win32/shell32/shell32_main.cpp index 5b914d40222..9d39b1bd3c2 100644 --- a/dll/win32/shell32/shell32_main.cpp +++ b/dll/win32/shell32/shell32_main.cpp @@ -752,7 +752,7 @@ DWORD_PTR WINAPI SHGetFileInfoA(LPCSTR path,DWORD dwFileAttributes, temppsfi.dwAttributes=psfi->dwAttributes; if (psfi == NULL) - ret = SHGetFileInfoW(pathW, dwFileAttributes, NULL, sizeof(temppsfi), flags); + ret = SHGetFileInfoW(pathW, dwFileAttributes, NULL, 0, flags); else ret = SHGetFileInfoW(pathW, dwFileAttributes, &temppsfi, sizeof(temppsfi), flags); diff --git a/dll/win32/shell32/shlfolder.cpp b/dll/win32/shell32/shlfolder.cpp index e821066ec1d..30dab9ebc4e 100644 --- a/dll/win32/shell32/shlfolder.cpp +++ b/dll/win32/shell32/shlfolder.cpp @@ -126,35 +126,41 @@ HRESULT SHELL32_ParseNextElement (IShellFolder2 * psf, HWND hwndOwner, LPBC pbc, LPITEMIDLIST * pidlInOut, LPOLESTR szNext, DWORD * pEaten, DWORD * pdwAttributes) { HRESULT hr = E_INVALIDARG; - LPITEMIDLIST pidlOut = NULL, - pidlTemp = NULL; - IShellFolder *psfChild; + LPITEMIDLIST pidlIn = pidlInOut ? *pidlInOut : NULL; + LPITEMIDLIST pidlOut = NULL; + LPITEMIDLIST pidlTemp = NULL; + CComPtr psfChild; - TRACE ("(%p, %p, %p, %s)\n", psf, pbc, pidlInOut ? *pidlInOut : NULL, debugstr_w (szNext)); + TRACE ("(%p, %p, %p, %s)\n", psf, pbc, pidlIn, debugstr_w (szNext)); /* get the shellfolder for the child pidl and let it analyse further */ - hr = psf->BindToObject(*pidlInOut, pbc, IID_PPV_ARG(IShellFolder, &psfChild)); + hr = psf->BindToObject(pidlIn, pbc, IID_PPV_ARG(IShellFolder, &psfChild)); + if (FAILED(hr)) + return hr; - if (SUCCEEDED(hr)) { hr = psfChild->ParseDisplayName(hwndOwner, pbc, szNext, pEaten, &pidlOut, pdwAttributes); - psfChild->Release(); + if (FAILED(hr)) + return hr; - if (SUCCEEDED(hr)) { - pidlTemp = ILCombine (*pidlInOut, pidlOut); - - if (!pidlTemp) + pidlTemp = ILCombine (pidlIn, pidlOut); + if (!pidlTemp) + { hr = E_OUTOFMEMORY; + if (pidlOut) + ILFree(pidlOut); + return hr; } if (pidlOut) ILFree (pidlOut); - } - ILFree (*pidlInOut); + if (pidlIn) + ILFree (pidlIn); + *pidlInOut = pidlTemp; TRACE ("-- pidl=%p ret=0x%08x\n", pidlInOut ? *pidlInOut : NULL, hr); - return hr; + return S_OK; } /***********************************************************************