[SHDOCVW][SHDOCVW_APITEST] Implement MRU List for Shell Bag, Part 6 (#5699)

Follow-up to #5691. Strengthen shdocvw_apitest
testcase. Fix CLSID_MruPidlList class.
CORE-9283

dll/win32/shdocvw/mrulist.cpp

@@ -34,6 +34,7 @@ class CMruClassFactory;
 
 extern "C" void __cxa_pure_virtual(void)
 {
+    ERR("__cxa_pure_virtual\n");
     ::DebugBreak();
 }
 
@@ -129,7 +130,7 @@ public:
     STDMETHODIMP AddData(LPCVOID pvData, DWORD cbData, UINT *piSlot) override;
     STDMETHODIMP FindData(LPCVOID pvData, DWORD cbData, UINT *piSlot) override;
     STDMETHODIMP GetData(UINT iSlot, LPVOID pvData, DWORD cbData) override;
-    STDMETHODIMP QueryInfo(UINT iSlot, UINT *puSlot, DWORD *pcbData) override;
+    STDMETHODIMP QueryInfo(UINT iSlot, UINT *piGotSlot, DWORD *pcbData) override;
     STDMETHODIMP Delete(UINT iSlot) override;
 
     // Non-standard methods
@@ -172,8 +173,7 @@ CMruBase::~CMruBase()
             m_pSlots[iSlot].pvData = ::LocalFree(m_pSlots[iSlot].pvData);
         }
 
-        ::LocalFree(m_pSlots);
+        m_pSlots = (SLOTITEMDATA*)::LocalFree(m_pSlots);
-        m_pSlots = NULL;
     }
 
     ::InterlockedDecrement(&SHDOCVW_refCount);
@@ -371,21 +371,21 @@ STDMETHODIMP CMruBase::GetData(UINT iSlot, LPVOID pvData, DWORD cbData)
         return hr;
 
     if (cbData < pItem->cbData)
-        return 0x8007007A; // FIXME: Magic number
+        return HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER);
 
     CopyMemory(pvData, pItem->pvData, pItem->cbData);
     return hr;
 }
 
-STDMETHODIMP CMruBase::QueryInfo(UINT iSlot, UINT *puSlot, DWORD *pcbData)
+STDMETHODIMP CMruBase::QueryInfo(UINT iSlot, UINT *piGotSlot, DWORD *pcbData)
 {
     UINT iGotSlot;
     HRESULT hr = _GetSlot(iSlot, &iGotSlot);
     if (FAILED(hr))
         return hr;
 
-    if (puSlot)
+    if (piGotSlot)
-        *puSlot = iGotSlot;
+        *piGotSlot = iGotSlot;
 
     if (pcbData)
     {
@@ -604,11 +604,7 @@ public:
 
     ~CMruLongList() override
     {
-        if (m_puSlotData)
+        m_puSlotData = (UINT*)::LocalFree(m_puSlotData);
-        {
-            ::LocalFree(m_puSlotData);
-            m_puSlotData = NULL;
-        }
     }
 };
 
@@ -1067,13 +1063,13 @@ BOOL CMruPidlList::_LoadNodeSlots()
     DWORD cbNodeSlots = m_cSlotRooms * sizeof(BYTE);
     if (SHGetValueW(m_hKey, NULL, L"NodeSlots", NULL, m_pbNodeSlots, &cbNodeSlots) != ERROR_SUCCESS)
         return FALSE;
-    m_cMaxNodeSlots = m_cSlotRooms;
+    m_cMaxNodeSlots = cbNodeSlots / sizeof(BYTE);
     return TRUE;
 }
 
 void CMruPidlList::_SaveNodeSlots()
 {
-    DWORD cbNodeSlots = m_cSlotRooms * sizeof(BYTE);
+    DWORD cbNodeSlots = m_cMaxNodeSlots * sizeof(BYTE);
     SHSetValueW(m_hKey, NULL, L"NodeSlots", REG_BINARY, m_pbNodeSlots, cbNodeSlots);
 }
 
