[ros-diffs] [dquintana] 62411: [RSHELL] * Improve encapsulation of the CMenuToolbarBase by avoiding unnecessary usages of the window handle from the derived classes. CORE-7881

Author: dquintana
Date: Mon Mar  3 10:19:35 2014
New Revision: 62411

URL: http://svn.reactos.org/svn/reactos?rev=62411&view=rev
Log:
[RSHELL]
* Improve encapsulation of the CMenuToolbarBase by avoiding unnecessary usages of the
window handle from the derived classes.
CORE-7881

Modified:
    branches/shell-experiments/base/shell/rshell/CMenuToolbars.cpp
    branches/shell-experiments/base/shell/rshell/CMenuToolbars.h

Modified: branches/shell-experiments/base/shell/rshell/CMenuToolbars.cpp
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/base/shell/rs…
==============================================================================
--- branches/shell-experiments/base/shell/rshell/CMenuToolbars.cpp	[iso-8859-1]
(original)
+++ branches/shell-experiments/base/shell/rshell/CMenuToolbars.cpp	[iso-8859-1] Mon Mar  3
10:19:35 2014
@@ -36,7 +36,6 @@
 
 #define TBSTYLE_EX_VERTICAL 4
 
-
 #define TIMERID_HOTTRACK 1
 #define SUBCLASS_ID_MENUBAND 1
 
@@ -170,10 +169,10 @@
 CMenuToolbarBase::CMenuToolbarBase(CMenuBand *menuBand, BOOL usePager) :
     m_hwnd(NULL),
     m_useFlatMenus(FALSE),
+    m_SubclassOld(NULL),
     m_menuBand(menuBand),
     m_hwndToolbar(NULL),
     m_dwMenuFlags(0),
-    m_SubclassOld(NULL),
     m_hasIdealSize(FALSE),
     m_usePager(usePager),
     m_hotItem(-1),
@@ -645,130 +644,56 @@
     return S_FALSE;
 }
 
