03/06: [SHELL32_APITEST] Show that our current SHCreateFileExtractIconW is incorrect. CORE-14082

[Mark Jansen]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=7fe78f2779bf47a9e2d14…
commit 7fe78f2779bf47a9e2d1495f96b56fce8fb8060e
Author:     Mark Jansen <mark.jansen(a)reactos.org>
AuthorDate: Tue Mar 27 19:58:43 2018 +0200
Commit:     Mark Jansen <mark.jansen(a)reactos.org>
CommitDate: Sat Apr 7 15:29:59 2018 +0200
    [SHELL32_APITEST] Show that our current SHCreateFileExtractIconW is incorrect.
    CORE-14082
---
 .../apitests/shell32/SHCreateFileExtractIconW.cpp  | 172 ++++++++++++++-------
 1 file changed, 113 insertions(+), 59 deletions(-)
diff --git a/modules/rostests/apitests/shell32/SHCreateFileExtractIconW.cpp
b/modules/rostests/apitests/shell32/SHCreateFileExtractIconW.cpp
index b48054ee86..144ce8128e 100644
--- a/modules/rostests/apitests/shell32/SHCreateFileExtractIconW.cpp
+++ b/modules/rostests/apitests/shell32/SHCreateFileExtractIconW.cpp
@@ -2,7 +2,7 @@
  * PROJECT:     ReactOS API tests
  * LICENSE:     GPL-2.0+ (https://spdx.org/licenses/GPL-2.0+)
  * PURPOSE:     Test for SHCreateFileExtractIconW
- * COPYRIGHT:   Copyright 2017 Mark Jansen (mark.jansen(a)reactos.org)
+ * COPYRIGHT:   Copyright 2017,2018 Mark Jansen (mark.jansen(a)reactos.org)
  */
 #include "shelltest.h"
@@ -35,6 +35,22 @@ static TestData IconTests[] =
     { NULL, FILE_ATTRIBUTE_DIRECTORY },
 };
+struct TestIID
+{
+    const GUID* IID;
+    HRESULT ExpectedCreate;
+    HRESULT ExpectedQueryInterface;
+};
+
+static TestIID InterfaceTests[] =
+{
+    { &IID_IDefaultExtractIconInit, E_NOINTERFACE, E_NOINTERFACE },
+    { &IID_IExtractIconW, S_OK, S_OK },
+    { &IID_IExtractIconA, S_OK, S_OK },
+    { &IID_IPersist, E_NOINTERFACE, E_NOINTERFACE },
+    { &IID_IPersistFile, E_NOINTERFACE, E_NOINTERFACE },
+};
+
 static void ExtractOneBitmap(HBITMAP hbm, CComHeapPtr<BYTE>& data, DWORD&
size)
 {
@@ -81,6 +97,9 @@ START_TEST(SHCreateFileExtractIconW)
     HICON myIcon;
     pSHCreateFileExtractIconW = (HRESULT (__stdcall *)(LPCWSTR, DWORD, REFIID, void
**))GetProcAddress(shell32, "SHCreateFileExtractIconW");
+    /* Show that icons returned are always the same */
+    UINT tryFlags[4] = { 0, GIL_FORSHORTCUT, GIL_OPENICON };
+
     CoInitialize(NULL);
     GetModuleFileNameW(NULL, CurrentModule, _countof(CurrentModule));
@@ -101,6 +120,24 @@ START_TEST(SHCreateFileExtractIconW)
         return;
     }