@@ -1126,34 +1122,53 @@ HRESULT CMruPidlList::GetEmptySlot(UINT *pnNodeSlot)
 
 STDMETHODIMP CMruPidlList::InitList(UINT cMRUSize, HKEY hKey, LPCWSTR pszSubKey)
 {
+    TRACE("%p -> %u %p %s\n", this, cMRUSize, hKey, debugstr_w(pszSubKey));
+
     HRESULT hr = InitData(cMRUSize, 0, hKey, pszSubKey, NULL);
     if (FAILED(hr))
+    {
+        ERR("0x%08lX\n", hr);
         return hr;
+    }
 
     hr = _InitNodeSlots();
     if (FAILED(hr))
+    {
+        ERR("0x%08lX\n", hr);
         return hr;
+    }
 
     m_hMutex = ::CreateMutexW(NULL, FALSE, L"Shell.CMruPidlList");
     if (!m_hMutex)
+    {
         hr = HRESULT_FROM_WIN32(GetLastError());
+        ERR("0x%08lX\n", hr);
+    }
 
     return hr;
 }
 
 STDMETHODIMP CMruPidlList::UsePidl(LPCITEMIDLIST pidl, UINT *pnNodeSlot)
 {
+    TRACE("%p -> %p %p\n", this, pidl, pnNodeSlot);
+
     CSafeMutex mutex;
     HRESULT hr = mutex.Enter(m_hMutex);
     if (FAILED(hr))
+    {
+        ERR("0x%08lX\n", hr);
         return hr;
+    }
 
     *pnNodeSlot = 0;
 
     CMruNode *pNode;
     hr = GetNode(TRUE, pidl, &pNode);
     if (FAILED(hr))
+    {
+        ERR("0x%08lX\n", hr);
         return hr;
+    }
 
     hr = pNode->GetNodeSlot(pnNodeSlot);
     if (FAILED(hr))
@@ -1175,17 +1190,25 @@ STDMETHODIMP CMruPidlList::QueryPidl(
     UINT *pnNodeSlots,
     UINT *pcNodeSlots)
 {
+    TRACE("%p -> %p %u %p %p\n", this, pidl, cSlots, pnNodeSlots, pcNodeSlots);
+
     CSafeMutex mutex;
     HRESULT hr = mutex.Enter(m_hMutex);
     if (FAILED(hr))
+    {
+        ERR("0x%08lX\n", hr);
         return hr;
+    }
 
     *pcNodeSlots = 0;
 
     CMruNode *pNode;
     hr = GetNode(FALSE, pidl, &pNode);
     if (FAILED(hr))
+    {
+        ERR("0x%08lX\n", hr);
         return hr;
+    }
 
     while (pNode && *pcNodeSlots < cSlots)
     {
@@ -1210,10 +1233,15 @@ STDMETHODIMP CMruPidlList::QueryPidl(
 
 STDMETHODIMP CMruPidlList::PruneKids(LPCITEMIDLIST pidl)
 {
+    TRACE("%p -> %p\n", this, pidl);
+
     CSafeMutex mutex;
     HRESULT hr = mutex.Enter(m_hMutex);
     if (FAILED(hr))
+    {
+        ERR("0x%08lX\n", hr);
         return hr;
+    }
 
     if (!_LoadNodeSlots())
         return hr;

dll/win32/shdocvw/shdocvw_v1.rgs

@@ -203,6 +203,10 @@ HKCR
         {
             InprocServer32 = s '%MODULE%' { val ThreadingModel = s 'Apartment' }
         }
+        '{42AEDC87-2188-41FD-B9A3-0C966FEABEC1}' = s 'MruPidlList'
+        {
+            InprocServer32 = s '%MODULE%' { val ThreadingModel = s 'Apartment' }
+        }
     }
     'Shell.Explorer.1' = s 'Microsoft Web Browser Version 1'
     {

modules/rostests/apitests/shdocvw/CMakeLists.txt

@@ -6,5 +6,5 @@ list(APPEND SOURCE
 add_executable(shdocvw_apitest ${SOURCE})
 set_module_type(shdocvw_apitest win32cui)
 target_link_libraries(shdocvw_apitest ${PSEH_LIB} uuid)
-add_importlibs(shdocvw_apitest shlwapi oleaut32 ole32 user32 advapi32 msvcrt kernel32)
+add_importlibs(shdocvw_apitest shlwapi shell32 oleaut32 ole32 user32 advapi32 msvcrt kernel32)
 add_rostests_file(TARGET shdocvw_apitest)

modules/rostests/apitests/shdocvw/MRUList.cpp

@@ -19,10 +19,11 @@
 #include <pseh/pseh2.h>
 
 #define SUBKEY0 L"Software\\MRUListTest"
+#define SUBSUBKEY0 L"Software\\MRUListTest\\0"
 #define TEXT0 L"This is a test."
 #define TEXT1 L"ReactOS rocks!"
 
-static void MRUList_List0(void)
+static void MRUList_DataList_0(void)
 {
     HRESULT hr;
     IMruDataList *pList = NULL;
@@ -64,27 +65,35 @@ static void MRUList_List0(void)
     pList->Release();
 }
 
-static void MRUList_List0_Check(void)
+static INT MRUList_Check(LPCWSTR pszSubKey, LPCWSTR pszValueName, LPCVOID pvData, DWORD cbData)
 {
     BYTE abData[512];
-    DWORD cbData, dwType;
+    LONG error;
+    DWORD dwSize = cbData;
 
-    cbData = sizeof(abData);
+    error = SHGetValueW(HKEY_CURRENT_USER, pszSubKey, pszValueName, NULL, abData, &dwSize);
-    LONG error = SHGetValueW(HKEY_CURRENT_USER, SUBKEY0, L"MRUListEx", &dwType, abData, &cbData);
+    if (error != ERROR_SUCCESS)
-    ok_long(error, ERROR_SUCCESS);
+        return -999;
-    ok_long(dwType, REG_BINARY);
+
-#if 1
+#if 0
-    ok_int(memcmp(abData, "\x01\x00\x00\x00\x00\x00\x00\x00\xFF\xFF\xFF\xFF", 12), 0);
+    printf("dwSize: %ld\n", dwSize);
-#else
+    for (DWORD i = 0; i < dwSize; ++i)
-    for (DWORD i = 0; i < cbData; ++i)
     {
         printf("%02X ", abData[i]);
     }
     printf("\n");
 #endif
+
+    if (dwSize != cbData)
+        return +999;
+
+    if (!pvData)
+        return TRUE;
+
+    return memcmp(abData, pvData, cbData) == 0;
 }
 
-static void MRUList_List1(void)
+static void MRUList_DataList_1(void)
 {
     HRESULT hr;
     IMruDataList *pList = NULL;
@@ -119,27 +128,7 @@ static void MRUList_List1(void)
     pList->Release();
 }
 
-static void MRUList_List1_Check(void)
+static void MRUList_DataList_2(void)
-{
-    BYTE abData[512];
-    DWORD cbData, dwType;
-
-    cbData = sizeof(abData);
-    LONG error = SHGetValueW(HKEY_CURRENT_USER, SUBKEY0, L"MRUListEx", &dwType, abData, &cbData);
-    ok_long(error, ERROR_SUCCESS);
-    ok_long(dwType, REG_BINARY);
-#if 1
-    ok_int(memcmp(abData, "\x01\x00\x00\x00\xFF\xFF\xFF\xFF", 8), 0);
-#else
-    for (DWORD i = 0; i < cbData; ++i)
-    {
-        printf("%02X ", abData[i]);
-    }
-    printf("\n");
-#endif
-}
-
-static void MRUList_List2(void)
 {
     HRESULT hr;
     IMruDataList *pList = NULL;
@@ -172,27 +161,7 @@ static void MRUList_List2(void)
     pList->Release();
 }
 
-static void MRUList_List2_Check(void)
+static void MRUList_DataList(void)
-{
-    BYTE abData[512];
-    DWORD cbData, dwType;
-
-    cbData = sizeof(abData);
-    LONG error = SHGetValueW(HKEY_CURRENT_USER, SUBKEY0, L"MRUListEx", &dwType, abData, &cbData);
-    ok_long(error, ERROR_SUCCESS);
-    ok_long(dwType, REG_BINARY);
-#if 1
-    ok_int(memcmp(abData, "\x00\x00\x00\x00\x01\x00\x00\x00\xFF\xFF\xFF\xFF", 12), 0);
-#else
-    for (DWORD i = 0; i < cbData; ++i)
-    {
-        printf("%02X ", abData[i]);
-    }
-    printf("\n");
-#endif
-}
-
-static void MRUList_List(void)
 {
     if (IsWindowsVistaOrGreater())
     {
@@ -209,11 +178,11 @@ static void MRUList_List(void)
     error = SHGetValueW(HKEY_CURRENT_USER, SUBKEY0, NULL, NULL, NULL, NULL);
     ok_long(error, ERROR_SUCCESS);
 
-    MRUList_List0();
+    MRUList_DataList_0();
-    MRUList_List0_Check();
+    ok_int(MRUList_Check(SUBKEY0, L"MRUListEx", "\x01\x00\x00\x00\x00\x00\x00\x00\xFF\xFF\xFF\xFF", 12), TRUE);
 
-    MRUList_List1();
+    MRUList_DataList_1();
-    MRUList_List1_Check();
+    ok_int(MRUList_Check(SUBKEY0, L"MRUListEx", "\x01\x00\x00\x00\xFF\xFF\xFF\xFF", 8), TRUE);
 
     error = SHDeleteValueW(HKEY_CURRENT_USER, SUBKEY0, L"MRUList");
     ok_long(error, ERROR_FILE_NOT_FOUND);
@@ -227,8 +196,8 @@ static void MRUList_List(void)
     error = SHSetValueW(HKEY_CURRENT_USER, SUBKEY0, L"b", REG_BINARY, L"XYZ", 4 * sizeof(WCHAR));
     ok_long(error, ERROR_SUCCESS);
 
-    MRUList_List2();
+    MRUList_DataList_2();
-    MRUList_List2_Check();
+    ok_int(MRUList_Check(SUBKEY0, L"MRUListEx", "\x00\x00\x00\x00\x01\x00\x00\x00\xFF\xFF\xFF\xFF", 12), TRUE);
 
     error = SHDeleteValueW(HKEY_CURRENT_USER, SUBKEY0, L"MRUList");
     ok_long(error, ERROR_FILE_NOT_FOUND);
@@ -238,12 +207,139 @@ static void MRUList_List(void)
     SHDeleteKeyW(HKEY_CURRENT_USER, SUBKEY0);
 }
 
+static void MRUList_PidlList_0(void)
+{
+    HRESULT hr;
+    IMruPidlList *pList = NULL;
+
+    hr = CoCreateInstance(CLSID_MruPidlList, NULL, CLSCTX_INPROC_SERVER,
+                          IID_IMruPidlList, (LPVOID*)&pList);
+    ok_hex(hr, S_OK);
+    if (pList == NULL)
+    {
+        skip("pList was NULL\n");
+        return;
+    }
+
+    LONG error;
+
+    error = SHGetValueW(HKEY_CURRENT_USER, SUBKEY0, NULL, NULL, NULL, NULL);
+    ok_long(error, ERROR_FILE_NOT_FOUND);
+
+    hr = pList->InitList(32, HKEY_CURRENT_USER, SUBKEY0);
+    ok_hex(hr, S_OK);
+
+    error = SHGetValueW(HKEY_CURRENT_USER, SUBKEY0, NULL, NULL, NULL, NULL);
+    ok_long(error, ERROR_FILE_NOT_FOUND);
+
+    LPITEMIDLIST pidl1, pidl2;
+    SHGetSpecialFolderLocation(NULL, CSIDL_DESKTOP, &pidl1);
+    SHGetSpecialFolderLocation(NULL, CSIDL_PERSONAL, &pidl2);
+
+    UINT uNodeSlot1 = 0xDEADFACE;
+    hr = pList->UsePidl(pidl1, &uNodeSlot1);
+    ok_hex(uNodeSlot1, 1);
+
+    // "NodeSlot" value
+    ok_int(MRUList_Check(SUBKEY0, L"NodeSlot", "\x01\x00\x00\x00", 4), TRUE);
+
+    // "NodeSlots" value (Not "NodeSlot")
+    ok_int(MRUList_Check(SUBKEY0, L"NodeSlots", "\x02", 1), TRUE);
+
+    UINT uNodeSlot2 = 0xDEADFACE;
+    hr = pList->UsePidl(pidl2, &uNodeSlot2);
+    ok_hex(uNodeSlot2, 2);
+
+    // "0" value
+    ok_int(MRUList_Check(SUBKEY0, L"0", NULL, 22), TRUE);
+
+    // "MRUListEx" value
+    ok_int(MRUList_Check(SUBKEY0, L"MRUListEx", "\x00\x00\x00\x00\xFF\xFF\xFF\xFF", 8), TRUE);
+
+    // "NodeSlot" value
+    ok_int(MRUList_Check(SUBKEY0, L"NodeSlot", "\x01\x00\x00\x00", 4), TRUE);
+
+    // "NodeSlots" value
+    ok_int(MRUList_Check(SUBKEY0, L"NodeSlots", "\x02\x02", 2), TRUE);
+
+    // SUBSUBKEY0: "MRUListEx" value
+    ok_int(MRUList_Check(SUBSUBKEY0, L"MRUListEx", "\xFF\xFF\xFF\xFF", 4), TRUE);
+
+    // SUBSUBKEY0: "NodeSlot" value
+    ok_int(MRUList_Check(SUBSUBKEY0, L"NodeSlot", "\x02\x00\x00\x00", 4), TRUE);
+
+    // QueryPidl
+    UINT anNodeSlot[2], cNodeSlots;
+    FillMemory(anNodeSlot, sizeof(anNodeSlot), 0xCC);
+    cNodeSlots = 0xDEAD;
+    hr = pList->QueryPidl(pidl1, _countof(anNodeSlot), anNodeSlot, &cNodeSlots);
+    ok_long(hr, S_OK);
+    ok_int(anNodeSlot[0], 1);
+    ok_int(anNodeSlot[1], 0xCCCCCCCC);
+    ok_int(cNodeSlots, 1);
+
+    hr = pList->PruneKids(pidl1);
+
+    // "MRUListEx" value
+    ok_int(MRUList_Check(SUBKEY0, L"MRUListEx", "\x00\x00\x00\x00\xFF\xFF\xFF\xFF", 8), TRUE);
+
+    // "NodeSlot" value
+    ok_int(MRUList_Check(SUBKEY0, L"NodeSlot", "\x01\x00\x00\x00", 4), TRUE);
+
+    // "NodeSlots" value
+    ok_int(MRUList_Check(SUBKEY0, L"NodeSlots", "\x02\x00", 2), TRUE);
+
+    FillMemory(anNodeSlot, sizeof(anNodeSlot), 0xCC);
+    cNodeSlots = 0xBEEF;
+    hr = pList->QueryPidl(pidl1, 0, anNodeSlot, &cNodeSlots);
+    ok_long(hr, E_FAIL);
+    ok_int(anNodeSlot[0], 0xCCCCCCCC);
+    ok_int(anNodeSlot[1], 0xCCCCCCCC);
+    ok_int(cNodeSlots, 0);
+
+    FillMemory(anNodeSlot, sizeof(anNodeSlot), 0xCC);
+    cNodeSlots = 0xDEAD;
+    hr = pList->QueryPidl(pidl1, _countof(anNodeSlot), anNodeSlot, &cNodeSlots);
+    ok_long(hr, S_OK);
+    ok_int(anNodeSlot[0], 1);
+    ok_int(anNodeSlot[1], 0xCCCCCCCC);
+    ok_int(cNodeSlots, 1);
+
+    FillMemory(anNodeSlot, sizeof(anNodeSlot), 0xCC);
+    cNodeSlots = 0xDEAD;
+    hr = pList->QueryPidl(pidl2, _countof(anNodeSlot), anNodeSlot, &cNodeSlots);
+    ok_long(hr, S_FALSE);
+    ok_int(anNodeSlot[0], 1);
+    ok_int(anNodeSlot[1], 0xCCCCCCCC);
+    ok_int(cNodeSlots, 1);
+
+    pList->Release();
+    ILFree(pidl1);
+    ILFree(pidl2);
+}
+
+static void MRUList_PidlList(void)
+{
+    if (IsWindowsVistaOrGreater())
+    {
+        skip("Vista+ doesn't support CLSID_MruPidlList\n");
+        return;
+    }
+
+    SHDeleteKeyW(HKEY_CURRENT_USER, SUBKEY0);
+
+    MRUList_PidlList_0();
+
+    SHDeleteKeyW(HKEY_CURRENT_USER, SUBKEY0);
+}
+
 START_TEST(MRUList)
 {
     HRESULT hr = CoInitialize(NULL);
     ok_hex(hr, S_OK);
 
-    MRUList_List();
+    MRUList_DataList();
+    MRUList_PidlList();
 
     if (SUCCEEDED(hr))
         CoUninitialize();