-BOOL
-AllocAndGetMenuString(HMENU hMenu, UINT ItemIDByPosition, WCHAR** String)
-{
-    int Length;
-
-    Length = GetMenuStringW(hMenu, ItemIDByPosition, NULL, 0, MF_BYPOSITION);
-
-    if (!Length)
-        return FALSE;
-
-    /* Also allocate space for the terminating NULL character */
-    ++Length;
-    *String = (PWSTR) HeapAlloc(GetProcessHeap(), 0, Length * sizeof(WCHAR));
-
-    GetMenuStringW(hMenu, ItemIDByPosition, *String, Length, MF_BYPOSITION);
-
-    return TRUE;
-}
-
-CMenuStaticToolbar::CMenuStaticToolbar(CMenuBand *menuBand) :
-    CMenuToolbarBase(menuBand, FALSE),
-    m_hmenu(NULL)
-{
-}
-
-HRESULT  CMenuStaticToolbar::GetMenu(
-    HMENU *phmenu,
-    HWND *phwnd,
-    DWORD *pdwFlags)
-{
-    *phmenu = m_hmenu;
-    *phwnd = NULL;
-    *pdwFlags = m_dwMenuFlags;
-
-    return S_OK;
-}
-
-HRESULT  CMenuStaticToolbar::SetMenu(
-    HMENU hmenu,
-    HWND hwnd,
-    DWORD dwFlags)
-{
-    m_hmenu = hmenu;
-    m_dwMenuFlags = dwFlags;
-
-    return S_OK;
-}
-
-HRESULT CMenuStaticToolbar::FillToolbar()
-{
-    int i;
-    int ic = GetMenuItemCount(m_hmenu);
-
-    for (i = 0; i < ic; i++)
-    {
-        MENUITEMINFOW info;
-        TBBUTTON tbb = { 0 };
-        PWSTR MenuString = NULL;
-
-        tbb.fsState = TBSTATE_ENABLED | TBSTATE_WRAP;
-        tbb.fsStyle = 0;
-
-        info.cbSize = sizeof(info);
-        info.fMask = MIIM_FTYPE | MIIM_ID;
-
-        GetMenuItemInfoW(m_hmenu, i, TRUE, &info);
-
-        if (info.fType == MFT_STRING)
-        {
-            if (!AllocAndGetMenuString(m_hmenu, i, &MenuString))
-                return E_OUTOFMEMORY;
-            if (::GetSubMenu(m_hmenu, i) != NULL)
-                tbb.fsStyle |= BTNS_DROPDOWN;
-            tbb.iString = (INT_PTR) MenuString;
-            tbb.idCommand = info.wID;
-
-            SMINFO * sminfo = new SMINFO();
-            sminfo->dwMask = SMIM_ICON | SMIM_FLAGS;
-            if (SUCCEEDED(m_menuBand->_CallCBWithItemId(info.wID, SMC_GETINFO, 0,
reinterpret_cast<LPARAM>(sminfo))))
-            {
-                tbb.iBitmap = sminfo->iIcon;
-                tbb.dwData = reinterpret_cast<DWORD_PTR>(sminfo);
-                // FIXME: remove before deleting the toolbar or it will leak
-            }
-        }
-        else
-        {
-            tbb.fsStyle |= BTNS_SEP;
-        }
-
-        SendMessageW(m_hwndToolbar, TB_ADDBUTTONS, 1,
reinterpret_cast<LPARAM>(&tbb));
-
-        if (MenuString)
-            HeapFree(GetProcessHeap(), 0, MenuString);
-    }
-
-    return S_OK;
-}
-
-HRESULT CMenuStaticToolbar::OnContextMenu(NMMOUSE * rclick)
-{
-    CComPtr<IContextMenu> contextMenu;
-    HRESULT hr = m_menuBand->_CallCBWithItemId(rclick->dwItemSpec, SMC_GETOBJECT,
reinterpret_cast<WPARAM>(&IID_IContextMenu),
reinterpret_cast<LPARAM>(&contextMenu));
-    if (hr != S_OK)
-        return hr;
-
-    return DoContextMenu(contextMenu);
-}
-
-HRESULT CMenuStaticToolbar::OnCommand(WPARAM wParam, LPARAM lParam, LRESULT *theResult)
-{
-    HRESULT hr;
-    hr = CMenuToolbarBase::OnCommand(wParam, lParam, theResult);
-    if (FAILED(hr))
-        return hr;
-
-    // in case the clicked item has a submenu, we do not need to execute the item
-    if (hr == S_FALSE)
-        return hr;
-
-    return m_menuBand->_CallCBWithItemId(wParam, SMC_EXEC, 0, 0);
-}
-
-HRESULT CMenuStaticToolbar::PopupItem(INT uItem)
+HRESULT CMenuToolbarBase::AddButton(DWORD commandId, LPCWSTR caption, BOOL hasSubMenu,
INT iconId, DWORD_PTR buttonData)
+{
+    TBBUTTON tbb = { 0 };
+
+    tbb.fsState = TBSTATE_ENABLED | TBSTATE_WRAP;
+    tbb.fsStyle = 0;
+
+    if (hasSubMenu)
+        tbb.fsStyle |= BTNS_DROPDOWN;
+
+    tbb.iString = (INT_PTR) caption;
+    tbb.idCommand = commandId;
+
+    tbb.iBitmap = iconId;
+    tbb.dwData = buttonData;
+
+    SendMessageW(m_hwndToolbar, TB_ADDBUTTONS, 1,
reinterpret_cast<LPARAM>(&tbb));
+
+    return S_OK;
+}
+
+HRESULT CMenuToolbarBase::AddSeparator()
+{
+    TBBUTTON tbb = { 0 };
+
+    tbb.fsState = TBSTATE_ENABLED | TBSTATE_WRAP;
+    tbb.fsStyle = BTNS_SEP;
+    tbb.iBitmap = 0;
+
+    SendMessageW(m_hwndToolbar, TB_ADDBUTTONS, 1,
reinterpret_cast<LPARAM>(&tbb));
+
+    return S_OK;
+}
+
+HRESULT CMenuToolbarBase::AddPlaceholder()
+{
+    TBBUTTON tbb = { 0 };
+    PCWSTR MenuString = L"(Empty)";
+
+    tbb.fsState = TBSTATE_WRAP; // disabled
+    tbb.fsStyle = 0;
+    tbb.iString = (INT_PTR) MenuString;
+    tbb.iBitmap = -1;
+
+    SendMessageW(m_hwndToolbar, TB_ADDBUTTONS, 1,
reinterpret_cast<LPARAM>(&tbb));
+
+    return S_OK;
+}
+
+HRESULT CMenuToolbarBase::GetDataFromId(INT uItem, INT* pIndex, DWORD_PTR* pData)
 {
     TBBUTTONINFO info = { 0 };
     info.cbSize = sizeof(TBBUTTONINFO);
@@ -777,10 +702,141 @@
     if (index < 0)
         return E_FAIL;
 
-    TBBUTTON btn = { 0 };
-    SendMessage(m_hwndToolbar, TB_GETBUTTON, index,
reinterpret_cast<LPARAM>(&btn));
-
-    SMINFO * nfo = reinterpret_cast<SMINFO*>(btn.dwData);
+    if (pIndex)
+        *pIndex = index;
+
+    if (pData)
+    {
+        TBBUTTON btn = { 0 };
+        if (!SendMessage(m_hwndToolbar, TB_GETBUTTON, index,
reinterpret_cast<LPARAM>(&btn)))
+            return E_FAIL;
+        *pData = btn.dwData;
+    }
+
+    return S_OK;
+}
+
+
+HRESULT CMenuToolbarBase::PopupItem(INT uItem)
+{
+    INT index;
+    DWORD_PTR dwData;
+
+    GetDataFromId(uItem, &index, &dwData);
+
+    return InternalPopupItem(uItem, index, dwData);
+}
+
+HRESULT CMenuToolbarBase::HasSubMenu(INT uItem)
+{
+    INT index;
+    DWORD_PTR dwData;
+
+    GetDataFromId(uItem, &index, &dwData);
+
+    return InternalHasSubMenu(uItem, index, dwData);
+}
+
+CMenuStaticToolbar::CMenuStaticToolbar(CMenuBand *menuBand) :
+    CMenuToolbarBase(menuBand, FALSE),
+    m_hmenu(NULL)
+{
+}
+
+HRESULT  CMenuStaticToolbar::GetMenu(
+    HMENU *phmenu,
+    HWND *phwnd,
+    DWORD *pdwFlags)
+{
+    *phmenu = m_hmenu;
+    *phwnd = NULL;
+    *pdwFlags = m_dwMenuFlags;
+
+    return S_OK;
+}
+
+HRESULT  CMenuStaticToolbar::SetMenu(
+    HMENU hmenu,
+    HWND hwnd,
+    DWORD dwFlags)
+{
+    m_hmenu = hmenu;
+    m_dwMenuFlags = dwFlags;
+
+    return S_OK;
+}
+
+HRESULT CMenuStaticToolbar::FillToolbar()
+{
+    int i;
+    int ic = GetMenuItemCount(m_hmenu);
+
+    for (i = 0; i < ic; i++)
+    {
+        MENUITEMINFOW info;
+
+        info.cbSize = sizeof(info);
+        info.dwTypeData = NULL;
+        info.fMask = MIIM_FTYPE | MIIM_ID | MIIM_STRING | MIIM_SUBMENU;
+
+        GetMenuItemInfoW(m_hmenu, i, TRUE, &info);
+
+        if (info.fType == MFT_STRING)
+        {
+            info.cch++;
+            info.dwTypeData = (PWSTR) HeapAlloc(GetProcessHeap(), 0, (info.cch + 1) *
sizeof(WCHAR));
+
+            info.fMask = MIIM_STRING;
+            GetMenuItemInfoW(m_hmenu, i, TRUE, &info);
+
+            SMINFO * sminfo = new SMINFO();
+            sminfo->dwMask = SMIM_ICON | SMIM_FLAGS;
+            // FIXME: remove before deleting the toolbar or it will leak
+
+            HRESULT hr = m_menuBand->_CallCBWithItemId(info.wID, SMC_GETINFO, 0,
reinterpret_cast<LPARAM>(sminfo));
+            if (FAILED(hr))
+                return hr;
+
+            AddButton(info.wID, info.dwTypeData, info.hSubMenu != NULL, sminfo->iIcon,
reinterpret_cast<DWORD_PTR>(sminfo));
+
+            HeapFree(GetProcessHeap(), 0, info.dwTypeData);
+        }
+        else
+        {
+            AddSeparator();
+        }
+    }
+
+    return S_OK;
+}
+
+HRESULT CMenuStaticToolbar::OnContextMenu(NMMOUSE * rclick)
+{
+    CComPtr<IContextMenu> contextMenu;
+    HRESULT hr = m_menuBand->_CallCBWithItemId(rclick->dwItemSpec, SMC_GETOBJECT,
reinterpret_cast<WPARAM>(&IID_IContextMenu),
reinterpret_cast<LPARAM>(&contextMenu));
+    if (hr != S_OK)
+        return hr;
+
+    return DoContextMenu(contextMenu);
+}
+
+HRESULT CMenuStaticToolbar::OnCommand(WPARAM wParam, LPARAM lParam, LRESULT *theResult)
+{
+    HRESULT hr;
+    hr = CMenuToolbarBase::OnCommand(wParam, lParam, theResult);
+    if (FAILED(hr))
+        return hr;
+
+    // in case the clicked item has a submenu, we do not need to execute the item
+    if (hr == S_FALSE)
+        return hr;
+
+    return m_menuBand->_CallCBWithItemId(wParam, SMC_EXEC, 0, 0);
+}
+
+HRESULT CMenuStaticToolbar::InternalPopupItem(INT uItem, INT index, DWORD_PTR dwData)
+{
+    SMINFO * nfo = reinterpret_cast<SMINFO*>(dwData);
     if (!nfo)
         return E_FAIL;
 
@@ -799,14 +855,8 @@
     }
 }
 