+    for (size_t n = 0; n < _countof(InterfaceTests); ++n)
+    {
+        {
+            CComPtr<IUnknown> spUnknown;
+            HRESULT hr = pSHCreateFileExtractIconW(L"test.txt",
FILE_ATTRIBUTE_NORMAL, *InterfaceTests[n].IID, (void**)&spUnknown);
+            ok(hr == InterfaceTests[n].ExpectedCreate, "Expected hr to be 0x%lx, was
0x%lx for %u\n", InterfaceTests[n].ExpectedCreate, hr, n);
+        }
+
+        {
+            CComPtr<IUnknown> spUnknown, spUnknown2;
+            HRESULT hr = pSHCreateFileExtractIconW(L"test.txt",
FILE_ATTRIBUTE_NORMAL, IID_PPV_ARG(IUnknown, &spUnknown));
+            ok(hr == S_OK, "Expected hr to be S_OK, was 0x%lx for %u\n", hr,
n);
+
+            hr = spUnknown->QueryInterface(*InterfaceTests[n].IID,
(void**)&spUnknown2);
+            ok(hr == InterfaceTests[n].ExpectedQueryInterface, "Expected hr to be
0x%lx, was 0x%lx for %u\n", InterfaceTests[n].ExpectedQueryInterface, hr, n);
+        }
+    }
+
     for (size_t n = 0; n < _countof(IconTests); ++n)
     {
         TestData& cur = IconTests[n];
@@ -119,85 +156,102 @@ START_TEST(SHCreateFileExtractIconW)
         if (!SUCCEEDED(hr))
             continue;
-        int ilIndex = -1;
-        UINT wFlags = 0xdeaddead;
-        WCHAR Buffer[MAX_PATH];
+        /* Show that GIL_DEFAULTICON does not work. */
+        {
+            int ilIndex = -1;
+            UINT wFlags = 0xdeaddead;
+            WCHAR Buffer[MAX_PATH];
+
+            hr = spExtract->GetIconLocation(GIL_DEFAULTICON, Buffer, _countof(Buffer),
&ilIndex, &wFlags);
+            ok(hr == S_FALSE, "Expected hr to be S_FALSE, was 0x%lx for
%S(0x%lx)\n", hr, cur.Name, cur.dwFlags);
+        }
-        hr = spExtract->GetIconLocation(0, Buffer, _countof(Buffer), &ilIndex,
&wFlags);
-        ok(hr == S_OK, "Expected hr to be S_OK, was 0x%lx for %S(%lx)\n", hr,
cur.Name, cur.dwFlags);
-        if (!SUCCEEDED(hr))
-            continue;
-        ok(wFlags & (GIL_NOTFILENAME|GIL_PERCLASS), "Expected
GIL_NOTFILENAME|GIL_PERCLASS to be set for %S(%lx)\n", cur.Name, cur.dwFlags);
-        ok(!wcscmp(Buffer, L"*"), "Expected '*', was '%S'
for %S(%lx)\n", Buffer, cur.Name, cur.dwFlags);
+        for (UINT idFlags = 0; idFlags < _countof(tryFlags); ++idFlags)
+        {
+            int ilIndex = -1;
+            UINT wFlags = 0xdeaddead;
+            WCHAR Buffer[MAX_PATH];
-        HICON ico;
-        hr = spExtract->Extract(Buffer, ilIndex, &ico, NULL, 0);
+            hr = spExtract->GetIconLocation(tryFlags[idFlags], Buffer,
_countof(Buffer), &ilIndex, &wFlags);
+            ok(hr == S_OK, "Expected hr to be S_OK, was 0x%lx for
%S(0x%lx,0x%x)\n", hr, cur.Name, cur.dwFlags, tryFlags[idFlags]);
+            if (!SUCCEEDED(hr))
+                continue;
-        /* Visualize the icon extracted for whoever is stepping through this code. */
-        HWND console = GetConsoleWindow();
-        SendMessage(console, WM_SETICON, ICON_BIG, (LPARAM)ico);
-        SendMessage(console, WM_SETICON, ICON_SMALL, (LPARAM)ico);
+            ok(wFlags & (GIL_NOTFILENAME|GIL_PERCLASS), "Expected
GIL_NOTFILENAME|GIL_PERCLASS to be set for %S(0x%lx,0x%x)\n", cur.Name, cur.dwFlags,
tryFlags[idFlags]);
+            ok(!wcscmp(Buffer, L"*"), "Expected '*', was
'%S' for %S(0x%lx,0x%x)\n", Buffer, cur.Name, cur.dwFlags,
tryFlags[idFlags]);
-        CComHeapPtr<BYTE> colorData, maskData;
-        DWORD colorSize = 0, maskSize = 0;
+            HICON ico;
+            hr = spExtract->Extract(Buffer, ilIndex, &ico, NULL, 0);
-        GetIconData(ico, colorData, colorSize, maskData, maskSize);
+            /* Visualize the icon extracted for whoever is stepping through this code. */
+            HWND console = GetConsoleWindow();
+            SendMessage(console, WM_SETICON, ICON_BIG, (LPARAM)ico);
+            SendMessage(console, WM_SETICON, ICON_SMALL, (LPARAM)ico);
-        if (!colorSize || !maskSize)
-            continue;
+            CComHeapPtr<BYTE> colorData, maskData;
+            DWORD colorSize = 0, maskSize = 0;
-        SHFILEINFOW shfi;
-        ULONG_PTR firet = SHGetFileInfoW(cur.Name, cur.dwFlags, &shfi, sizeof(shfi),
SHGFI_USEFILEATTRIBUTES | SHGFI_ICON);
+            GetIconData(ico, colorData, colorSize, maskData, maskSize);
-        if (!firet)
-            continue;
+            if (!colorSize || !maskSize)
+                continue;
-        CComHeapPtr<BYTE> colorDataRef, maskDataRef;
-        DWORD colorSizeRef = 0, maskSizeRef = 0;
-        GetIconData(shfi.hIcon, colorDataRef, colorSizeRef, maskDataRef, maskSizeRef);
+            SHFILEINFOW shfi;
+            ULONG_PTR firet = SHGetFileInfoW(cur.Name, cur.dwFlags, &shfi,
sizeof(shfi), SHGFI_USEFILEATTRIBUTES | SHGFI_ICON | SHGFI_SYSICONINDEX);
-        ok(colorSizeRef == colorSize, "Expected %lu, was %lu for %S(%lx)\n",
colorSizeRef, colorSize, cur.Name, cur.dwFlags);
-        ok(maskSizeRef == maskSize, "Expected %lu, was %lu for %S(%lx)\n",
maskSizeRef, maskSize, cur.Name, cur.dwFlags);
+            if (!firet)
+                continue;
-        if (colorSizeRef == colorSize)
-        {
-            ok(!memcmp(colorData, colorDataRef, colorSize), "Expected equal
colorData for %S(%lx)\n", cur.Name, cur.dwFlags);
-        }
+            ok(shfi.iIcon == ilIndex, "Expected ilIndex to be 0%x, was 0x%x for
%S(0x%lx,0x%x)\n", shfi.iIcon, ilIndex, cur.Name, cur.dwFlags, tryFlags[idFlags]);
-        if (maskSizeRef == maskSize)
-        {
-            ok(!memcmp(maskData, maskDataRef, maskSize), "Expected equal maskData
for %S(%lx)\n", cur.Name, cur.dwFlags);
-        }
-        if (useMyIcon)
-        {
-            colorDataRef.Free();
-            maskDataRef.Free();
-            colorSizeRef = maskSizeRef = 0;
-            GetIconData(myIcon, colorDataRef, colorSizeRef, maskDataRef, maskSizeRef);
+            CComHeapPtr<BYTE> colorDataRef, maskDataRef;
+            DWORD colorSizeRef = 0, maskSizeRef = 0;
+            GetIconData(shfi.hIcon, colorDataRef, colorSizeRef, maskDataRef,
maskSizeRef);
-            ok(colorSizeRef == colorSize, "Expected %lu, was %lu for
%S(%lx)\n", colorSizeRef, colorSize, cur.Name, cur.dwFlags);
-            ok(maskSizeRef == maskSize, "Expected %lu, was %lu for %S(%lx)\n",
maskSizeRef, maskSize, cur.Name, cur.dwFlags);
+            ok(colorSizeRef == colorSize, "Expected %lu, was %lu for
%S(0x%lx,0x%x)\n", colorSizeRef, colorSize, cur.Name, cur.dwFlags,
tryFlags[idFlags]);
+            ok(maskSizeRef == maskSize, "Expected %lu, was %lu for
%S(0x%lx,0x%x)\n", maskSizeRef, maskSize, cur.Name, cur.dwFlags, tryFlags[idFlags]);
             if (colorSizeRef == colorSize)
             {
-                /* Incase requested filetype does not match, the exe icon is not used! */
-                if (cur.dwFlags == FILE_ATTRIBUTE_DIRECTORY)
-                {
-                    ok(memcmp(colorData, colorDataRef, colorSize), "Expected
colorData to be changed for %S(%lx)\n", cur.Name, cur.dwFlags);
-                }
-                else
+                ok(!memcmp(colorData, colorDataRef, colorSize), "Expected equal
colorData for %S(0x%lx,0x%x)\n", cur.Name, cur.dwFlags, tryFlags[idFlags]);
+            }
+
+            if (maskSizeRef == maskSize)
+            {
+                ok(!memcmp(maskData, maskDataRef, maskSize), "Expected equal
maskData for %S(0x%lx,0x%x)\n", cur.Name, cur.dwFlags, tryFlags[idFlags]);
+            }
+
+            if (useMyIcon)
+            {
+                colorDataRef.Free();
+                maskDataRef.Free();
+                colorSizeRef = maskSizeRef = 0;
+                GetIconData(myIcon, colorDataRef, colorSizeRef, maskDataRef,
maskSizeRef);
+
+                ok(colorSizeRef == colorSize, "Expected %lu, was %lu for
%S(0x%lx,0x%x)\n", colorSizeRef, colorSize, cur.Name, cur.dwFlags,
tryFlags[idFlags]);
+                ok(maskSizeRef == maskSize, "Expected %lu, was %lu for
%S(0x%lx,0x%x)\n", maskSizeRef, maskSize, cur.Name, cur.dwFlags, tryFlags[idFlags]);
+
+                if (colorSizeRef == colorSize)
                 {
-                    ok(!memcmp(colorData, colorDataRef, colorSize), "Expected equal
colorData for %S(%lx)\n", cur.Name, cur.dwFlags);
+                    /* In case requested filetype does not match, the exe icon is not
used! */
+                    if (cur.dwFlags == FILE_ATTRIBUTE_DIRECTORY)
+                    {
+                        ok(memcmp(colorData, colorDataRef, colorSize), "Expected
colorData to be changed for %S(0x%lx,0x%x)\n", cur.Name, cur.dwFlags,
tryFlags[idFlags]);
+                    }
+                    else
+                    {
+                        ok(!memcmp(colorData, colorDataRef, colorSize), "Expected
equal colorData for %S(0x%lx,0x%x)\n", cur.Name, cur.dwFlags, tryFlags[idFlags]);
+                    }
                 }
-            }
-            // Mask is not reliable for some reason
-            //if (maskSizeRef == maskSize)
-            //{
-            //    ok(!memcmp(maskData, maskDataRef, maskSize), "Expected equal
maskData for %S(%lx)\n", cur.Name, cur.dwFlags);
-            //}
+                // Mask is not reliable for some reason
+                //if (maskSizeRef == maskSize)
+                //{
+                //    ok(!memcmp(maskData, maskDataRef, maskSize), "Expected equal
maskData for %S(0x%lx,0x%lx)\n", cur.Name, cur.dwFlags);
+                //}
+            }
         }
     }
 }