[ros-diffs] [dquintana] 63546: [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...

Author: dquintana
Date: Sat Jun  7 13:54:11 2014
New Revision: 63546

URL: http://svn.reactos.org/svn/reactos?rev=63546&view=rev
Log:
[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.

Modified:
    branches/shell-experiments/base/shell/rshell/CMenuBand.cpp
    branches/shell-experiments/base/shell/rshell/CMenuSite.cpp
    branches/shell-experiments/base/shell/rshell/precomp.h
    branches/shell-experiments/dll/win32/browseui/internettoolbar.cpp
    branches/shell-experiments/dll/win32/browseui/internettoolbar.h
    branches/shell-experiments/dll/win32/browseui/shellbrowser.cpp
    branches/shell-experiments/dll/win32/shell32/dialogs.cpp
    branches/shell-experiments/dll/win32/shell32/pidl.cpp
    branches/shell-experiments/dll/win32/shell32/shell32_main.cpp
    branches/shell-experiments/dll/win32/shell32/shlfolder.cpp

Modified: branches/shell-experiments/base/shell/rshell/CMenuBand.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/base/shell/rs…
==============================================================================
--- branches/shell-experiments/base/shell/rshell/CMenuBand.cpp	[iso-8859-1] (original)
+++ branches/shell-experiments/base/shell/rshell/CMenuBand.cpp	[iso-8859-1] Sat Jun  7
13:54:11 2014
@@ -44,7 +44,7 @@
 #else
     *ppv = NULL;
 
-    CMenuBand * site = new CComObject<CMenuBand>();
+    CMenuBand * site = new CComDebugObject<CMenuBand>();
 
     if (!site)
         return E_OUTOFMEMORY;

Modified: branches/shell-experiments/base/shell/rshell/CMenuSite.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/base/shell/rs…
==============================================================================
--- branches/shell-experiments/base/shell/rshell/CMenuSite.cpp	[iso-8859-1] (original)
+++ branches/shell-experiments/base/shell/rshell/CMenuSite.cpp	[iso-8859-1] Sat Jun  7
13:54:11 2014
@@ -354,7 +354,6 @@
             {
                 RECT Rect = { 0 };
                 GetClientRect(&Rect);
-                Rect.right = Rect.right;
                 pMenuPopup->OnPosRectChangeDB(&Rect);
             }
         }

Modified: branches/shell-experiments/base/shell/rshell/precomp.h
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/base/shell/rs…
==============================================================================
--- branches/shell-experiments/base/shell/rshell/precomp.h	[iso-8859-1] (original)
+++ branches/shell-experiments/base/shell/rshell/precomp.h	[iso-8859-1] Sat Jun  7
13:54:11 2014
@@ -113,3 +113,75 @@
 #else
 #define FAILED_UNEXPECTEDLY(hr) FAILED(hr)
 #endif
+
+
+template <class Base>
+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<Base> **pp)
+    {
+        CComDebugObject<Base>				*newInstance;
+        HRESULT								hResult;
+
+        ATLASSERT(pp != NULL);
+        if (pp == NULL)
+            return E_POINTER;
+
+        hResult = E_OUTOFMEMORY;
+        newInstance = NULL;
+        ATLTRY(newInstance = new CComDebugObject<Base>())
+            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;
+    }
+};

Modified: branches/shell-experiments/dll/win32/browseui/internettoolbar.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/bro…
==============================================================================
--- branches/shell-experiments/dll/win32/browseui/internettoolbar.cpp	[iso-8859-1]
(original)
+++ branches/shell-experiments/dll/win32/browseui/internettoolbar.cpp	[iso-8859-1] Sat Jun
 7 13:54:11 2014
@@ -547,9 +547,11 @@
     fLocked = false;
     fMenuBandWindow = NULL;
     fNavigationWindow = NULL;
-    fMenuCallback.AddRef();
+    fMenuCallback = new CComDebugObject<CMenuCallback>();
     fToolbarWindow = NULL;
     fAdviseCookie = 0;
+
+    fMenuCallback->AddRef();
 }
 
 CInternetToolbar::~CInternetToolbar()
@@ -585,73 +587,77 @@
     return ResizeBorderDW(&availableBorderSpace, fSite, FALSE);
 }
 
-HRESULT CInternetToolbar::CreateMenuBar(IShellMenu **menuBar)
-{
-    CComPtr<IOleCommandTarget>              siteCommandTarget;
-    CComPtr<IOleWindow>                     oleWindow;
-    CComPtr<IOleCommandTarget>              commandTarget;
+HRESULT CInternetToolbar::CreateMenuBar(IShellMenu **pMenuBar)
+{
+    CComPtr<IShellMenu>                     menubar;
     CComPtr<IShellMenuCallback>             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));