-HRESULT CMenuStaticToolbar::HasSubMenu(INT uItem)
-{
-    TBBUTTONINFO info = { 0 };
-    info.cbSize = sizeof(TBBUTTONINFO);
-    info.dwMask = 0;
-    int index = SendMessage(m_hwndToolbar, TB_GETBUTTONINFO, uItem,
reinterpret_cast<LPARAM>(&info));
-    if (index < 0)
-        return E_FAIL;
+HRESULT CMenuStaticToolbar::InternalHasSubMenu(INT uItem, INT index, DWORD_PTR dwData)
+{
     return ::GetSubMenu(m_hmenu, index) ? S_OK : S_FALSE;
 }
 
@@ -836,10 +886,6 @@
         INT index = 0;
         INT indexOpen = 0;
 
-        TBBUTTON tbb = { 0 };
-        tbb.fsState = TBSTATE_ENABLED | TBSTATE_WRAP;
-        tbb.fsStyle = 0;
-
         STRRET sr = { STRRET_CSTR, { 0 } };
 
         hr = m_shellFolder->GetDisplayNameOf(item, SIGDN_NORMALDISPLAY, &sr);
@@ -855,37 +901,19 @@
         SFGAOF attrs = SFGAO_FOLDER;
         hr = m_shellFolder->GetAttributesOf(1, &itemc, &attrs);
 
-        if (attrs & SFGAO_FOLDER)
-        {
-            tbb.fsStyle |= BTNS_DROPDOWN;
-        }
-
-        tbb.idCommand = ++i;
-        tbb.iString = (INT_PTR) MenuString;
-        tbb.iBitmap = index;
-        tbb.dwData = reinterpret_cast<DWORD_PTR>(ILClone(item));
+        DWORD_PTR dwData = reinterpret_cast<DWORD_PTR>(ILClone(item));
         // FIXME: remove before deleting the toolbar or it will leak
 
-        SendMessageW(m_hwndToolbar, TB_ADDBUTTONS, 1,
reinterpret_cast<LPARAM>(&tbb));
+        AddButton(++i, MenuString, attrs & SFGAO_FOLDER, index, dwData);
+
         CoTaskMemFree(MenuString);
-
     }
     CoTaskMemFree(item);
 
     // If no items were added, show the "empty" placeholder
     if (i == 0)
     {
-        TBBUTTON tbb = { 0 };
-        PCWSTR MenuString = L"(Empty)";
-
-        tbb.fsState = 0/*TBSTATE_DISABLED*/;
-        tbb.fsStyle = 0;
-        tbb.iString = (INT_PTR) MenuString;
-        tbb.iBitmap = -1;
-
-        SendMessageW(m_hwndToolbar, TB_ADDBUTTONS, 1,
reinterpret_cast<LPARAM>(&tbb));
-
-        return S_OK;
+        return AddPlaceholder();
     }
 
     return hr;
@@ -931,25 +959,6 @@
     return hr;
 }
 
-LPITEMIDLIST CMenuSFToolbar::GetPidlFromId(INT uItem, INT* pIndex)
-{
-    TBBUTTONINFO info = { 0 };
-    info.cbSize = sizeof(TBBUTTONINFO);
-    info.dwMask = 0;
-    int index = SendMessage(m_hwndToolbar, TB_GETBUTTONINFO, uItem,
reinterpret_cast<LPARAM>(&info));
-    if (index < 0)
-        return NULL;
-
-    if (pIndex)
-        *pIndex = index;
-
-    TBBUTTON btn = { 0 };
-    if (!SendMessage(m_hwndToolbar, TB_GETBUTTON, index,
reinterpret_cast<LPARAM>(&btn)))
-        return NULL;
-
-    return reinterpret_cast<LPITEMIDLIST>(btn.dwData);
-}
-
 HRESULT CMenuSFToolbar::OnContextMenu(NMMOUSE * rclick)
 {
     HRESULT hr;
@@ -974,20 +983,22 @@
     if (hr == S_FALSE)
         return hr;
 
-    return m_menuBand->_CallCBWithItemPidl(GetPidlFromId(wParam), SMC_SFEXEC, 0, 0);
-}
-
-HRESULT CMenuSFToolbar::PopupItem(INT uItem)
+    DWORD_PTR data;
+    GetDataFromId(wParam, NULL, &data);
+
+    return m_menuBand->_CallCBWithItemPidl(reinterpret_cast<LPITEMIDLIST>(data),
SMC_SFEXEC, 0, 0);
+}
+
+HRESULT CMenuSFToolbar::InternalPopupItem(INT uItem, INT index, DWORD_PTR dwData)
 {
     HRESULT hr;
     UINT uId;
     UINT uIdAncestor;
     DWORD flags;
-    int index;
     CComPtr<IShellMenuCallback> psmc;
     CComPtr<IShellMenu> shellMenu;
 
-    LPITEMIDLIST pidl = GetPidlFromId(uItem, &index);
+    LPITEMIDLIST pidl = reinterpret_cast<LPITEMIDLIST>(dwData);
 
     if (!pidl)
         return E_FAIL;
@@ -1027,10 +1038,10 @@
     return PopupSubMenu(uItem, index, shellMenu);
 }
 