-    }
-#else
-    hResult = CoCreateInstance(CLSID_MenuBand, NULL, CLSCTX_INPROC_SERVER,
-        IID_PPV_ARG(IShellMenu, menuBar));
-#endif
-    if (FAILED_UNEXPECTEDLY(hResult))
-        return hResult;
-    hResult = fMenuCallback.QueryInterface(IID_PPV_ARG(IShellMenuCallback,
&callback));
-    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);
-    if (FAILED_UNEXPECTEDLY(hResult))
-        return hResult;
+            IID_PPV_ARG(IShellMenu, &menubar));
+        if (FAILED_UNEXPECTEDLY(hResult))
+            return hResult;
+    }
+    
+    hResult = fMenuCallback->QueryInterface(IID_PPV_ARG(IShellMenuCallback,
&callback));
+    if (FAILED_UNEXPECTEDLY(hResult))
+        return hResult;
+
+    hResult = menubar->Initialize(callback, -1, ANCESTORDEFAULT, SMINIT_HORIZONTAL |
SMINIT_TOPLEVEL);
+    if (FAILED_UNEXPECTEDLY(hResult))
+        return hResult;
+
+    // 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 @@
     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,

Modified: branches/shell-experiments/dll/win32/browseui/internettoolbar.h
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/bro…
==============================================================================
--- branches/shell-experiments/dll/win32/browseui/internettoolbar.h	[iso-8859-1]
(original)
+++ branches/shell-experiments/dll/win32/browseui/internettoolbar.h	[iso-8859-1] Sat Jun 
7 13:54:11 2014
@@ -83,7 +83,7 @@
     CComPtr<IUnknown>                       fLogoBar;           // the reactos
logo
     CComPtr<IUnknown>                       fControlsBar;       // navigation
controls
     CComPtr<IUnknown>                       fNavigationBar;     // address bar
-    CComObject<CMenuCallback>               fMenuCallback;
+    CComPtr<CMenuCallback>                  fMenuCallback;
     CComPtr<IOleCommandTarget>              fCommandTarget;
     GUID                                    fCommandCategory;
     HWND                                    fToolbarWindow;

Modified: branches/shell-experiments/dll/win32/browseui/shellbrowser.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/bro…
==============================================================================
--- branches/shell-experiments/dll/win32/browseui/shellbrowser.cpp	[iso-8859-1]
(original)
+++ branches/shell-experiments/dll/win32/browseui/shellbrowser.cpp	[iso-8859-1] Sat Jun  7
13:54:11 2014
@@ -903,10 +903,21 @@
     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<LPCITEMIDLIST
*>(&childPIDL), rgfInOut);
+        if (FAILED(hResult))
+            return hResult;
+    }
+
     ILFree(childPIDL);
     return S_OK;
 }

Modified: branches/shell-experiments/dll/win32/shell32/dialogs.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/she…
==============================================================================
--- branches/shell-experiments/dll/win32/shell32/dialogs.cpp	[iso-8859-1] (original)
+++ branches/shell-experiments/dll/win32/shell32/dialogs.cpp	[iso-8859-1] Sat Jun  7
13:54:11 2014
@@ -62,7 +62,7 @@
     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);
 

Modified: branches/shell-experiments/dll/win32/shell32/pidl.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/she…
==============================================================================
--- branches/shell-experiments/dll/win32/shell32/pidl.cpp	[iso-8859-1] (original)
+++ branches/shell-experiments/dll/win32/shell32/pidl.cpp	[iso-8859-1] Sat Jun  7 13:54:11
2014
@@ -753,11 +753,13 @@
  */
 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;

Modified: branches/shell-experiments/dll/win32/shell32/shell32_main.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/she…
==============================================================================
--- branches/shell-experiments/dll/win32/shell32/shell32_main.cpp	[iso-8859-1] (original)
+++ branches/shell-experiments/dll/win32/shell32/shell32_main.cpp	[iso-8859-1] Sat Jun  7
13:54:11 2014
@@ -752,7 +752,7 @@
         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);
 

Modified: branches/shell-experiments/dll/win32/shell32/shlfolder.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/dll/win32/she…
==============================================================================
--- branches/shell-experiments/dll/win32/shell32/shlfolder.cpp	[iso-8859-1] (original)
+++ branches/shell-experiments/dll/win32/shell32/shlfolder.cpp	[iso-8859-1] Sat Jun  7
13:54:11 2014
@@ -126,35 +126,41 @@
                   LPITEMIDLIST * pidlInOut, LPOLESTR szNext, DWORD * pEaten, DWORD *
pdwAttributes)
 {
     HRESULT hr = E_INVALIDARG;
-    LPITEMIDLIST pidlOut = NULL,
-      pidlTemp = NULL;
-    IShellFolder *psfChild;
-
-    TRACE ("(%p, %p, %p, %s)\n", psf, pbc, pidlInOut ? *pidlInOut : NULL,
debugstr_w (szNext));
+    LPITEMIDLIST pidlIn = pidlInOut ? *pidlInOut : NULL;
+    LPITEMIDLIST pidlOut = NULL;
+    LPITEMIDLIST pidlTemp = NULL;
+    CComPtr<IShellFolder> psfChild;
+
+    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));
-
-    if (SUCCEEDED(hr)) {
+    hr = psf->BindToObject(pidlIn, pbc, IID_PPV_ARG(IShellFolder, &psfChild));
+    if (FAILED(hr))
+        return hr;
+
     hr = psfChild->ParseDisplayName(hwndOwner, pbc, szNext, pEaten, &pidlOut,
pdwAttributes);
-    psfChild->Release();
-
-    if (SUCCEEDED(hr)) {
-        pidlTemp = ILCombine (*pidlInOut, pidlOut);
-
-        if (!pidlTemp)
+    if (FAILED(hr))
+        return hr;
+
+    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;
 }
 
 /***********************************************************************