-HRESULT CMenuSFToolbar::HasSubMenu(INT uItem)
+HRESULT CMenuSFToolbar::InternalHasSubMenu(INT uItem, INT index, DWORD_PTR dwData)
 {
     HRESULT hr;
-    LPCITEMIDLIST pidl = GetPidlFromId(uItem);
+    LPCITEMIDLIST pidl = reinterpret_cast<LPITEMIDLIST>(dwData);
 
     SFGAOF attrs = SFGAO_FOLDER;
     hr = m_shellFolder->GetAttributesOf(1, &pidl, &attrs);

Modified: branches/shell-experiments/base/shell/rshell/CMenuToolbars.h
URL:
http://svn.reactos.org/svn/reactos/branches/shell-experiments/base/shell/rs…
==============================================================================
--- branches/shell-experiments/base/shell/rshell/CMenuToolbars.h	[iso-8859-1] (original)
+++ branches/shell-experiments/base/shell/rshell/CMenuToolbars.h	[iso-8859-1] Mon Mar  3
10:19:35 2014
@@ -28,12 +28,12 @@
     HWND  m_hwnd;        // May be the pager
     HFONT m_marlett;
     BOOL  m_useFlatMenus;
+    WNDPROC m_SubclassOld;
 
 protected:
     CMenuBand * m_menuBand;
     HWND        m_hwndToolbar;
     DWORD       m_dwMenuFlags;
-    WNDPROC     m_SubclassOld;
     BOOL        m_hasIdealSize;
     SIZE        m_idealSize;
     BOOL        m_usePager;
@@ -73,14 +73,23 @@
     void InvalidateDraw();
 
     virtual HRESULT FillToolbar() = 0;
-    virtual HRESULT PopupItem(INT uItem) = 0;
-    virtual HRESULT HasSubMenu(INT uItem) = 0;
     virtual HRESULT OnContextMenu(NMMOUSE * rclick) = 0;
+
+    HRESULT PopupItem(INT uItem);
+    HRESULT HasSubMenu(INT uItem);
+    HRESULT GetDataFromId(INT uItem, INT* pIndex, DWORD_PTR* pData);
 
 protected:
     virtual HRESULT OnCommand(WPARAM wParam, LPARAM lParam, LRESULT *theResult);
 
+    virtual HRESULT InternalPopupItem(INT uItem, INT index, DWORD_PTR dwData) = 0;
+    virtual HRESULT InternalHasSubMenu(INT uItem, INT index, DWORD_PTR dwData) = 0;
+
     LRESULT CALLBACK SubclassProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
+
+    HRESULT AddButton(DWORD commandId, LPCWSTR caption, BOOL hasSubMenu, INT iconId,
DWORD_PTR buttonData);
+    HRESULT AddSeparator();
+    HRESULT AddPlaceholder();
 
     HRESULT UpdateImageLists();
 };
@@ -99,11 +108,12 @@
     HRESULT GetMenu(HMENU *phmenu, HWND *phwnd, DWORD *pdwFlags);
 
     virtual HRESULT FillToolbar();
-    virtual HRESULT PopupItem(INT uItem);
-    virtual HRESULT HasSubMenu(INT uItem);
     virtual HRESULT OnCommand(WPARAM wParam, LPARAM lParam, LRESULT *theResult);
     virtual HRESULT OnContextMenu(NMMOUSE * rclick);
 
+protected:
+    virtual HRESULT InternalPopupItem(INT uItem, INT index, DWORD_PTR dwData);
+    virtual HRESULT InternalHasSubMenu(INT uItem, INT index, DWORD_PTR dwData);
 };
 
 class CMenuSFToolbar :
@@ -122,11 +132,10 @@
     HRESULT GetShellFolder(DWORD *pdwFlags, LPITEMIDLIST *ppidl, REFIID riid, void
**ppv);
 
     virtual HRESULT FillToolbar();
-    virtual HRESULT PopupItem(INT uItem);
-    virtual HRESULT HasSubMenu(INT uItem);
     virtual HRESULT OnCommand(WPARAM wParam, LPARAM lParam, LRESULT *theResult);
     virtual HRESULT OnContextMenu(NMMOUSE * rclick);
 
-private:
-    LPITEMIDLIST GetPidlFromId(INT uItem, INT* pIndex = NULL);
+protected:
+    virtual HRESULT InternalPopupItem(INT uItem, INT index, DWORD_PTR dwData);
+    virtual HRESULT InternalHasSubMenu(INT uItem, INT index, DWORD_PTR dwData);
 };